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

Bug - annotation inserted to podTemplate by auto-gen should reflect the policy name #850

Merged
merged 7 commits into from
May 18, 2020

Conversation

yindia
Copy link
Contributor

@yindia yindia commented May 15, 2020

Status Type Env Vars Change Issue
Ready Bug No #829

Problem

Describe the bug
Currently, with the auto-gen feature, it inserts an annotation pod-policies.kyverno.io/autogen-applied: "true" to podTemplate, but this creates an issue that the policy with auto-gen disabled will never apply to the pod. Related issue #819.

The annotation should contain the policy information(name) and only skip applying to the pod for that particular policy.

For example, there are 2 policies: policy-a with auto-gen enabled, policy-b with auto-gen disabled, the inserted annotation should be similar to policy-a.pod-policies.kyverno.io/autogen-applied: "true", while the policy-b should still apply to the pod.

Solution

As per f1ko
However, my proposal would cover your concern.
Policy pol-a has pod-policies.kyverno.io/autogen-controllers: all set.
Therefore, controller rules will be added to the policy (same as now). Then these controller rules should add pod-policies.kyverno.io/autogen-applied: "true" to a Pod (same as now). By doing so all Pod rules in the policy will ignore Pods that have that annotation set (same as now).
So basically there is no change in behavior when it comes to pol-a.
Now lets take a look at pol-b. pol-b has pod-policies.kyverno.io/autogen-controllers: none, therefore no additional rules are added (same as now). However, because autogen-controllers has been set to none, kyverno shall allow Pod rules within that policy to ignore pod-policies.kyverno.io/autogen-applied: "true".

Other changes

Yuvraj added 2 commits May 15, 2020 03:02
* 'master' of github.com:nirmata/kyverno: (102 commits)
  fix 843 (#844)
  chart readme fixes
  Fix Helm chart README.md for Helm 3
  Add Helm chart for Kyverno - #835
  Documentation update
  664 tested prototype
  823 tested prototype
  Fixes #817 - slack channel URL
  797 typo fix
  Fixes #797 - update example for mutate patch policy
  remove cpu limit in BP require_pod_requests_limits.yaml (#807)
  808 test fixes
  808_prototype
  753 reverting autogen rule changes
  786 fixed tests
  786 tested prototype
  775 circle ci fixes
  753 fixing tests
  775 working prototype
  753_avoiding_duplicate_vals
  ...
@yindia yindia requested a review from realshuting May 15, 2020 12:33
@realshuting
Copy link
Member

@evalsocket Please add a brief description of what this PR changes.

@yindia yindia changed the title Fix 829 annotation inserted to podTemplate by auto-gen should reflect the policy name May 15, 2020
@yindia yindia changed the title annotation inserted to podTemplate by auto-gen should reflect the policy name Bug - annotation inserted to podTemplate by auto-gen should reflect the policy name May 15, 2020
@realshuting
Copy link
Member

Is this PR completed? The code change does not seem to achieve the proposed solution.

@@ -34,8 +34,13 @@ func (pc *PolicyController) processExistingResources(policy kyverno.ClusterPolic
}

// skip reporting violation on pod which has annotation pod-policies.kyverno.io/autogen-applied
if skipPodApplication(resource, logger) {
continue
ann := resource.GetAnnotations()
Copy link
Member

Choose a reason for hiding this comment

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

We should check the annotation on Policy, not resource.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch @realshuting!

@realshuting realshuting self-requested a review May 17, 2020 05:16
Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

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

See comment.

Yuvraj added 2 commits May 16, 2020 23:51
* 'fix-829' of github.com:evalsocket/kyverno:
  Revert Changes
@yindia yindia requested a review from realshuting May 17, 2020 07:36
Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

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

See comment.

pkg/policy/existing.go Show resolved Hide resolved
Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

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

See comment.

@realshuting realshuting merged commit 0635df7 into kyverno:master May 18, 2020
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.

None yet

3 participants