OCP4: Filter out new 4.11 SCCs that allow capabilites in addition to the privileged SCC #8996
Conversation
|
Start a new ephemeral environment with changes proposed in this pull request: |
|
/retest |
|
I brought up the failures to pull the must gather image from build05 on slack on forum-testplatform, someone is looking into it. |
| @@ -44,13 +44,13 @@ ocil: |- | |||
|
|
|||
| warnings: | |||
| - general: |- | |||
| {{{ openshift_filtered_cluster_setting({'/apis/security.openshift.io/v1/securitycontextconstraints': '[.items[] | select(.metadata.name != "privileged")] | map(.allowedCapabilities == null)'}) | indent(4) }}} | |||
| {{{ openshift_filtered_cluster_setting({'/apis/security.openshift.io/v1/securitycontextconstraints': '[.items[] | select(.metadata.name != "privileged") | select(.metadata.name | test("^.*-v2$"; "") | not)] | map(.allowedCapabilities == null)'}) | indent(4) }}} | |||
There was a problem hiding this comment.
Wondering if we exclude all SCCs ending with v2, will it give false-positive result
There was a problem hiding this comment.
right, that was one of the things I was considering. And you are right that v2 is too generic to use for a filter (what if someone has myapp-v2 SCC?).
We can filter out the names of the SCCs ourselves, but a chain of selects would be ugly. I'll try to come up with a regex instead that I can use in test.
Is there anything nicer we could use?
There was a problem hiding this comment.
If we chained all the SCC together using selects, we'd need to update that list each time it changed, right?
There was a problem hiding this comment.
yes. I'm going to research if we can filter our "system" SCCs.
There was a problem hiding this comment.
The short answer is "no". The only suggested filter (by Standa on Slack) was to look at some of the openshift.io annotations. I'm unsure if those are available in older releases like 4.6, going to check this.
As a fallback, I would rather use a hardcoded list, because it's a "subtraction" from the available SCC, so if some new SCCs appear in a new release then the test will fail..
There was a problem hiding this comment.
Unfortunately our SCC annotations are not super consistent, most system SCCs have include.release.openshift.io/self-managed-high-availability but not all (e.g. not hostmounter). Most have release.openshift.io/create-only but not all.
So in the end, I think we could add a list.
@Vincent056 do you know if we can use a variable in the JQ filter? If yes, then it would be nice to make the list of SCCs that are OK to allow capabilities as a variable that would default to those that we ship and the customers could add their own.
There was a problem hiding this comment.
sorry for that late reply, yes, I think we can use the variable in the JQ filter, you just need to create a variable and use it as {{.var_scc_exclude_name}}
There was a problem hiding this comment.
thank you, let me work on that
There was a problem hiding this comment.
Done. @Vincent056 I have one more question, do we need to document the variable somehow?
There was a problem hiding this comment.
actually, hold the review, I'm going to add the docs either way
jhrozek
changed the title
|
/retest |
1 similar comment
|
/retest |
|
/test e2e-aws |
|
@jhrozek: The specified target(s) for
Use
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 kubernetes/test-infra repository. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
I think the two remaining errors are expected with this PR. |
…list OCP 4.11 adds several new SCCs whose allowedCapabilities are set. All of their names end with -v2, so let's filter them out as well from the list of SCCs we check.
|
Code Climate has analyzed commit 267bcd2 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 42.7% (0.0% change). View more on Code Climate. |
|
/retest |
|
I think the changes look good. The Automatus CI is supposed to fail because we're adding a new check that doesn't have a test, but this will be tested in e2e CI, is that correct? |
Correct, the rule has an expected PASS manifest |
|
/lgtm |
Description:
OCP 4.11 adds several new SCCs whose allowedCapabilities are set. All of
their names end with -v2, so let's filter them out as well from the list
of SCCs we check.
Rationale:
We need to support OCP 4.11
For the failure, see https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_release/29623/rehearse-29623-pull-ci-ComplianceAsCode-content-master-e2e-aws-ocp4-moderate/1538803521816629248/build-log.txt