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: avoid creating duplicate urs for background policies #10431

Merged
merged 23 commits into from
Jun 12, 2024

Conversation

realshuting
Copy link
Member

@realshuting realshuting commented Jun 11, 2024

Explanation

This PR fixes duplicate urs creation for the same rule. The issue happens when a background policy has generateExisting or mutateExistingOnPolicyUpdate enabled.

When such a policy is created:

  • before this change:

27 updaterequests are created for 3 matching triggers, 3 rules

  • after this change:

9 updaterequests are created for 3 matching triggers, 3 rules

policy:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: zk-kafka-address
spec:
  generateExisting: true
  rules:
  - exclude:
      any:
      - resources:
          namespaces:
          - kube-system
          - default
          - kube-public
          - kyverno
          - local-path-storage
    generate:
      apiVersion: v1
      data:
        data:
          KAFKA_ADDRESS: 192.168.10.13:9092,192.168.10.14:9092,192.168.10.15:9092
          ZK_ADDRESS: 192.168.10.10:2181,192.168.10.11:2181,192.168.10.12:2181
        kind: ConfigMap
        metadata:
          labels:
            somekey: somevalue
      kind: ConfigMap
      name: zk-kafka-address
      namespace: '{{request.object.metadata.name}}'
      synchronize: true
    match:
      any:
      - resources:
          kinds:
          - Namespace
    name: k-kafka-address
  - exclude:
      any:
      - resources:
          namespaces:
          - kube-system
          - default
          - kube-public
          - kyverno
          - local-path-storage
    generate:
      apiVersion: v1
      data:
        data:
          KAFKA_ADDRESS: 192.168.10.13:9092,192.168.10.14:9092,192.168.10.15:9092
          ZK_ADDRESS: 192.168.10.10:2181,192.168.10.11:2181,192.168.10.12:2181
        kind: ConfigMap
        metadata:
          labels:
            somekey: somevalue
      kind: ConfigMap
      name: zk-kafka-address-2
      namespace: '{{request.object.metadata.name}}'
      synchronize: true
    match:
      any:
      - resources:
          kinds:
          - Namespace
    name: k-kafka-address-2
  - exclude:
      any:
      - resources:
          namespaces:
          - kube-system
          - default
          - kube-public
          - kyverno
          - local-path-storage
    generate:
      apiVersion: v1
      data:
        data:
          KAFKA_ADDRESS: 192.168.10.13:9092,192.168.10.14:9092,192.168.10.15:9092
          ZK_ADDRESS: 192.168.10.10:2181,192.168.10.11:2181,192.168.10.12:2181
        kind: ConfigMap
        metadata:
          labels:
            somekey: somevalue
      kind: ConfigMap
      name: zk-kafka-address-3
      namespace: '{{request.object.metadata.name}}'
      synchronize: true
    match:
      any:
      - resources:
          kinds:
          - Namespace
    name: k-kafka-address-3

Related issue

#9633

Milestone of this PR

Documentation (required for features)

My PR contains new or altered behavior to Kyverno.

What type of PR is this

Proposed Changes

Proof Manifests

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.

Further Comments

Signed-off-by: ShutingZhao <[email protected]>
Signed-off-by: ShutingZhao <[email protected]>
Signed-off-by: ShutingZhao <[email protected]>
Signed-off-by: ShutingZhao <[email protected]>
Signed-off-by: ShutingZhao <[email protected]>
Signed-off-by: ShutingZhao <[email protected]>
Signed-off-by: ShutingZhao <[email protected]>
@realshuting realshuting changed the title fix: avoid duplicate urs created for background policies fix: avoid creating duplicate urs for background policies Jun 11, 2024
@realshuting realshuting added this to the Kyverno Release 1.12.4 milestone Jun 11, 2024
@realshuting
Copy link
Member Author

/cherry-pick release-1.12

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 38 lines in your changes missing coverage. Please review.

Project coverage is 10.02%. Comparing base (6813fc0) to head (41ffc3f).
Report is 1 commits behind head on main.

Current head 41ffc3f differs from pull request most recent head f0800af

Please upload reports for the commit f0800af to get more accurate results.

Files Patch % Lines
pkg/policy/mutate.go 0.00% 22 Missing ⚠️
pkg/policy/generate.go 0.00% 11 Missing ⚠️
pkg/policy/policy_controller.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10431      +/-   ##
==========================================
- Coverage   10.03%   10.02%   -0.01%     
==========================================
  Files        1041     1041              
  Lines       93857    93864       +7     
==========================================
  Hits         9414     9414              
- Misses      83407    83414       +7     
  Partials     1036     1036              

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

@realshuting realshuting marked this pull request as ready for review June 12, 2024 10:14
@realshuting realshuting requested a review from treydock as a code owner June 12, 2024 10:14
@realshuting realshuting requested a review from eddycharly as a code owner June 12, 2024 10:14
@realshuting realshuting enabled auto-merge (squash) June 12, 2024 10:14
auto-merge was automatically disabled June 12, 2024 14:57

Base branch was modified

@realshuting realshuting enabled auto-merge (squash) June 12, 2024 14:58
@realshuting realshuting merged commit fe8c429 into kyverno:main Jun 12, 2024
250 of 254 checks passed
gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Jun 12, 2024
* feat: add generator abstraction

Signed-off-by: ShutingZhao <[email protected]>

* feat: replace urgenerator

Signed-off-by: ShutingZhao <[email protected]>

* fix: ko build

Signed-off-by: ShutingZhao <[email protected]>

* feat: load threshold from kyverno configmap

Signed-off-by: ShutingZhao <[email protected]>

* feat: add metadata client to get ur count

Signed-off-by: ShutingZhao <[email protected]>

* feat: add helm option to preserve configmap settings during upgrade

Signed-off-by: ShutingZhao <[email protected]>

* feat: add helm option to preserve configmap settings during upgrade 2

Signed-off-by: ShutingZhao <[email protected]>

* chore: rename imports

Signed-off-by: ShutingZhao <[email protected]>

* chore: update codegen manifests

Signed-off-by: ShutingZhao <[email protected]>

* fix: handle nil value

Signed-off-by: ShutingZhao <[email protected]>

* fix: linter issue

Signed-off-by: ShutingZhao <[email protected]>

* chore: update threshold to 1000

Signed-off-by: ShutingZhao <[email protected]>

* fix: avoid duplicate URs creation

Signed-off-by: ShutingZhao <[email protected]>

* fix: revert false changes

Signed-off-by: ShutingZhao <[email protected]>

* fix: simplify background applications

Signed-off-by: ShutingZhao <[email protected]>

---------

Signed-off-by: ShutingZhao <[email protected]>
@realshuting realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label Jun 12, 2024
realshuting added a commit that referenced this pull request Jun 13, 2024
…10444)

* feat: add generator abstraction



* feat: replace urgenerator



* fix: ko build



* feat: load threshold from kyverno configmap



* feat: add metadata client to get ur count



* feat: add helm option to preserve configmap settings during upgrade



* feat: add helm option to preserve configmap settings during upgrade 2



* chore: rename imports



* chore: update codegen manifests



* fix: handle nil value



* fix: linter issue



* chore: update threshold to 1000



* fix: avoid duplicate URs creation



* fix: revert false changes



* fix: simplify background applications



---------

Signed-off-by: ShutingZhao <[email protected]>
Co-authored-by: shuting <[email protected]>
jslivka pushed a commit to jslivka/kyverno-hpa that referenced this pull request Jul 2, 2024
)

* feat: add generator abstraction

Signed-off-by: ShutingZhao <[email protected]>

* feat: replace urgenerator

Signed-off-by: ShutingZhao <[email protected]>

* fix: ko build

Signed-off-by: ShutingZhao <[email protected]>

* feat: load threshold from kyverno configmap

Signed-off-by: ShutingZhao <[email protected]>

* feat: add metadata client to get ur count

Signed-off-by: ShutingZhao <[email protected]>

* feat: add helm option to preserve configmap settings during upgrade

Signed-off-by: ShutingZhao <[email protected]>

* feat: add helm option to preserve configmap settings during upgrade 2

Signed-off-by: ShutingZhao <[email protected]>

* chore: rename imports

Signed-off-by: ShutingZhao <[email protected]>

* chore: update codegen manifests

Signed-off-by: ShutingZhao <[email protected]>

* fix: handle nil value

Signed-off-by: ShutingZhao <[email protected]>

* fix: linter issue

Signed-off-by: ShutingZhao <[email protected]>

* chore: update threshold to 1000

Signed-off-by: ShutingZhao <[email protected]>

* fix: avoid duplicate URs creation

Signed-off-by: ShutingZhao <[email protected]>

* fix: revert false changes

Signed-off-by: ShutingZhao <[email protected]>

* fix: simplify background applications

Signed-off-by: ShutingZhao <[email protected]>

---------

Signed-off-by: ShutingZhao <[email protected]>
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.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants