-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
architecture/additional_concepts/authorization: improvements to the list of allowed volume types #4831
Conversation
LGTM |
PTAL @openshift/team-documentation |
LGTM. Looks like this is targeting 3.6, correct? |
@@ -551,14 +551,14 @@ The recommended minimum set of allowed volumes for new SCCs are *configMap*, | |||
|
|||
[NOTE] | |||
==== | |||
`***` is a special value to allow the use of all volume types. | |||
`***` is a special value to allow the use of all volume types. `*none*` is a special value to disallow the use of all volumes types (exist only for backward compatibility). |
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.
Should none be in the list above, under *** ?
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.
Yes, I can put it there.
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. I also add more unrelated changes that I found.
@php-coder One question. Otherwise LGTM |
Correct. (The field name can by applied to other branches...) |
5c697cf
to
3ebab9c
Compare
PTAL again, I've added many minor improvements. |
|
||
[NOTE] | ||
==== | ||
`***` is a special value to allow the use of all volume types. | ||
The list of allowable volume types could be not full because the new types are | ||
adding in each release of {product-title}. |
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.
s/"The list of allowable volume types is not exhaustive because new types are added with each release of {product-title}." Or similar.
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.
Can we keep the list updated with each release, so that the list is complete?
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.
Can we keep the list updated with each release, so that the list is complete?
Yes, perhaps, but who will responsible to do that each release?
@php-coder One comment and question |
…the list of allowed volume types. - add missing volume types - mention "none" volume type - add links to the Kubernetes documentation for the volume types - fix field name - add note that the list can be outdated
3ebab9c
to
cf78f30
Compare
@mburke5678 Comment addressed. PTAL. |
LGTM! |
Thanks! |
No revision history needed |
@pweil- @liggitt PTAL and let me know if we don't want to document "none" value.
CC @simo5