ROX-30434: Updates to install docs for admission controller changes #100082
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@kcarmichael08: This pull request references ROX-30434 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
size/M
|
@kcarmichael08: This pull request references ROX-30434 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
kcarmichael08
force-pushed
the
ROX-30434-admission-controller-install
branch
3 times, most recently
from
4ea5380 to
1689d47
Compare
|
@kcarmichael08: This pull request references ROX-30434 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@kcarmichael08: This pull request references ROX-30434 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
kcarmichael08
added
rhacs-docs-4.9
RHACS
modules/roxctl-sensor-generate.adoc
Outdated
| |Disable the bypass annotations for the admission controller. The default value is `false`. | ||
|
|
||
| |`--admission-controller-enforcement` | ||
| |Valid values are `true` and `false`. The default value is `true`. When set to `true`, the admission controller enforces a policy and rejects the deployment or update attempt. When set to `false`, the admission controller does not enforce a policy and allows a deployment or update to the cluster that violates the policy conditions. |
There was a problem hiding this comment.
| |Valid values are `true` and `false`. The default value is `true`. When set to `true`, the admission controller enforces a policy and rejects the deployment or update attempt. When set to `false`, the admission controller does not enforce a policy and allows a deployment or update to the cluster that violates the policy conditions. | |
| |Valid values are `true` and `false`. The default value is `true`. When set to `true`, the admission controller enforces policies, which means that it rejects creation or update attempts which are in violation with an enabled policy. When set to `false`, the admission controller does not enforce policies. |
In my suggestion I have tried to address a few points, I guess it is helpful to spell them out here:
-
I went with the plural form "policies", because I would say that it is a general mode of operation "enforcing policies or not", it's not so much about a single policy.
-
I have replaced "deployment" with "creation", because -- to the best of my knowledge -- the admission controller watches out for generic resource modification attempts, not only for deployments and updates. (@clickboo do you agree?).
-
I have replaced the "end"-construct with "which means" (maybe there is a better wording), but what I am trying to express here is that these are not two different things: "enforce policies" on the one hand and "preventing violating resource modification attempts" on the other hand. Instead, this is -- as far as I know -- exactly the definition of policy enforcement.
-
Removed the last part, because that is simply the negation of what has been said before.
There was a problem hiding this comment.
Maybe this is an English writing thing but "enforces a policy and rejects the deployment" is meant to explain what enforcing the policy is (not outline 2 different things that are happening). I will rewrite to try to be more clear.
modules/roxctl-sensor-generate.adoc
Outdated
|
|
||
| |`--admission-controller-enforce-on-creates` | ||
| |Dynamic enable for enforcing on object creation in the admission controller. The default value is `false`. | ||
| |This field is deprecated. |
There was a problem hiding this comment.
Maybe also mention that using it has no effect anymore. Users are expected to use the new high-level option --admission-controller-enforcement.
modules/roxctl-sensor-generate.adoc
Outdated
|
|
||
| |`--admission-controller-enforce-on-updates` | ||
| |Enable dynamic enforcement of object updates in the admission controller. The default value is `false`. | ||
| |This field is deprecated. |
There was a problem hiding this comment.
Same as for --admission-controller-enforce-on-creates.
modules/roxctl-sensor-generate.adoc
Outdated
| |This field is deprecated. | ||
|
|
||
| |`--admission-controller-fail-on-error` | ||
| | Valid values are `true` and `false`. The default value is `false`. Determines the action that should occur when an error or timeout happens in the admission controller. When set to `true`, the admission controller does not allow a request to reach the API server, or fails closed. When set to `false`, the admission controller allows the request to reach the server, or fails open. |
There was a problem hiding this comment.
-
nit, but I kind of dislike the "occur", because such an action is not something that simply "occurs" (at random occasions), it is instead the action, one could say "fallback logic", which happens exactly when the admission controller fails to respond to admission controller requests in time.
-
I think the statements about requests "reaching the API server" is not correct. The flow is instead: the users sends a request (e.g. to create/update some resource) to the API server. The kube API server then forwards the request to the admission controller and the admission controller is expected to send back a response indicating if the request should be allowed or not.
-
Maybe put the "fails closed" and "fails open" in parantheses as a description? It is saying the same thing using other/fewer words.
There was a problem hiding this comment.
- So what I am trying to say is that this field determines the action that occurs when it times out - it fails. I will rewrite but I do not think "fallback logic" is very user friendly to describe this.
- I guess this is based on my understanding from the existing docs (Note about API latency) that mention allowing/denying requests from reaching the API server but I will rewrite.
- Our style guide prohibits parentheses.
modules/roxctl-sensor-generate.adoc
Outdated
|
|
||
| |`--admission-controller-listen-on-creates` | ||
| |Configure the admission controller webhook to listen to deployment creation. The default value is `false`. | ||
| |This field is deprecated. |
There was a problem hiding this comment.
Also, the command (sensor generate) will behave by default as if this flag has been specified.
There was a problem hiding this comment.
Does the user need to know that, if there is nothing they can change (configure)?
There was a problem hiding this comment.
The reason why I have mentioned this the following: I imagine the user who -- until now -- has happily enabled "listen-on-creates" (through whatever install method). Now we are saying "This is deprecated". The user would, I guess, wonder: "If it is deprecated, then how am I expected to enable the feature behind this flag? How do I make sure that the admission controller webhook is also listening for, in this case, 'creates'?".
That's why I thought it would be valuable to say: "Not only is this field deprecated anymore -- you don't need it anymore, because the behaviour will be so that it is enabled by default, so don't worry about enabling it."
This line of thinking would not be limited to this specific flag.
Hope this makes sense.
modules/roxctl-sensor-generate.adoc
Outdated
|
|
||
| |`--admission-controller-listen-on-updates` | ||
| |Configure the admission controller webhook to listen to deployment updates. The default value is `false`. | ||
| |This field is deprecated. |
There was a problem hiding this comment.
Also, the command (sensor generate) will behave by default as if this flag has been specified.
modules/roxctl-sensor-generate.adoc
Outdated
|
|
||
| |`--admission-controller-scan-inline` | ||
| |Get scans inline when using the admission controller. The default value is `false`. | ||
| |This field is deprecated. |
There was a problem hiding this comment.
Also, the command (sensor generate) will behave by default as if this flag has been specified.
modules/roxctl-sensor-generate.adoc
Outdated
|
|
||
| |`--admission-controller-timeout int32` | ||
| |Set the timeout in seconds for the admission controller. The default value is `3`. | ||
| |This field is deprecated. |
| | Parameter | Description | ||
|
|
||
| |`admissionControl.enforce` | ||
| | This parameter determines if admission controller enforcement is enabled on a cluster. For a new installation, the default value is `true.` For an update to an existing installation, the system checks the previous values for the admission controller parameters. If one of the `admissionControl.dynamic.enforceOnCreates` or `admissionControl.dynamic.enforceOnUpdates` parameters was set to `true` before the update, the value of this parameter is set to `true` with the update. If one of these parameters was set to `false`, the value is set `false` with the update. |
There was a problem hiding this comment.
-
It's not true that the parameters
admissionControl.dynamic.enforceOnCreatesandadmissionControl.dynamic.enforceOnUpdatesare checked. Instead it isadmissionControl.listenOnCreatesandadmissionControl.listenOnEvents(the ones below which are also deprecated now). -
It is correct, on updates if any of them is true, the new
admissionControl.enforceoption will be enabled. But the last sentence is wrong, it should be "If all of these ... are false", not "If one of these ... are false". Or "If none of these ... is true".
| | Specify `true` to enable monitoring and enforcement for Kubernetes events, such as `port-forward` and `exec` events. | ||
| It is used to control access to resources through the Kubernetes API. | ||
| The default value is `true`. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
| | `admissionControl.listenOnCreates` | ||
| | Specify `true` to enable preventive policy enforcement for object creations. | ||
| The default value is `true`. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
I'm curious, where does this "If you edit this parameter, ... will not work as expected" stem from?
We can be precise here. On updates to 4.9, these settings will have effect in the sense that they will be used to derive a default value for the new enforce option. On fresh installations, these settings will have no effect.
There was a problem hiding this comment.
A comment from @clickboo in your original doc but I am totally open to rewording.
| | Specify `true` to enable preventive policy enforcement for object updates. | ||
| It will not have any effect unless `Listen On Creates` is set to `true` as well. | ||
| The default value is `true`. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
Same comment as for admissionControl.listenOnCreates above.
|
|
||
| * `ScanIfMissing` if the scan results for the image are missing. | ||
| * `DoNotScanInline` to skip scanning the image when processing the admission request. | ||
| | This field is deprecated. |
|
|
||
| The default value is `DoNotScanInline`. | ||
| |`admissionControl.failurePolicy` | ||
| | Determines the action that should occur when an error or timeout happens in the admission controller. You can configure if the admission controller should allow a request to reach the API server, or fail open; or to stop the request, or fail closed. The default value is `Ignore`, or fail open, and allows the request to reach the server. Setting the value to `closed` does not allow the request to reach the server. |
There was a problem hiding this comment.
Maybe I am being overly pedantic, but (regarding the first sentence) it's not only about errors or timeouts happening in the admission controller itself, but I look at it more from the cluster's perspective: If the kube API server fails to get a proper response from the admission controller in time, then this failure policy kicks in. So, this is basically a failure policy for the component which attempts to contact the admission controller, this is not a failure policy being applied by our admission controller. Cc'ing @clickboo.
|
|
||
| | `admissionControl.timeoutSeconds` | ||
| | Use this parameter to specify the maximum number of seconds {product-title-short} must wait for an admission review before marking it as fail open. If the admission webhook does not receive information that it is requesting before the end of the timeout period, it fails, but in fail open status, it still allows the operation to succeed. For example, the admission controller would allow a deployment to be created even if a scan had timed out and {product-title-short} could not determine if the deployment violated a policy. Beginning in release 4.5, Red{nbsp}Hat reduced the default timeout setting for the {product-title-short} admission controller webhooks from 20 seconds to 10 seconds, resulting in an effective timeout of 12 seconds within the `ValidatingWebhookConfiguration`. This change does not negatively affect {ocp} users because {ocp} caps the timeout at 13 seconds. | ||
| | The maximum number of seconds {product-title-short} must wait for an admission review before marking it as failed. This option is not configurable and is set to 10 seconds. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
Again, this field can be set by the user but it won't cause breakage, it will be ignored.
Also, basically following up on my above comment for the failurePolicy, this is the timeout to be applied by the component which calls the admission controller. It's not that "RHACS" waits this number of seconds, it's Kubernetes -- the kube API server.
The failurePolicy and this timeout belong together.
Thinking about it, WDYT about not mentioning the number to which this is set? One could consider this an implementation detail and if we want to fine-tune this in the future, we wouldn't need doc changes. Also, one could say that the value of this "10 seconds" mentioned here wouldn't be too helpful anyway, because this number is not what will end up in the ValidatingWebhookConfiguration resource anyway, because there will be some padding added to this "10 seconds", so it will be effectively a different number ending up in the resource on the cluster.
Cc'ing @clickboo (for making sure that I'm not writing nonsense).
| | The internal service-to-service TLS certificate key that Collector uses. | ||
|
|
||
| |`admissionControl.enforce` | ||
| | This parameter determines if admission controller enforcement is enabled when checking policies. For a new installation, the default value is `true.` For an update to an existing installation, the system checks the previous values for the admission controller parameters. If one of the `admissionControl.dynamic.enforceOnCreates` or `admissionControl.dynamic.enforceOnUpdates` parameters was set to `true` before the update, the value of this parameter is set to `true` with the update. If one of these parameters was set to `false`, the value is set to `false` with the update. |
There was a problem hiding this comment.
I would phrase this a bit different. It's not the policies which are being checked. Requests to the admission controller are being checked against policies.
Regarding the terminology, IIRC in other places we used "policy enforcement" (instead of "admission controller enforcement"). I can imagine that we are currently using these interchangeably. Just wondering if we should maybe settle on one instead. Cc'ing @lickboo.
nit, but I would not use "previous values" in this context, because when upgrading with Helm the user essentially has to provide the full configuration again, it's not like we are just pushing an "upgrade" button and some upgrade flow automatically reuses previously set values. (For completeness, I know there is helm upgrade --reuse-values, which is a short-cut but I don't know how common this is actually being used.) But no strong feelings on this one from me.
The last sentence is bogus -- same issue as for the operator updates: It shouldn't be "If one of these .." but "If all of these...", i.e. the negation of "If one of them is true".
| | This parameter determines if admission controller enforcement is enabled when checking policies. For a new installation, the default value is `true.` For an update to an existing installation, the system checks the previous values for the admission controller parameters. If one of the `admissionControl.dynamic.enforceOnCreates` or `admissionControl.dynamic.enforceOnUpdates` parameters was set to `true` before the update, the value of this parameter is set to `true` with the update. If one of these parameters was set to `false`, the value is set to `false` with the update. | ||
|
|
||
| |`admissionControl.failurePolicy` | ||
| | Determines the action that should occur when an error or timeout happens in the admission controller. You can configure if the admission controller should allow the request to reach the API server, or fail open; or to stop the request, or fail closed. The default value is `Ignore`, or fail open, and allows the request to reach the server. Setting the value to `closed` does not allow the request to reach the server. |
There was a problem hiding this comment.
What I have written in the operator context equally applies here.
|
|
||
| | `admissionControl.listenOnCreates` | ||
| | This setting controls whether Kubernetes is configured to contact {product-title} with `AdmissionReview` requests for workload creation events. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
Will be ignored (listening will be enabled uncondtionally).
| | This setting controls whether Kubernetes is configured to contact {product-title} with `AdmissionReview` requests for workload creation events. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. | ||
|
|
||
| | `admissionControl.listenOnUpdates` |
There was a problem hiding this comment.
Will be ignored (listening will be enabled uncondtionally).
| | `admissionControl.listenOnEvents` | ||
| | This setting controls whether the cluster is configured to contact {product-title} with `AdmissionReview` requests for Kubernetes `exec` and `portforward` events. | ||
| {product-title-short} does not support this feature on {ocp} 3.11. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
Will be ignored (listening will be enabled in most cases, only exception being openshift 3, which doesn't support it). In other words, it is enabled whenever the cluster supports it.
| | `admissionControl.dynamic.enforceOnCreates` | ||
| | This setting controls whether {product-title} evaluates policies; | ||
| if it is disabled, all AdmissionReview requests are automatically accepted. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
Setting it will only have an effect during upgrades from <4.9.
Ignored for fresh installs.
| | `admissionControl.dynamic.enforceOnUpdates` | ||
| | This setting controls the behavior of the admission control service. | ||
| You must specify `listenOnUpdates` as `true` for this to work. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
Setting it will only have an effect during upgrades from <4.9.
Ignored for fresh installs.
| | If you set this option to `true`, the admission control service requests an image scan before making an admission decision. | ||
| Since image scans take several seconds, enable this option only if you can ensure that all images used in your cluster are scanned before deployment (for example, by a CI integration during image build). | ||
| This option corresponds to the *Contact image scanners* option in the {product-title-short} portal. | ||
| | This parameter is deprecated. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
| ifndef::openshift[] | ||
| For Kubernetes clusters, image scanning and pulling that are done as part of the webhook execution might require you to configure a longer timeout value. | ||
| endif::openshift[] | ||
| | The maximum number of seconds {product-title-short} must wait for an admission review before marking it as failed. This option is not configurable and is set to 10 seconds. Do not edit this parameter. If you edit this parameter, policy evaluation will not work as expected. |
There was a problem hiding this comment.
What I have written for the operator also applies here.
kcarmichael08
force-pushed
the
ROX-30434-admission-controller-install
branch
from
1689d47 to
c1bfbd5
Compare
| |This field is deprecated and has no effect. Use the `--admission-controller-enforcement` option to configure enforcement. | ||
|
|
||
| |`--admission-controller-fail-on-error` | ||
| | Valid values are `true` and `false`. The default value is `false`. This parameter determines what happens if a request times out, for example, when the admission controller fails to respond to a create or update request from the API server. When set to `false`, the admission controller "fails open," meaning that the API server is allowed to accept the request. When set to `true`, the admission controller "fails closed", meaning that the API server rejects the requested action. |
There was a problem hiding this comment.
@mclasmeier I think this is better but please give feedback - thank you again for the detailed flow and explanation that you provided!
|
@kcarmichael08: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| |This field is deprecated and has no effect. Use the `--admission-controller-enforcement` option to configure enforcement. | ||
|
|
||
| |`--admission-controller-fail-on-error` | ||
| | Valid values are `true` and `false`. The default value is `false`. This parameter determines what happens if a request times out, for example, when the admission controller fails to respond to a create or update request from the API server. When set to `false`, the admission controller "fails open," meaning that the API server is allowed to accept the request. When set to `true`, the admission controller "fails closed", meaning that the API server rejects the requested action. |
There was a problem hiding this comment.
Just a comma fix:
| | Valid values are `true` and `false`. The default value is `false`. This parameter determines what happens if a request times out, for example, when the admission controller fails to respond to a create or update request from the API server. When set to `false`, the admission controller "fails open," meaning that the API server is allowed to accept the request. When set to `true`, the admission controller "fails closed", meaning that the API server rejects the requested action. | |
| | Valid values are `true` and `false`. The default value is `false`. This parameter determines what happens if a request times out, for example, when the admission controller fails to respond to a create or update request from the API server. When set to `false`, the admission controller "fails open", meaning that the API server is allowed to accept the request. When set to `true`, the admission controller "fails closed", meaning that the API server rejects the requested action. |
Version(s): 4.9+
Issue
Links to docs previews:
Parameters
GUI instructions
QE review:
Additional information:
Related PRs:
#100022 - general doc changes for admission controller changes