Skip to content
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

Add initContainers and ephemeralContainers to Require Images Use Checksums #1066

Merged
merged 9 commits into from
Aug 9, 2024

Conversation

nsagark
Copy link
Contributor

@nsagark nsagark commented Jul 9, 2024

Related Issue(s)

Description

Checklist

  • I have read the policy contribution guidelines.
  • I have added test manifests and resources covering both positive and negative tests that prove this policy works as intended.
  • I have added the artifacthub-pkg.yml file and have verified it is complete and correct.

Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to add name as a conditional since this is a mandatory field. You also don't need to deviate from the pattern shown since digests can be specified using algorithms other than sha256. Please just add in ephemeral and initContainers following the same pattern that has already been established.

Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if you're adding in checks for additional containers, you should cover for them in the test cases which doesn't look like has been done.

Signed-off-by: nsagark <sagar@nirmata.com>
Signed-off-by: nsagark <sagar@nirmata.com>
@nsagark
Copy link
Contributor Author

nsagark commented Jul 18, 2024

@chipzoller I have updated the policy. Also, I have updated the chainsaw tests to include init containers. I could not include ephermeral container as my chainsaw test was failing with below error.. Please review and let me know.

Pod "goodpod04" is invalid: spec.ephemeralContainers: Forbidden: cannot be set on create | 22:20:58 | require-image-checksum | step-02 | APPLY | ERROR | v1/Pod @ chainsaw-mint-sponge/goodpod04 === ERROR Pod "goodpod04" is invalid: spec.ephemeralContainers: Forbidden: cannot be set on create

@nsagark nsagark requested a review from chipzoller July 18, 2024 03:17
@chipzoller
Copy link
Contributor

You cannot add ephemeral containers on a create request. These can only be added on an update. Either use kubectl debug in a step or apply an updated manifest which contains them.

Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ephemeralContainers cannot be created with an initial Pod's CREATE request. They may only be added to existing Pods (therefore an UPDATE request). In order to test ephemeralContainers, you can either use kubectl debug or an update/patch to the Pod if you need to supply additional fields.

@chipzoller chipzoller changed the title Updating require-image-checksum for init and ephemeral containersCheck image digest Add initContainers and ephemeralContainers to Require Images Use Checksums Aug 2, 2024
@nsagark
Copy link
Contributor Author

nsagark commented Aug 2, 2024

Hi @chipzoller Do we have any policy examples where we have chainsaw test case written for ephemeral containers. I tried something like below but getting an error.

`apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
  creationTimestamp: null
  name: require-image-checksum
spec:
  steps:
  - name: step-01
    try:
    - apply:
        file: ../require-image-checksum.yaml
    - patch:
        resource:
          apiVersion: kyverno.io/v1
          kind: ClusterPolicy
          metadata:
            name: require-image-checksum
          spec:
            validationFailureAction: Enforce
    - assert:
        file: chainsaw-step-01-assert-1.yaml
  - name: step-02
    try:
    - apply:
        file: pod-good.yaml
    - apply:
        expect:
        - check:
            ($error != null): true
        file: pod-bad.yaml
    - apply:
        file: podcontroller-good.yaml
    - apply:
        expect:
        - check:
            ($error != null): true
        file: podcontroller-bad.yaml
  - name: step-03
    try:
    - apply:
        file: pod-good.yaml
    - patch:
        resource:
          apiVersion: v1
          kind: Pod
          metadata:
            name: goodpod01
          spec:
            containers:
            - name: busybox
              image: busybox@sha256:67a8ef886e2ca4055f00e7cd13aedb9b24148c1451a6832d16fcc997a157eedc
            - name: busybox02
              image: busybox@sha256:67a8ef886e2ca4055f00e7cd13aedb9b24148c1451a6832d16fcc997a157eedc
            ephemeralContainers:
            - name: debug-container
              image: busybox@sha256:67a8ef886e2ca4055f00e7cd13aedb9b24148c1451a6832d16fcc997a157eedc
              command: ['sh', '-c', 'sleep 1000']
---     

`

| 12:52:43 | require-image-checksum | step-03 | PATCH | ERROR | v1/Pod @ chainsaw-vital-doberman/goodpod01 === ERROR Pod "goodpod01" is invalid: spec: Forbidden: pod updates may not change fields other thanspec.containers[].image,spec.initContainers[].image,spec.activeDeadlineSeconds,spec.tolerations (only additions to existing tolerations),spec.terminationGracePeriodSeconds (allow it to be set to 1 if it was previously negative) core.PodSpec{ Volumes: {{Name: "kube-api-access-vhvdt", VolumeSource: {Projected: &{Sources: {{ServiceAccountToken: &{ExpirationSeconds: 3607, Path: "token"}}, {ConfigMap: &{LocalObjectReference: {Name: "kube-root-ca.crt"}, Items: {{Key: "ca.crt", Path: "ca.crt"}}}}, {DownwardAPI: &{Items: {{Path: "namespace", FieldRef: &{APIVersion: "v1", FieldPath: "metadata.namespace"}}}}}}, DefaultMode: &420}}}}, InitContainers: nil, Containers: []core.Container{

@chipzoller
Copy link
Contributor

Please use code blocks to show code/YAML. I can't read that.

@nsagark
Copy link
Contributor Author

nsagark commented Aug 2, 2024

I have edited my response. Please take a look.

@chipzoller
Copy link
Contributor

You can't patch with containers[] being as part of the partial resource. Try with only ephemeralContainers.

@nsagark
Copy link
Contributor Author

nsagark commented Aug 2, 2024

Tried with just ephemeral containers. But still seeing an error.

apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
  creationTimestamp: null
  name: require-image-checksum
spec:
  steps:
  - name: step-01
    try:
    - apply:
        file: ../require-image-checksum.yaml
    - patch:
        resource:
          apiVersion: kyverno.io/v1
          kind: ClusterPolicy
          metadata:
            name: require-image-checksum
          spec:
            validationFailureAction: Enforce
    - assert:
        file: chainsaw-step-01-assert-1.yaml
  - name: step-02
    try:
    - apply:
        file: pod-good.yaml
    - apply:
        expect:
        - check:
            ($error != null): true
        file: pod-bad.yaml
    - apply:
        file: podcontroller-good.yaml
    - apply:
        expect:
        - check:
            ($error != null): true
        file: podcontroller-bad.yaml
  - name: step-03
    try:
    - apply:
        file: pod-good.yaml
    - patch:
        resource:
          apiVersion: v1
          kind: Pod
          metadata:
            name: goodpod01
          spec:
            ephemeralContainers:
            - name: debug-container
              image: busybox@sha256:67a8ef886e2ca4055f00e7cd13aedb9b24148c1451a6832d16fcc997a157eedc
              command: ['sh', '-c', 'sleep 1000']
---     

Here is the error:

| 13:11:57 | require-image-checksum | step-03  | PATCH     | ERROR | v1/Pod @ chainsaw-humorous-quagga/goodpod01
        === ERROR
        Pod "goodpod01" is invalid: spec: Forbidden: pod updates may not change fields other than `spec.containers[*].image`,`spec.initContainers[*].image`,`spec.activeDeadlineSeconds`,`spec.tolerations` (only additions to existing tolerations),`spec.terminationGracePeriodSeconds` (allow it to be set to 1 if it was previously negative)

@chipzoller
Copy link
Contributor

Ok, the issue is that ephemeral containers is a subresource, so you can't patch the Pod directly. My suggestion would be to do a kubectl debug as a Chainsaw step.

Signed-off-by: nsagark <sagar@nirmata.com>
@nsagark
Copy link
Contributor Author

nsagark commented Aug 7, 2024

Hi @chipzoller I have updated the chainsaw-test.yaml to include tests for ephemeral containers as well. Please take a look.

Signed-off-by: nsagark <sagar@nirmata.com>
Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there.

Signed-off-by: nsagark <sagar@nirmata.com>
@nsagark
Copy link
Contributor Author

nsagark commented Aug 9, 2024

Hi @chipzoller I have removed the ephemeral containers from the resource.yaml. Please take a look.

@chipzoller chipzoller merged commit 1208ed3 into kyverno:main Aug 9, 2024
280 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants