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: add separate workflow for CEL policies to unblock e2e tests #1087

Merged
merged 7 commits into from
Jul 30, 2024

Conversation

Chandan-DK
Copy link
Contributor

@Chandan-DK Chandan-DK commented Jul 27, 2024

Related Issue(s)

Description

This PR resolves Strategy expansion exceeded 256 results for job 'chainsaw' error that is currently blocking the E2E tests from running.

A separate workflow file has been created for CEL policies and this PR makes use of composite actions to reuse steps in both the workflow files (test.yml and cel-test.yml)

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.

Signed-off-by: Chandan-DK <chandan.dk@nirmata.com>
@Chandan-DK Chandan-DK marked this pull request as draft July 27, 2024 14:23
@Chandan-DK
Copy link
Contributor Author

Chandan-DK commented Jul 27, 2024

Made this PR by mistake. Was trying to find a way to avoid Strategy expansion exceeded 256 results for job 'chainsaw' error that is currently blocking the E2E tests from running. If separating the regular kyverno policies from the CEL policies with two workflow files seems like a suitable approach, I can reopen this and get it in shape. Closing for now.

@Chandan-DK Chandan-DK closed this Jul 27, 2024
@chipzoller
Copy link
Contributor

Yes, I think this would be a good approach! Please do reopen!

@realshuting
Copy link
Member

Hi @Chandan-DK - let us know if you are still interested in contribution!

@Chandan-DK
Copy link
Contributor Author

Hi! I will get this PR in shape for review 🙌

Signed-off-by: Chandan-DK <chandan.dk@nirmata.com>
Signed-off-by: Chandan-DK <chandan.dk@nirmata.com>
@Chandan-DK Chandan-DK changed the title add separate workflow file for CEL policies fix: add separate workflow for CEL policies to unblock e2e tests Jul 29, 2024
@Chandan-DK Chandan-DK marked this pull request as ready for review July 29, 2024 18:48
@realshuting
Copy link
Member

Thanks @Chandan-DK !

All new tests failed due to this:

   === ERROR
    admission webhook "validate-policy.kyverno.svc" denied the request: no unique match for kind GitRepository
l.go:53: ���������| 18:25:20 | verify-git-repositories | 01 - Create policy and verify  | CREATE    | ERROR | kyverno.io/v1/ClusterPolicy @ verify-git-repositories
    === ERROR
    admission webhook "validate-policy.kyverno.svc" denied the request: no unique match for kind GitRepository

Looks like something is missing, can you take a look?

@realshuting realshuting self-assigned this Jul 30, 2024
@Chandan-DK
Copy link
Contributor Author

Thanks @Chandan-DK !

All new tests failed due to this:

   === ERROR
    admission webhook "validate-policy.kyverno.svc" denied the request: no unique match for kind GitRepository
l.go:53: ���������| 18:25:20 | verify-git-repositories | 01 - Create policy and verify  | CREATE    | ERROR | kyverno.io/v1/ClusterPolicy @ verify-git-repositories
    === ERROR
    admission webhook "validate-policy.kyverno.svc" denied the request: no unique match for kind GitRepository

Looks like something is missing, can you take a look?

verify-git-repositories cel policy is not being created due to this issue (kyverno/kyverno#10313). I have removed this policy for now.

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.

Could we rename the .chainsaw-test directory of the problematic policy rather than just deleting it?

@Chandan-DK
Copy link
Contributor Author

Sure, but there will still be noise in the CI since Chainsaw executes the chainsaw-test.yaml file. We could rename this file too, to prevent noise. What do you think?

@chipzoller
Copy link
Contributor

I would ask @eddycharly what's the most reliable thing to do here. I'm not sure what the current recommended approach is for Chainsaw given its present state.

@Chandan-DK
Copy link
Contributor Author

Chandan-DK commented Jul 30, 2024

Chainsaw by default looks for the chainsaw-test.yaml file inside every folder. We could rename the .chainsaw-test directory of the problematic policy to something like .chainsaw-test-rename-after-issue-10313-fix to make it easy to identify this problematic policy in the future. Changing the directory name won't stop the chainsaw-test.yaml file that is present in it from being executed. So, we should probably name the file this way chainsaw-test-rename-after-issue-10313-fix.yaml and avoid a failing chainsaw test for the time being.

By the way, the policies that have been deleted due to this issue are being tracked here. I have been referencing the link of the issue in my commits when deleting policies. We could use those commits to help add the policies back when the issue is resolved.
image

@chipzoller
Copy link
Contributor

Ok, that sounds good. Let's give it a try.

Signed-off-by: Chandan-DK <chandan.dk@nirmata.com>
… running for verify-git-repositories policy

Signed-off-by: Chandan-DK <chandan.dk@nirmata.com>
Signed-off-by: Chandan-DK <chandan.dk@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.

Much thanks! I'm going to go ahead and merge this so we can run our e2e tests again.

@chipzoller chipzoller merged commit 32371e4 into kyverno:main Jul 30, 2024
265 checks passed
@realshuting
Copy link
Member

Good job @Chandan-DK !

@Chandan-DK
Copy link
Contributor Author

Thanks @realshuting @chipzoller !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants