CORENET-6822: Add ingress-node-firewall non-payload OTE extension step#79937
CORENET-6822: Add ingress-node-firewall non-payload OTE extension step#79937anuragthehatter wants to merge 2 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@anuragthehatter: This pull request references CORENET-7206 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an infw-extension CI step (ref, metadata, OWNERS, commands) and inserts an optional e2e-aws-ovn-infw-extension test step into the ingress-node-firewall job; the step creates TestExtensionAdmission, namespace, and ImageStream pointing at the extension image and verifies them. ChangesIngress Firewall Extension Testing
Sequence Diagram(s): sequenceDiagram
participant Job as ingress-node-firewall job
participant CI as ci-operator
participant Ref as infw-extension ref
participant Cluster as OpenShift API
Job->>CI: trigger optional e2e-aws-ovn-infw-extension
CI->>Ref: execute infw-extension ref (runs script)
Ref->>Cluster: create TestExtensionAdmission, namespace, ImageStreamTag (import from EXTENSION_IMAGE)
Ref->>Cluster: dump/verify resources
Ref->>CI: exit/signal completion
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/invalid-owners-file
openshift-merge-bot
Bot
added
the
rehearsals-ack
anuragthehatter
changed the title
|
@anuragthehatter: This pull request references CORENET-6822 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yaml`:
- Around line 75-85: You added a new test entry (ref: infw-extension) and job
metadata (as: e2e-aws-ovn-infw-extension, workflow: openshift-e2e-aws-ovn) in
the CI-operator config but did not regenerate the derived Prow job configs; run
make update to regenerate CI outputs, commit the changed files under
ci-operator/jobs (and any other generated job/config diffs), and ensure the new
job e2e-aws-ovn-infw-extension and its generated prow job entries are present
and validated before pushing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 553c96d9-beca-42bd-b1d1-ce42f58b9e24
📒 Files selected for processing (5)
ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yamlci-operator/step-registry/infw-extension/OWNERSci-operator/step-registry/infw-extension/infw-extension-commands.shci-operator/step-registry/infw-extension/infw-extension-ref.metadata.jsonci-operator/step-registry/infw-extension/infw-extension-ref.yaml
anuragthehatter
force-pushed
the
infw-extension
branch
2 times, most recently
from
ffeea56 to
c3a74b2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/infw-extension/infw-extension-commands.sh (1)
49-52: ⚡ Quick winMake verification error handling consistent.
Line 51 will cause the script to exit if the
testextensionadmissionresource doesn't exist (due toset -o errexit), while line 52 tolerates failures with|| true. For verification commands, consider making both tolerate failures consistently.♻️ Proposed fix for consistent error handling
# Verify setup echo "Verifying extension setup..." -oc get testextensionadmission infw-extensions -o yaml +oc get testextensionadmission infw-extensions -o yaml || true oc get imagestreamtag ingress-node-firewall-tests:latest -n test-extensions -o jsonpath='{.metadata.annotations}' | python3 -m json.tool || true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/infw-extension/infw-extension-commands.sh` around lines 49 - 52, The verification step is inconsistent: the command "oc get testextensionadmission infw-extensions -o yaml" will fail the script under errexit while the subsequent "oc get imagestreamtag ingress-node-firewall-tests:latest -n test-extensions -o jsonpath='{.metadata.annotations}' | python3 -m json.tool || true" intentionally tolerates failure; make them consistent by ensuring both verification commands tolerate non-existence (e.g., append "|| true" to the testextensionadmission oc get or wrap both in a non-failing check), updating the invocation referencing "oc get testextensionadmission infw-extensions -o yaml" and the imagestreamtag command so neither causes the script to exit on missing resources.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ci-operator/step-registry/infw-extension/infw-extension-commands.sh`:
- Around line 49-52: The verification step is inconsistent: the command "oc get
testextensionadmission infw-extensions -o yaml" will fail the script under
errexit while the subsequent "oc get imagestreamtag
ingress-node-firewall-tests:latest -n test-extensions -o
jsonpath='{.metadata.annotations}' | python3 -m json.tool || true" intentionally
tolerates failure; make them consistent by ensuring both verification commands
tolerate non-existence (e.g., append "|| true" to the testextensionadmission oc
get or wrap both in a non-failing check), updating the invocation referencing
"oc get testextensionadmission infw-extensions -o yaml" and the imagestreamtag
command so neither causes the script to exit on missing resources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 12ac790c-bcd7-45b5-a2cc-485ea9846412
📒 Files selected for processing (5)
ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yamlci-operator/step-registry/infw-extension/OWNERSci-operator/step-registry/infw-extension/infw-extension-commands.shci-operator/step-registry/infw-extension/infw-extension-ref.metadata.jsonci-operator/step-registry/infw-extension/infw-extension-ref.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/infw-extension/OWNERS
- ci-operator/step-registry/infw-extension/infw-extension-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yaml
|
/pj-rehearse pull-ci-openshift-ingress-node-firewall-master-e2e-aws-ovn-infw-extension |
|
@anuragthehatter: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
removed
the
rehearsals-ack
|
/testwith openshift/ingress-node-firewall#694 |
anuragthehatter
force-pushed
the
infw-extension
branch
from
c3a74b2 to
2672a1c
Compare
|
/pj-rehearse pull-ci-openshift-ingress-node-firewall-master-e2e-aws-ovn-infw-extension |
|
@anuragthehatter: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
anuragthehatter
force-pushed
the
infw-extension
branch
from
2672a1c to
a104170
Compare
|
/pj-rehearse pull-ci-openshift-ingress-node-firewall-master-e2e-aws-ovn-infw-extension |
|
@anuragthehatter: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yaml (1)
75-91:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRun
make updateto regenerate ProwJob configs for the new test.Adding a new test definition is a structural change that requires regenerating the derived ProwJob configurations under
ci-operator/jobs/. This was already flagged in a previous review and remains unresolved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yaml` around lines 75 - 91, You added a new test definition (as: e2e-aws-ovn-infw-extension, refs: optional-operators-subscribe and infw-extension, workflow: openshift-e2e-aws-ovn) but did not regenerate derived ProwJob configs; run the repository regeneration step (make update) to regenerate ci-operator/jobs/* ProwJob files for this new test and commit the resulting changes so the new test is picked up by CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yaml`:
- Around line 75-91: You added a new test definition (as:
e2e-aws-ovn-infw-extension, refs: optional-operators-subscribe and
infw-extension, workflow: openshift-e2e-aws-ovn) but did not regenerate derived
ProwJob configs; run the repository regeneration step (make update) to
regenerate ci-operator/jobs/* ProwJob files for this new test and commit the
resulting changes so the new test is picked up by CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e0d2576a-c221-4aea-ae70-1b4956a47a7e
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yamlci-operator/step-registry/infw-extension/OWNERSci-operator/step-registry/infw-extension/infw-extension-commands.shci-operator/step-registry/infw-extension/infw-extension-ref.metadata.jsonci-operator/step-registry/infw-extension/infw-extension-ref.yaml
✅ Files skipped from review due to trivial changes (1)
- ci-operator/step-registry/infw-extension/infw-extension-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (3)
- ci-operator/step-registry/infw-extension/infw-extension-ref.yaml
- ci-operator/step-registry/infw-extension/OWNERS
- ci-operator/step-registry/infw-extension/infw-extension-commands.sh
anuragthehatter
force-pushed
the
infw-extension
branch
from
a104170 to
510adaa
Compare
|
/pj-rehearse pull-ci-openshift-ingress-node-firewall-master-e2e-aws-ovn-infw-extension |
|
@anuragthehatter: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@anuragthehatter: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
anuragthehatter
force-pushed
the
infw-extension
branch
from
510adaa to
c4f2c67
Compare
Make the infw-extension step suite-configurable via INFW_TEST_SUITE env var, defaulting to the aws suite (excludes Baremetal-labeled tests). Add the infw-extension step to the baremetal workflow with INFW_TEST_SUITE set to "all" to include Baremetal-labeled tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: anuragthehatter The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
The OWNERS file contains untrusted users, which makes it INVALID. The following users are mentioned in OWNERS file(s) but are untrusted for the following reasons. One way to make the user trusted is to add them as members of the openshift org. You can then trigger verification by writing
|
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
2 participants
Summary
infw-extensionfor ingress-node-firewall non-payload OTE extension discoveryTestExtensionAdmissionCR and ImageStream withtestextension.redhat.ioannotationse2e-aws-ovn-infw-extensiontest job to ingress-node-firewall master configRelated PRs
References
TestExtensionAdmissionCRD during cluster install #76232🤖 Generated with Claude Code
Summary by CodeRabbit
This PR adds reusable OpenShift CI support to run ingress-node-firewall’s non-payload OpenTest Extension (OTE) discovery tests and wires an optional master CI job to invoke it in the ingress-node-firewall pipeline.
What changed in practical terms:
New ci-operator step-registry entry: ci-operator/step-registry/infw-extension
CI job wiring: ci-operator/config/openshift/ingress-node-firewall/openshift-ingress-node-firewall-master.yaml
Why this matters: