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

Simplify Disallow hostPorts in CEL expressions #1101

Closed

Conversation

lavishpal
Copy link

Related Issue(s)

fixes #1093

Description

This PR simplify the CEL expression by removing the repeated terms.

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.

@lavishpal
Copy link
Author

lavishpal commented Jul 30, 2024

cc: @JimBugwadia

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 see the contribution guide here, specifically the part about modifying Artifact Hub metadata.

@lavishpal
Copy link
Author

@chipzoller I read the contribution guide lines and about Artifact Hub.
A quick question : Do I need to update the artifacthub-pkg.yml as per the changes?

@chipzoller
Copy link
Contributor

Yes, you need to recalculate the digest of the policy and update the relevant section in the AH metadata file.

@chipzoller
Copy link
Contributor

Tests failing.

@lavishpal
Copy link
Author

Can there be different digest values ​​for same policy?

@chipzoller
Copy link
Contributor

If you generate the digest on WIndows versus Linux, yes. The difference in digests come from control characters and line endings. The digests should be generated under Linux or else CI will fail.

@lavishpal
Copy link
Author

Still getting the different digest value from the value given in artifacthub-pkg.yaml of the file
OS used : Debian
version : 12

@chipzoller
Copy link
Contributor

chipzoller commented Aug 1, 2024

Ran it in a Code Space, I get the expected digest.

image

image

@lavishpal
Copy link
Author

Thank you @chipzoller for the guidance, now I will quickly update the artifacthub file.

@chipzoller chipzoller changed the title Enhanced: Simplify the CEL policy to disallow host ports Simplify Disallow hostPorts in CEL expressions Aug 2, 2024
@lavishpal lavishpal force-pushed the simplify-cel-hostport-policy branch from 65ac8eb to 504f84c Compare August 2, 2024 13:12
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.

CLI tests now failing after modifications in this policy.

Signed-off-by: Lavish pal <[email protected]>
@lavishpal lavishpal force-pushed the simplify-cel-hostport-policy branch from 091d84e to 1cc36de Compare August 3, 2024 04:58
@lavishpal lavishpal closed this Aug 3, 2024
@lavishpal lavishpal deleted the simplify-cel-hostport-policy branch August 3, 2024 05:23
@lavishpal
Copy link
Author

Closed this pr in favor of #1108

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.

disallow-host-ports: simplify CEL expressions
2 participants