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

OPRUN-3458,OPRUN-3459: add policy enforcement for bundle metadata format based on OCP versions #1327

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

grokspawn
Copy link
Contributor

@grokspawn grokspawn commented Aug 21, 2024

This update augments the fbc-validation task to enforce bundle metadata formats for incoming fragments based on the destination catalog's OCP release.

For OCP releases before 4.17, this will ensure that all bundles use only the olm.bundle.object object.
For OCP releases at/after 4.17, this will ensure that all bundles use only the olm.csv.metadata object.

@openshift-ci openshift-ci bot requested review from dirgim and jsztuka August 21, 2024 20:51
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch from 601797e to 01c08e3 Compare August 21, 2024 20:51
@grokspawn grokspawn changed the title add policy enforcement for bundle metadata format based on OCP versions OPRUN-3458: add policy enforcement for bundle metadata format based on OCP versions Aug 21, 2024
task/fbc-validation/0.1/fbc-validation.yaml Outdated Show resolved Hide resolved
task/fbc-validation/0.1/fbc-validation.yaml Outdated Show resolved Hide resolved
task/fbc-validation/0.1/fbc-validation.yaml Outdated Show resolved Hide resolved
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from afdfe35 to d080c27 Compare August 22, 2024 13:10
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from 1e03811 to 38110b0 Compare August 26, 2024 18:55
Copy link
Contributor

@jsztuka jsztuka left a comment

Choose a reason for hiding this comment

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

lgtm, +1 for discussion on failure format.

@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from 3a24a04 to 770ab77 Compare August 28, 2024 21:09
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from 97412d8 to 27f0550 Compare August 29, 2024 15:03
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 3 times, most recently from 3b8b031 to 0b9933a Compare August 29, 2024 16:16
@grokspawn grokspawn changed the title OPRUN-3458: add policy enforcement for bundle metadata format based on OCP versions OPRUN-3458,OPRUN-3459: add policy enforcement for bundle metadata format based on OCP versions Aug 29, 2024
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from 1ad5364 to 4afb3d1 Compare August 30, 2024 19:39
@grokspawn grokspawn requested a review from joelanford August 30, 2024 19:40
@grokspawn grokspawn requested review from dirgim, arewm and jsztuka August 30, 2024 19:40
@grokspawn
Copy link
Contributor Author

I'm not clear on how to kick the CI.... I see that the CI is not running because

User grokspawn is not allowed to trigger CI via pull_request on this repo.

Help?

@arewm
Copy link
Member

arewm commented Aug 30, 2024

/ok-to-test

@grokspawn
Copy link
Contributor Author

🎉 Now it looks like there is an additional required action. I see an approval. Is there something I must do?

@arewm
Copy link
Member

arewm commented Aug 31, 2024

The Checkton check is showing violations in your task changes.

The trusted artifact variant check is likely flagging issues due to your change to the README generator. I know that we have gone back and forth on it, but you can back out that change if you don't want to resolve the differences. That change can be proposed later.

@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from 6792d91 to 55268b6 Compare September 3, 2024 14:21
@grokspawn
Copy link
Contributor Author

The Checkton check is showing violations in your task changes.

The trusted artifact variant check is likely flagging issues due to your change to the README generator. I know that we have gone back and forth on it, but you can back out that change if you don't want to resolve the differences. That change can be proposed later.

I understand what the tasks are communicating. The tasks aren't identified by the CI flow as required (and the checkton check didn't exist until 8/30).

To help folks contribute to this repo, it would be really helpful if

  1. the set of required checks would reflect the merge check minimums; and
  2. PRs that introduce checks should also vet impacts to existing code (and IMO create a PR to address, so that other contributors just have to merge/rebase to update their efforts).

@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch 2 times, most recently from 0f63b1b to 3669f89 Compare September 3, 2024 18:11
@grokspawn grokspawn force-pushed the bundle-metadata-decisions branch from 3669f89 to a2c6498 Compare September 3, 2024 18:31
@arewm arewm added this pull request to the merge queue Sep 3, 2024
Merged via the queue into konflux-ci:main with commit 0057cbc Sep 3, 2024
13 checks passed
@grokspawn grokspawn deleted the bundle-metadata-decisions branch September 5, 2024 00:19
@hongweiliu17
Copy link
Contributor

The issue from #konflux-users might be caused by this commit, see analysis
The BASE_IMAGE is like registry.redhat.io/openshift4/ose-operator-registry-rhel9@sha256:56e5c57b9d94bcadad6ff0cd47d157d35affc4bfd2c23a2ac948eee1cac3b8a3, not registry.redhat.io/openshift4/ose-operator-registry-rhel9:v4.16, so that the OCP major version and minor version can't be derived correctly.

@chmeliik
Copy link
Contributor

chmeliik commented Sep 11, 2024

1. the set of required checks would reflect the merge check minimums; and

I don't think we treat any of the existing checks as optional. PRs don't get merged if the checks are not green.

2. PRs that introduce checks should also vet impacts to existing code (and IMO create a PR to address, so that other contributors just have to merge/rebase to update their efforts).

I believe that's how we do it (checks have to be passing before PRs get merged)

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.

8 participants