-
Notifications
You must be signed in to change notification settings - Fork 378
Simplify credentials from 8 to 4 #200
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
Simplify credentials from 8 to 4 #200
Conversation
csi.proto
Outdated
// passing through the required credentials. This information is | ||
// sensitive and MUST be treated as such (not logged, etc.) by the CO. | ||
// Secrets required by plugin to complete controller unpublish volume | ||
// request. This should be the same secrets passed to the |
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.
nit: s/should/SHOULD/
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.
Done.
csi.proto
Outdated
// A secret is a string to string map where the key identifies the | ||
// name of the secret (e.g. "username" or "password"), and the value | ||
// contains the secret data (e.g. "bob" or "abc123"). | ||
// Each key must consist of alphanumeric characters, '-', '_' or '.'. |
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.
nit: s/must/MUST/ (here and throughout changeset)
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.
Done.
spec.md
Outdated
@@ -1636,7 +1631,7 @@ Supervised plugins MAY be isolated and/or resource-bounded. | |||
* Logging configuration flags and/or variables, including working sample configurations. | |||
* Default log destination(s) (where do the logs go if no configuration is specified?) | |||
* Log lifecycle management ownership and related guidance (size limits, rate limits, rolling, archiving, expunging, etc.) applicable to the logging mechanism embedded within the Plugin. | |||
* Plugins SHOULD NOT write potentially sensitive data to logs (e.g. `Credentials`). | |||
* Plugins SHOULD NOT write potentially sensitive data to logs (e.g. `Secrets`). |
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.
nit: there are no types or fields called Secrets
. maybe just replace "Secrets
" with "secrets"?
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.
Done.
5ee1992
to
1a3affd
Compare
1a3affd
to
b5dd57d
Compare
Feedback addressed. PTAL |
Conclusion on today's call was folks prefer #201 over this. So I will close this PR. |
Fixes #198
provisioner_secrets
CreateVolume()
andDeleteVolume()
.controller_publish_secrets
ControllerPublishVolume()
andControllerUnpublishVolume()
node_stage_secrets
NodeStageVolume()
only.NodeUnstageVolume()
which we can consider adding in the future, if needed.node_publish_secrets
NodePublishVolume()
onlyNodeUnpublishVolume()
which we can consider adding in the future, if needed.