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

Update disallow-helm-tiller and disallow-latest-tag to include all container types in a pod #1111

Merged
merged 38 commits into from
Aug 14, 2024

Conversation

dolisss
Copy link
Contributor

@dolisss dolisss commented Aug 5, 2024

Related Issue(s)

Partially addresses #951

Description

Updating disallow-helm-tiller and disallow-latest-tag policies to handle all pod types and test cases to handle initContainers.

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: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
@dolisss dolisss marked this pull request as draft August 5, 2024 20:43
@dolisss dolisss marked this pull request as ready for review August 5, 2024 20:45
@dolisss dolisss changed the title Update sample policies to include all container types in a pod. Issue reference: #951 Update sample policies to include all container types in a pod. Aug 5, 2024
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.

Tiller (Helm v2) is so old technology I don't think it's worth making this change. Would suggest focusing on more useful policies in the present.

@dolisss
Copy link
Contributor Author

dolisss commented Aug 6, 2024

I am updating all the best practice policies. Can you approve this one while I add the logic in other policies too?

@chipzoller
Copy link
Contributor

chipzoller commented Aug 6, 2024

Go ahead and make those updates. Remove from draft mode when ready for review. Please remember to follow the contribution guide as linked in the PR template. And please also ensure to increase test coverage of the policies you update in this PR as well.

@chipzoller chipzoller marked this pull request as draft August 6, 2024 13:04
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
@dolisss
Copy link
Contributor Author

dolisss commented Aug 6, 2024

@chipzoller Extended the logic to the "disallow latest tag" policy along with changes needed for the chainsaw test cases.

@dolisss dolisss requested a review from chipzoller August 6, 2024 19:33
@dolisss dolisss closed this Aug 6, 2024
@dolisss dolisss reopened this Aug 6, 2024
@dolisss dolisss marked this pull request as ready for review August 6, 2024 19:37
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.

Missing extended tests for disallow-helm-tiller.

Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
@dolisss
Copy link
Contributor Author

dolisss commented Aug 6, 2024

@chipzoller thought the Helm Tiller policy was outdated and that test cases might not be necessary. However, I’ve updated them now. Please review the changes

@dolisss dolisss requested a review from chipzoller August 6, 2024 19:45
@chipzoller chipzoller changed the title Update sample policies to include all container types in a pod. Update disallow-helm-tiller and disallow-latest-tag to include all container types in a pod Aug 6, 2024
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.

Please fix failing tests. Also complete the PR template (checklist).

Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
@dolisss dolisss requested a review from chipzoller August 7, 2024 14:34
@dolisss
Copy link
Contributor Author

dolisss commented Aug 7, 2024

@chipzoller I tried updating the digest using my Mac, but the check failed. I reverted to the old digest and I think it got verified. How can I generate the digest? I am using the following command.
It's strange that it takes the digest for the other policy I generated in the same way.

 shasum -a 256 disallow-latest-tag.yaml 
2760272e57d9988ba447f62d23bba382092d00a5e14dbf00555e4170ea90593a 

Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.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.

Digests are expected to be generated in Linux; you can see the digest expected in the Policy Test step and use that if you're unable to generate one on your own system.

Tests still need to be fixed here.

Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
@dolisss
Copy link
Contributor Author

dolisss commented Aug 12, 2024

@chipzoller Thanks! it worked with Linux. What checks are you referring to? I don't see any on my page.

@chipzoller
Copy link
Contributor

Please see failing CI checks.

Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
Signed-off-by: Dolis Sharma <71091713+dolisss@users.noreply.github.com>
@dolisss
Copy link
Contributor Author

dolisss commented Aug 14, 2024

@chipzoller Please review.

@chipzoller chipzoller merged commit 4ee239a into kyverno:main Aug 14, 2024
280 checks passed
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.

2 participants