CMP-3765: Add customRule enforce-trusted-tls-registries#845
Conversation
|
Hi @Anna-Koudelkova. Thanks for your PR. I'm waiting for a github.com 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. |
Anna-Koudelkova
changed the title
|
@Anna-Koudelkova: This pull request references CMP-3765 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn 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. |
...g/samples/custom-rules/openshift-virtualization/kubevirt-enforce-trusted-tls-registries.yaml
Show resolved
Hide resolved
| checkType: Platform | ||
| scannerType: CEL | ||
| inputs: | ||
| - name: hcoList |
There was a problem hiding this comment.
Since we're dealing with a singular resource we can use hco.
| h.metadata.name == 'kubevirt-hyperconverged' && | ||
| h.metadata.namespace == 'openshift-cnv' | ||
| ).all(h, | ||
| h.spec.storageImport.insecureRegistries == null |
There was a problem hiding this comment.
Then we should be able to simplify this to:
expression: |-
!has(hco.spec.storageImport) ||
hco.spec.storageImport.insecureRegistries.size() == 0
rhmdnd
left a comment
rhmdnd
left a comment
There was a problem hiding this comment.
Nice work 🎉
I was able to get this working on a live cluster with just a few changes:
diff --git a/config/samples/custom-rules/openshift-virtualization/kubevirt-enforce-trusted-tls-registries.yaml b/config/samples/custom-rules/openshift-virtualization/kubevirt-enforce-trusted-tls-registries.yaml
index 550bbbcc4..6c9eed345 100644
--- a/config/samples/custom-rules/openshift-virtualization/kubevirt-enforce-trusted-tls-registries.yaml
+++ b/config/samples/custom-rules/openshift-virtualization/kubevirt-enforce-trusted-tls-registries.yaml
@@ -1,11 +1,11 @@
apiVersion: compliance.openshift.io/v1alpha1
kind: CustomRule
metadata:
- name: kubevirt-enforce-trusted-tls
+ name: kubevirt-enforce-trusted-tls-registries
namespace: openshift-compliance
spec:
title: "Only trusted registries using TLS can be used"
- id: kubevirt_enforce_trusted_tls
+ id: kubevirt_enforce_trusted_tls_registries
description: |-
By only pulling container images from trusted registries, organizations
can reduce the risk of introducing unknown vulnerabilities or malicious
@@ -18,18 +18,12 @@ spec:
checkType: Platform
scannerType: CEL
inputs:
- - name: hcoList
+ - name: hco
kubernetesInputSpec:
apiVersion: hco.kubevirt.io/v1beta1
resource: hyperconvergeds
+ resourceName: kubevirt-hyperconverged
+ resourceNamespace: openshift-cnv
expression: |
- hcoList.items.filter(h,
- h.metadata.name == 'kubevirt-hyperconverged' &&
- h.metadata.namespace == 'openshift-cnv'
- ).size() == 1 &&
- hcoList.items.filter(h,
- h.metadata.name == 'kubevirt-hyperconverged' &&
- h.metadata.namespace == 'openshift-cnv'
- ).all(h,
- h.spec.storageImport.insecureRegistries == null
- )
+ !has(hco.spec.storageImport) ||
+ hco.spec.storageImport.insecureRegistries.size() == 0
|
🤖 To deploy this PR, run the following command: |
Anna-Koudelkova
force-pushed
the
CMP-3765-add-rule-enforce-trusted-TLS
branch
from
28c78c9 to
53d515b
Compare
|
🤖 To deploy this PR, run the following command: |
|
Thanks for the reviews and explanations! I have forgotten this exists for a while - but now it should be rebased and modified based on your suggestions. |
Anna-Koudelkova
force-pushed
the
CMP-3765-add-rule-enforce-trusted-TLS
branch
from
53d515b to
aa3b73e
Compare
|
🤖 To deploy this PR, run the following command: |
|
@Anna-Koudelkova: 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. |
rhmdnd
left a comment
rhmdnd
left a comment
There was a problem hiding this comment.
/lgtm
/approve
Only one minor recommendation if we absolutely have to respin the patch.
| resourceNamespace: openshift-cnv | ||
| expression: |- | ||
| !has(hco.spec.storageImport) || | ||
| hco.spec.storageImport.insecureRegistries.size() == 0 |
There was a problem hiding this comment.
Another potential protection we could take would be to guard against insecureRegistries not being present.
expression: |-
!has(hco.spec.storageImport) ||
!has(hco.spec.storageImport.insecureRegistries) ||
hco.spec.storageImport.insecureRegistries.size() == 0
That would mean if someone has storageImports specified, but not insecureRegistries, then the check behaves the same as if they didn't have storageImports configured at all.
We can catch that in a follow up unless there is a reason to respin the patch.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Anna-Koudelkova, rhmdnd The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Merging since this isn't baked into a profile yet, doesn't have any outstanding issues, and is completely unrelated to CI failures. |
4 participants
Sketch of a rule for enforcing using only registries using TLS.
This is more of a fun and play than actual help, feel free to close this immediately - thanks a lot for including me in the meeting!