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

fix: skip rules without operation in resource webhook creation #10146

Merged
merged 4 commits into from
Apr 30, 2024

Conversation

snorwin
Copy link
Contributor

@snorwin snorwin commented Apr 30, 2024

Explanation

The error MutatingWebhookConfiguration: webhooks[0].rules[0].operations: Required value occurs when using mutating and generating rules in the same policy.

Related issue

Fixes #10143 .

Milestone of this PR

/milestone 1.12.1

What type of PR is this

/kind bug

Proposed Changes

Add a check in order to skip rules without any mapped operation.

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
  • CLI support should be added and my PR doesn't contain that functionality.

Copy link

welcome bot commented Apr 30, 2024

Thanks for opening your first Pull Request here! Please check out our Contributing guidelines and confirm that you Signed off.

@realshuting
Copy link
Member

Hi @snorwin - thanks for the fix.

Can you also attach the configured mutatingwebhookconfigurations and validatingwebhookconfigurations for the policy with generate and mutate rules?

@realshuting
Copy link
Member

/cherry-pick release-1.12

@snorwin
Copy link
Contributor Author

snorwin commented Apr 30, 2024

Hi @snorwin - thanks for the fix.

Can you also attach the configured mutatingwebhookconfigurations and validatingwebhookconfigurations for the policy with generate and mutate rules?

The policy:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: example
spec:
  generateExisting: true
  rules:
    - name: k-kafka-address
      match:
        any:
          - resources:
              kinds:
                - Namespace
      exclude:
        any:
          - resources:
              namespaces:
                - kube-system
                - default
                - kube-public
                - kyverno
      generate:
        synchronize: true
        apiVersion: v1
        kind: ConfigMap
        name: zk-kafka-address
        # generate the resource in the new namespace
        namespace: "{{request.object.metadata.name}}"
        data:
          kind: ConfigMap
          metadata:
            labels:
              somekey: somevalue
          data:
            ZK_ADDRESS: "192.168.10.10:2181,192.168.10.11:2181,192.168.10.12:2181"
            KAFKA_ADDRESS: "192.168.10.13:9092,192.168.10.14:9092,192.168.10.15:9092"
    - name: set-image-pull-policy
      match:
        any:
          - resources:
              kinds:
                - Pod
      mutate:
        patchStrategicMerge:
          spec:
            containers:
              # match images which end with :latest
              - (image): "*:latest"
                # set the imagePullPolicy to "IfNotPresent"
                imagePullPolicy: "IfNotPresent"

generated resource webhook configurations:

apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
  annotations:
    admissions.enforcer/disabled: "true"
  labels:
    webhook.kyverno.io/managed-by: kyverno
  name: kyverno-resource-mutating-webhook-cfg
webhooks:
- admissionReviewVersions:
  - v1
  clientConfig:
    service:
      name: kyverno-operator-deploy-svc
      namespace: kyverno
      path: /mutate/fail
      port: 443
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: mutate.kyverno.svc-fail
  namespaceSelector:
    matchExpressions:
    - key: kubernetes.io/metadata.name
      operator: NotIn
      values:
      - kube-system
    - key: kubernetes.io/metadata.name
      operator: NotIn
      values:
      - kyverno
  objectSelector: {}
  reinvocationPolicy: IfNeeded
  rules:
  - apiGroups:
    - ""
    apiVersions:
    - v1
    operations:
    - CREATE
    - UPDATE
    resources:
    - namespaces
    scope: '*'
  - apiGroups:
    - ""
    apiVersions:
    - v1
    operations:
    - CREATE
    - UPDATE
    resources:
    - configmaps
    - pods
    - pods/ephemeralcontainers
    scope: Namespaced
  sideEffects: NoneOnDryRun
  timeoutSeconds: 10
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations:
    admissions.enforcer/disabled: "true"
  labels:
    webhook.kyverno.io/managed-by: kyverno
  name: kyverno-resource-validating-webhook-cfg
webhooks:
  - admissionReviewVersions:
      - v1
    clientConfig:
      service:
        name: kyverno-operator-deploy-svc
        namespace: kyverno
        path: /validate/fail
        port: 443
    failurePolicy: Fail
    matchPolicy: Equivalent
    name: validate.kyverno.svc-fail
    namespaceSelector:
      matchExpressions:
        - key: kubernetes.io/metadata.name
          operator: NotIn
          values:
            - kube-system
        - key: kubernetes.io/metadata.name
          operator: NotIn
          values:
            - kyverno
    objectSelector: {}
    rules:
      - apiGroups:
          - ""
        apiVersions:
          - v1
        operations:
          - CREATE
          - UPDATE
          - DELETE
          - CONNECT
        resources:
          - configmaps
        scope: Namespaced
      - apiGroups:
          - ""
        apiVersions:
          - v1
        operations:
          - CREATE
          - UPDATE
          - DELETE
          - CONNECT
        resources:
          - namespaces
        scope: '*'
    sideEffects: NoneOnDryRun
    timeoutSeconds: 10

@realshuting
Copy link
Member

realshuting commented Apr 30, 2024

Why there's an extra rule for namespaces in mutating webhook?

  rules:
  - apiGroups:
    - ""
    apiVersions:
    - v1
    operations:
    - CREATE
    - UPDATE
    resources:
    - namespaces
    scope: '*'

@snorwin
Copy link
Contributor Author

snorwin commented Apr 30, 2024

Why there's an extra rule for namespaces in mutating webhook?

  rules:
  - apiGroups:
    - ""
    apiVersions:
    - v1
    operations:
    - CREATE
    - UPDATE
    resources:
    - namespaces
    scope: '*'

defaultOpn in buildRulesWithOperations (https://github.com/kyverno/kyverno/blob/main/pkg/controllers/webhook/utils.go#L74) are CREATE and UPDATE. We could move the skip to this function if that makes sense?

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
@snorwin snorwin force-pushed the fix-invalid-webhook-configuration branch from 6d50b08 to 6bc7dda Compare April 30, 2024 14:41
Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
realshuting
realshuting previously approved these changes Apr 30, 2024
@realshuting
Copy link
Member

The linter check fails, @snorwin - can you fix it https://github.com/kyverno/kyverno/actions/runs/8897683625/job/24433129022?pr=10146?

Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.15%. Comparing base (e66a550) to head (4321a09).

❗ Current head 4321a09 differs from pull request most recent head 1143762. Consider uploading reports for the commit 1143762 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10146      +/-   ##
==========================================
+ Coverage   10.13%   10.15%   +0.02%     
==========================================
  Files        1030     1030              
  Lines       91776    91779       +3     
==========================================
+ Hits         9299     9321      +22     
+ Misses      81461    81439      -22     
- Partials     1016     1019       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
@snorwin snorwin force-pushed the fix-invalid-webhook-configuration branch from 1e34e4b to 1143762 Compare April 30, 2024 16:23
@realshuting realshuting enabled auto-merge (squash) April 30, 2024 16:31
@realshuting realshuting merged commit 5d50022 into kyverno:main Apr 30, 2024
244 of 248 checks passed
Copy link

welcome bot commented Apr 30, 2024

Congratulations! 🎉

Great job merging your first Pull Request here! How awesome! If you are new to this project, feel free to join our Slack community
200w

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Apr 30, 2024
* fix: skip rules without operation in resource webhook creation

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>

* test: add unit test for buildRulesWithOperations

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>

* fix liniting issues

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>

---------

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
Co-authored-by: shuting <shuting@nirmata.com>
@realshuting realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label May 1, 2024
realshuting added a commit that referenced this pull request May 1, 2024
… (#10151)

* fix: skip rules without operation in resource webhook creation



* test: add unit test for buildRulesWithOperations



* fix liniting issues



---------

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
Co-authored-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
Co-authored-by: shuting <shuting@nirmata.com>
anushkamittal2001 pushed a commit to nirmata/kyverno that referenced this pull request May 24, 2024
…no#10146) (kyverno#10151)

* fix: skip rules without operation in resource webhook creation



* test: add unit test for buildRulesWithOperations



* fix liniting issues



---------

Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
Co-authored-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
Co-authored-by: shuting <shuting@nirmata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-completed The PR was cherry-picked (or merged) to required release branches cherry-pick-required milestone 1.12.1 milestone 1.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Error creating MutatingWebhookConfiguration: webhooks[0].rules[0].operations: Required value (v1.12.0)
3 participants