feat(helm): support admission control failurePolicy configuration#13838
feat(helm): support admission control failurePolicy configuration#13838flowdopip wants to merge 1 commit intostackrox:masterstackrox/stackrox:masterfrom
Conversation
|
Hi @flowdopip. Thanks for your PR. I'm waiting for a stackrox member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Anything I can do to speed up this "feature"? I need this asap for an internal compliance audit. |
vladbologa
left a comment
There was a problem hiding this comment.
Hi @flowdopip, thanks for contributing to Stackrox! The change looks good to me, but I see some style check failures.
I would also like that another colleague reviews this as well.
|
/ok-to-test |
|
Thanks @vladbologa. image/templates/helm/stackrox-secured-cluster/values-public.yaml.example:312: trailing whitespace. |
| - istio-system | ||
| {{- end }} | ||
| failurePolicy: Ignore | ||
| failurePolicy: {{ ._rox.admissionControl.failurePolicy | default "Ignore" }} |
There was a problem hiding this comment.
We are currently aiming for consolidating our default settings for configurable settings in one place. Could you add the default here instead of having the defaulting here?
Same below.
There was a problem hiding this comment.
done! Please review!
| listenOnCreates: true | ||
| listenOnEvents: false | ||
| expect: | | ||
| .validatingwebhookconfigurations[].webhooks[].failurePolicy == "Ignore" |
There was a problem hiding this comment.
This test doesn't work in it's current form. Here's a patch:
--- a/pkg/helm/charts/tests/securedclusterservices/testdata/helmtest/admission-control.test.yaml
+++ b/pkg/helm/charts/tests/securedclusterservices/testdata/helmtest/admission-control.test.yaml
@@ -170,12 +170,12 @@ tests:
listenOnCreates: true
listenOnEvents: false
expect: |
- .validatingwebhookconfigurations[].webhooks[].failurePolicy == "Ignore"
+ .validatingwebhookconfigurations[].webhooks[].failurePolicy | assertThat(. == "Ignore")
- name: "override failurePolicy to fail"
values:
admissionControl:
listenOnCreates: true
listenOnEvents: false
- failurePolicy: "fail"
+ failurePolicy: "Fail"
expect: |
- .validatingwebhookconfigurations[].webhooks[].failurePolicy == "Fail"
\ No newline at end of file
+ .validatingwebhookconfigurations[].webhooks[].failurePolicy | assertThat(. == "Fail")
You can run the tests as follows:
❯ pwd
/Users/mclasmeier/go/src/github.com/stackrox/stackrox/pkg/helm/charts/tests/securedclusterservices
❯ go test -v ./ -run TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy
=== RUN TestWithHelmtest
=== RUN TestWithHelmtest/testdata/helmtest
=== PAUSE TestWithHelmtest/testdata/helmtest
=== CONT TestWithHelmtest/testdata/helmtest
=== RUN TestWithHelmtest/testdata/helmtest/admission-control.test.yaml
=== PAUSE TestWithHelmtest/testdata/helmtest/admission-control.test.yaml
=== CONT TestWithHelmtest/testdata/helmtest/admission-control.test.yaml
=== RUN TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy
=== PAUSE TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy
=== CONT TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy
=== RUN TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/default_failure_policy_ignore
=== PAUSE TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/default_failure_policy_ignore
=== RUN TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/override_failurePolicy_to_fail
=== PAUSE TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/override_failurePolicy_to_fail
=== CONT TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/default_failure_policy_ignore
=== CONT TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/override_failurePolicy_to_fail
--- PASS: TestWithHelmtest (0.02s)
--- PASS: TestWithHelmtest/testdata/helmtest (0.00s)
--- PASS: TestWithHelmtest/testdata/helmtest/admission-control.test.yaml (0.00s)
--- PASS: TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy (0.00s)
--- PASS: TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/override_failurePolicy_to_fail (0.20s)
--- PASS: TestWithHelmtest/testdata/helmtest/admission-control.test.yaml/Admission_Controller_Failure_Policy/default_failure_policy_ignore (0.20s)
PASS
ok github.com/stackrox/rox/pkg/helm/charts/tests/securedclusterservices 0.981s
Besides the assertion syntax, there was a typo (fail vs. Fail) and a missing newline at the end of the fail, which would probably be criticized by our automatic style checks.
There was a problem hiding this comment.
Thanks for the explanation!
Please review the update
mclasmeier
left a comment
There was a problem hiding this comment.
Thank you for the contribution!
|
/retest |
|
@flowdopip: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
@flowdopip @mclasmeier I have opened another PR using @flowdopip's commit, so that CI can run properly. We can merge that one instead. |
|
@vladbologa why you need to open a new PR for the CI to work? |
Looking into it, but for now I thought it would be faster if I just open another PR. It's merged now btw, so nothing left for you to do. It'll be available in the 4.7 release. |
Description
Add support to define which failurePolicy the admission controller can apply.
FailurePolicy defines how unrecognized errors from the admission endpoint are handled - allowed values are Ignore or Fail. Defaults to Fail.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Added unit test for current config and custom configuration.