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: fixes version check for Artifact Hub generation #339

Merged
merged 2 commits into from
May 12, 2023

Conversation

nilekhc
Copy link
Contributor

@nilekhc nilekhc commented May 4, 2023

What this PR does / why we need it:

Currently, we are checking the template hash from a local file. This requires a constant version upgrade when we add new template files and incrementally update it before merging. This is incorrect since for a new file, the version would start at 1.0.0.

With this fix, we are now validating the hash of the template file from GitHub to the hash of the local template file. So when a contributor is trying to create a new template file, it will skip the version check and perform the same for an existing template file for a potential version bump.

Which issue(s) does this PR fix (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #331

Special notes for your reviewer:

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks @nilekhc!

I think hub_test.go maybe missing when an expected error does not occur (err == nil && tc.expectedError)? https://github.com/open-policy-agent/gatekeeper-library/blob/master/scripts/artifacthub/hub_test.go#L215

scripts/artifacthub/hub.go Outdated Show resolved Hide resolved
scripts/artifacthub/hub.go Show resolved Hide resolved
Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com>
@nilekhc
Copy link
Contributor Author

nilekhc commented May 12, 2023

Thanks @nilekhc!

I think hub_test.go maybe missing when an expected error does not occur (err == nil && tc.expectedError)? https://github.com/open-policy-agent/gatekeeper-library/blob/master/scripts/artifacthub/hub_test.go#L215

@apeabody I updated the entire test method. PTAL.

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks @nilekhc LGTM!

@nilekhc nilekhc merged commit c638e1c into open-policy-agent:master May 12, 2023
@nilekhc nilekhc deleted the fix-ah-version-check branch May 12, 2023 22:05
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.

fix version check in Artifact Hub generation
2 participants