Add warning for horizon timeout

[vwallfahrer]

Horizon timeout is impacted by keystone token timeout since when the
token gets outdated, then the user is automatically unable to use
horizon. This patch includes a check for the timeout of horizon and
keystone barclamps.

[houndci-bot]

Style/IndentationWidth: Use 2 (not 4) spaces for indentation.

[vuntz]

I'd suggest rephrasing to: "The session timeout cannot be larger than the Keystone token expiration timeout."

[rsalevsky]

Hrm... code looks good so far but it's the same code twice. Maybe but it in an validation helper method and just call the method.

[AbelNavarro]

@rsalevsky you are right there is some code repeated (3 lines). Since the code is repeated in two different files and it's very specific for the validation, I'd say it would make it harder to read if you need to go to a third file for just this three lines. Would the code had been in the same file or had been more critical, I would totally go for a non-repetition approach.

[AbelNavarro]

@vuntz 's comment seems to have disappeared after the pull request update. This is his original comment:

I'd suggest rephrasing to: "The session timeout cannot be larger than the Keystone token expiration timeout."

Btw, I agree with the rephrasing. Although I'd explicitly mention this is horizon's session timeout:
"The horizon session timeout cannot be larger than the Keystone token expiration timeout."

[AbelNavarro]

Just thought of another corner case to try: what happens if we have keystone barclamp installed but not the horizon one? Probably there should be a check when retrieving the proposal to see it's not empty.

[AbelNavarro]

I checked our default values for the expirations in cloud5:

• horizon: 24 hours (1440 minutes)
• keystone: 4 hours (14400 seconds)

So, the situation that triggers this problem is there by default. There won't be any warning in crowbar because it will only show upon a change. We will need to change the default values.

If we change the default values, should we also change any documentation for the end user?

[AbelNavarro]

I still haven't fully backported this to cloud5 and probably cannot finish this soon. FTR, this is what the code would look like for cloud5:

    horizon_timeout = proposal["attributes"]["nova_dashboard"]["session_timeout"]
    keystone_proposal = ProposalObject.find_proposal("keystone", "default")
    unless keystone_proposal.nil?
      keystone_timeout = keystone_proposal["attributes"]["keystone"]["token_expiration"]

      # keystone_timeout is in seconds and horizon_timeout is in minutes
      if horizon_timeout * 60 > keystone_timeout
        validation_error("The horizon session timeout cannot be larger than the Keystone token expiration timeout.")
      end
    end

[vuntz]

If we change the default values, should we also change any documentation for the end user?

I don't think we document that, so go ahead.

[AbelNavarro]

Regarding the default timeouts, what about making both (horizon + keystone) to be 4 hours?

[vuntz]

Regarding the default timeouts, what about making both (horizon + keystone) to be 4 hours?

We need to make the settings consistent, at least. I'd possibly go for 24 hours, but no strong opinion.

[vuntz]

In this case, there's no guarantee that we have a horizon proposal, so we should check if horizon_proposal is nil.

[vuntz]

Also needs a rebase

[houndci-bot]

unexpected token tSTRING_BEG
unexpected token tRPAREN

[vuntz]

Here I would change the error and say "Please increase the Keystone token expiration timeout." (since the user is editing the keystone proposal)

[houndci-bot]

Metrics/LineLength: Line is too long. [159/150]

[tboerger]

I would prefer the messages to be part of the i18n file as they are getting touched anyway.

[AbelNavarro]

since @vwallfahrer is sick, I'm continuing the PR in this new one: #51

[tboerger]

Continued in #51