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] 1.5.4-rc2 exclude clusterRole from Policy broken #2964

Closed
MaxRink opened this issue Jan 11, 2022 · 13 comments · Fixed by #2990
Closed

[BUG] 1.5.4-rc2 exclude clusterRole from Policy broken #2964

MaxRink opened this issue Jan 11, 2022 · 13 comments · Fixed by #2990
Assignees
Labels
bug Something isn't working end user This label is used to track the issue that is raised by the end user.

Comments

@MaxRink
Copy link

MaxRink commented Jan 11, 2022

Software version numbers
State the version numbers of applications involved in the bug.

  • Kubernetes version: 1.20.13
  • Kubernetes platform (if applicable; ex., EKS, GKE, OpenShift): CAPI
  • Kyverno version: 1.5.4-rc2

Describe the bug
We have the following Policy that doesnt work after an upgrade from 1.5.2 to 1.5.4-rc2:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: restrict-labels
  labels:
    policy.schiff.telekom.de: enforced
  annotations:
    policies.kyverno.io/title: Restrict Labels on Namespaces
    policies.kyverno.io/category: Labels
    policies.kyverno.io/minversion: 1.3.0
    policies.kyverno.io/description: >-
      This policy prevents the use of an label beginning with a common
      key name (in this case "platform.das-schiff.telekom.de/owner | owner"). This can be useful to ensure users either
      don't set reserved labels or to force them to
      use a newer version of an label.
spec:
  validationFailureAction: enforce
  background: false
  rules:
  - name: restrict-labels
    match:
      resources:
        kinds:
        - Namespace
    exclude:
      clusterRoles:
      - cluster-admin
    validate:
      message: 'Every namespace has to have `platform.das-schiff.telekom.de/owner` label. It must not have value `das-schiff` which is reserved for system namespaces'
      pattern:
        metadata:
          labels:
            platform.das-schiff.telekom.de/owner: "!das-schiff"
            # For forward compatibility
            =(schiff.telekom.de/owner): "!schiff"

The exclusion for clusterAdmin doesnt apply anymore, thus creation of a namespace with those labels fails.

k apply -f -
apiVersion: v1
kind: Namespace
metadata:
  name: kyverno-system-tst
  labels:
    name: kyverno-system-tst
    schiff.telekom.de/owner: schiff
    platform.das-schiff.telekom.de/owner: das-schiff
Error from server: error when creating "STDIN": admission webhook "validate.kyverno.svc-fail" denied the request: 

resource Namespace//kyverno-system-tst was blocked due to the following policies

restrict-labels:
  restrict-labels: 'validation error: Every namespace has to have `platform.das-schiff.telekom.de/owner`
    label. It must not have value `das-schiff` which is reserved for system namespaces.
    Rule restrict-labels failed at path /metadata/labels/schiff.telekom.de/owner/'

To Reproduce
Steps to reproduce the behavior:
Apply the policy
Try to create the NS as cluster-admin

Expected behavior
i am able to create the NS
Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@MaxRink MaxRink added the bug Something isn't working label Jan 11, 2022
@MaxRink MaxRink changed the title [BUG] 1.5.4-rc2 exclude cluSterRole from Policy broken [BUG] 1.5.4-rc2 exclude clusterRole from Policy broken Jan 11, 2022
@chipzoller
Copy link
Member

Does it work if you nest the exclusion under an any or all tag? We recently fixed a bug there and this might be a consequence of that.

@realshuting realshuting added the end user This label is used to track the issue that is raised by the end user. label Jan 12, 2022
@realshuting realshuting added this to the Kyverno Release 1.6.1 milestone Jan 12, 2022
@MaxRink
Copy link
Author

MaxRink commented Jan 12, 2022

Does it work if you nest the exclusion under an any or all tag? We recently fixed a bug there and this might be a consequence of that.

I dont know, we rolled back to 1.5.2 as that bug basically prevents usage of our clusters

@chipzoller
Copy link
Member

chipzoller commented Jan 12, 2022

Looks like this didn't get fixed as initially reported in #2819. Exclusion of clusterRoles is now entirely broken, whether placed under any/all or not.

@chipzoller
Copy link
Member

Regression re prioritized for Kyverno 1.5.4.

@MaxRink
Copy link
Author

MaxRink commented Jan 12, 2022

Regression re prioritized for Kyverno 1.5.4.

Which is already released :S

@chipzoller
Copy link
Member

1.6.0

@dkulchinsky
Copy link
Contributor

@chipzoller @JimBugwadia considering this is quite a regression and we had to upgrade to 1.5.5 due other bugs in previous versions, would you consider cherry-picking this for a patch release in 1.5? I'm not sure how far away 1.6 is, but looks like there's still plenty of issues open on the 1.6 milestone.

@chipzoller
Copy link
Member

1.6 is within the next couple weeks.

@JimBugwadia
Copy link
Member

@dkulchinsky - the team is currently working on getting an 1.6 RC completed, in the next day or so. As Chip mentioned, getting the release completed may have a couple of weeks.

The fix looks fairly localized, so should be OK to cherry-pick / merge to 1.5.x. Do you want to submit a PR? Otherwise, we can revisit after the RC.

@realshuting - any additional thoughts?

@dkulchinsky
Copy link
Contributor

dkulchinsky commented Jan 23, 2022

@dkulchinsky - the team is currently working on getting an 1.6 RC completed, in the next day or so. As Chip mentioned, getting the release completed may have a couple of weeks.

The fix looks fairly localized, so should be OK to cherry-pick / merge to 1.5.x. Do you want to submit a PR? Otherwise, we can revisit after the RC.

@realshuting - any additional thoughts?

Thanks @JimBugwadia, just looked at the fix, indeed looks simple enough so happy to try a PR for 1.5, this is impacting us with several policies, so would be great to have patch release with this fix asap (we also can't go back to 1.5.2 because of other bugs there were fixed since than)

@dkulchinsky
Copy link
Contributor

@JimBugwadia @realshuting looking at cherry picking this for 1.5, but seems that it depends on a function (checkForRBACInfo) that seem to be introduced in 1.6 dev branch, so I may need some guidance on how we want to address this for 1.5? should I backport this function as well?

@JimBugwadia
Copy link
Member

Hi @dkulchinsky - yes it seems like that function will need to be back ported as well:

func checkForRBACInfo(rule kyverno.Rule) bool {

It looks fairly independent, so should not require pulling in anything additions.

Let me know if you run into any issues with porting it.

@dkulchinsky
Copy link
Contributor

I think I managed to get it sorted @JimBugwadia

#3072

/cc @realshuting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working end user This label is used to track the issue that is raised by the end user.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants