-
Notifications
You must be signed in to change notification settings - Fork 40
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
Ability to skip individual errors; rework configs #86
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kolyshkin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This was tested on the following payloads: check-payload scan payload -V 4.9 -p --url registry.ci.openshift.org/ocp/release:4.9.0-0.nightly-2023-07-21-125156
check-payload scan payload -V 4.10 -p --url registry.ci.openshift.org/ocp/release:4.10.0-0.nightly-2023-07-20-215419
check-payload scan payload -V 4.11 -p --url registry.ci.openshift.org/ocp/release:4.11.0-0.nightly-2023-07-20-121744
check-payload scan payload -V 4.12 -p --url registry.ci.openshift.org/ocp/release:4.12.0-0.nightly-2023-07-20-151221 |
Retested against the latest 4.9 to 4.12 nightlies, as per @mrunalp suggestion: ./check-payload scan payload -V 4.12 -p --url registry.ci.openshift.org/ocp/release:4.12.0-0.nightly-2023-07-24-212044
./check-payload scan payload -V 4.11 -p --url registry.ci.openshift.org/ocp/release:4.11.0-0.nightly-2023-07-24-221011
./check-payload scan payload -V 4.10 -p --url registry.ci.openshift.org/ocp/release:4.10.0-0.nightly-2023-07-21-181058
./check-payload scan payload -V 4.9 -p --url registry.ci.openshift.org/ocp/release:4.9.0-0.nightly-2023-07-24-232617 Clean runs, no errors. |
Whoops, hit Send too early. One new issue reported:
|
@nickboldt you might take a look at this PR, as it removes the rules you've added in #63, #74, and #77. This is done because we are now excluding individual errors. I'll be happy to add the remove exceptions back if you provide me a payload or image URL to test on. |
This introduces (and uses) a set of known errors, and a map to resolve their names. To be used by the next commit. The map is generated from the source code, so add a Makefile target to check the generated code is in sync with its source. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This allows to exclude specific known errors for specific files, meaning that instead of skipping all checks for a particular file, we can only skip some specific ones (or, rather, ignore specific errors for specific files). The rules to ignore errors can be specified globally, or per-tag, component, or rpm. The implementation is somewhat complicated because we have multiple set of rules, plus in case of payload scan we do not know the rpm name beforehand. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
751b987
to
c425133
Compare
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This allows to avoid repetition in for-version configs. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Place common rules into the main config, additional ones into the versioned ones. 2. Use per-rpm per-error exclusions where possible. Warning: I might have thrown a baby or two out with the bathwater. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
/lgtm |
@kolyshkin: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
This is a rebase/rework of #33, adding an ability to ignore individual errors (rather than skip a file from the validation entirely).
This also reworks configuration to be mostly per-rpm per-error based, and makes the versioned config files additions to the main one.
Please review commit-by-commit.