-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kb for renewing expired harvester cloud credentials #60
Conversation
✅ Deploy Preview for harvester-home-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
bf188a1
to
f05a9c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, LGTM.
Should we also provide a resolution for users who are going to create a cloud credential for their managed Harvester cluster? By updating the global ttl setting? However, as this is a global setting, it will be applied to other cases as well rather than Harvester only.
Also, replace crd with CRD.
@innobead i guess this can be used to set a custom expiry on harvester cloud credentials manually while retaining normal expiry intervals for non cloud credentials. |
f05a9c5
to
84f3669
Compare
@FrankYang0529 @Vicente-Cheng please review this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. Just add a suggestion, we could also provide the one-line command for the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This KB article should also mention the two other cases:
- The workaround how to avoid the tokens from expiring in the first place, for new clusters, i.e. setting the default- and maximum-token-ttl settings to 0
- The workaround to recover a cluster whose token has expired:
Create new cloud credentials (preferably such that the token doesn't expire, see 1) and patch the cluster's yaml such that the.spec.cloudCredentialSecretName
references the new cloud credential.
These two workarounds would still need to be confirmed by someone on the Rancher team though, as I'm not sure they are completely free of other subtle bugs.
Another problem with this workaround in general is that it relies on the token still being there. This isn't guaranteed, because expired tokens are garbage-collected after a while, so if the user can't react to the tokens expiry within a short time, this workaround won't work for them at all. See case 2 on how to recover in that case.
Highlight known issue about token TTL in Harvester node driver/Rancher integration. The highlighting also links to the Knowledgebase article describing the temporary workaround. depends-on: harvester/harvesterhci.io#60 Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
You mean setting the TTLs to If yes, then this is very risk as explained in #60 (comment). |
There are three different problems this KB article should cover:
That's all I'm asking. I don't care what the exact solution to each of the three problems is, but these three points should be covered. [1]: A short side note on security in this context: I've found absolutely zero discussion on expiring tokens and their improvement of security, besides that the original PR, which changed the token TTL for Rancher v2.8.x, claims that it does. |
update instructions for patching cloud credentials updated title
8789a0b
to
17940f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reused content from doc PR 567, which is ready for merging. Let me know if you have concerns about the changes.
include feedback from Jilllian. Co-authored-by: Jillian <67180770+jillian-maroket@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
Highlight known issue about token TTL in Harvester node driver/Rancher integration. The highlighting also links to the Knowledgebase article describing the temporary workaround. depends-on: harvester/harvesterhci.io#60 Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
* Docs: Highlight known issue about token TTL Highlight known issue about token TTL in Harvester node driver/Rancher integration. The highlighting also links to the Knowledgebase article describing the temporary workaround. depends-on: harvester/harvesterhci.io#60 Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com> * re-format after code-review suggestion Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com> * update KB article URL Update KB article URL to the final published one. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com> --------- Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
KB detailing steps on how to renew expired new harvester cloud credentials on harvester
ref: harvester/harvester#5827