Run koncur tests that require maven settings in nightlies - #256
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughKoncur actions now use isolated checkouts, explicit Maven controls, and workflow-provided references. Global CI invokes local Koncur actions and passes the resolved Kantra image. Nightly and pull-request workflows pass Koncur references separately. ChangesKoncur CI behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CheckImages
participant GlobalCI
participant KoncurAction
participant KoncurCheckout
participant TestRunner
CheckImages->>GlobalCI: Export resolved kantra_image
GlobalCI->>KoncurAction: Pass OS, Koncur ref, Maven setting, and image
KoncurAction->>KoncurCheckout: Check out Koncur into koncur
KoncurAction->>TestRunner: Build and run tests from koncur
TestRunner-->>GlobalCI: Upload test artifacts
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/nightly-koncur-0.9.yaml:
- Line 201: In the report_failure job of
.github/workflows/nightly-koncur-0.9.yaml, add SLACK_WEBHOOK_URL to the
job-level env block and change the check at lines 201-201 to use
env.SLACK_WEBHOOK_URL != ''. Apply the same env declaration and condition change
in .github/workflows/nightly-koncur.yaml at lines 205-205; update both affected
checks consistently.
In `@koncur-tackle-hub/action.yml`:
- Line 6: Update the skip_maven condition in the action workflow to compare the
input explicitly against the string 'false' rather than negating
inputs.skip_maven. Apply this to the Maven setup conditions and corresponding
inputs.os checks so the steps run when the composite-action input is passed as
"false".
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 40a35a10-c33e-40c4-a962-af0fbd1e59ca
📒 Files selected for processing (5)
.github/workflows/ci-repo.yaml.github/workflows/nightly-koncur-0.9.yaml.github/workflows/nightly-koncur.yamlkoncur-kantra/action.ymlkoncur-tackle-hub/action.yml
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/global-ci-bundle.yml:
- Line 600: Update the local composite action manifest used by `uses:
./koncur-kantra` by removing unsupported `type` metadata keys from its input
definitions. Keep supported input fields such as descriptions and defaults
intact so the matrix jobs can start successfully.
In @.github/workflows/nightly-koncur.yaml:
- Around line 30-34: Update the concurrency group expression in the nightly
workflow to include the caller workflow identifier and the logical target branch
from inputs.branch, rather than relying on github.ref. Preserve
cancel-in-progress behavior while ensuring release and main runs use distinct
concurrency keys.
- Around line 16-20: Add a boolean slack_report input with default true under
the workflow_dispatch.inputs configuration in the nightly workflow, matching the
existing workflow_call input so manual runs also provide inputs.slack_report and
trigger failure reporting correctly.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 62b1a0c9-b082-47bb-991b-ab7bdeceb914
📒 Files selected for processing (4)
.github/workflows/global-ci-bundle.yml.github/workflows/nightly-koncur.yamlkoncur-kantra/action.ymlkoncur-tackle-hub/action.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- koncur-tackle-hub/action.yml
- koncur-kantra/action.yml
Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
095fe91 to
c3895ea
Compare
The composite action receives os='windows' from the CI matrix but the firewall rule and *nix maven path conditions compared against 'windows-latest', causing both Windows and *nix maven settings steps to run on Windows and corrupting the MAVEN_SETTINGS_PATH. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
Revert composite action defaults to true (skip maven) and explicitly pass skip_maven: false from nightlies and through global-ci-bundle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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 @.github/workflows/global-ci-bundle.yml:
- Around line 600-604: Disable persisted credentials on the checkout steps in
global-ci-bundle.yml and koncur-kantra/action.yml by setting persist-credentials
to false. Add an explicit least-privilege permissions block to the
koncur-kantra-tests job, granting only the permissions it requires.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d9183e67-24a6-4fa2-8793-30678801d278
📒 Files selected for processing (4)
.github/workflows/global-ci-bundle.yml.github/workflows/nightly-koncur.yamlkoncur-kantra/action.ymlkoncur-tackle-hub/action.yml
Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
Separate the image fallback tag (ref) from the koncur repo checkout ref (koncur_ref) so callers can target different branches for images and tests independently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
775c5c2 to
fd742a4
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
koncur-tackle-hub/action.yml (1)
13-21: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRemove unsupported
typekeys from the action input definitions.
action.ymlinputs do not supporttype; only workflow inputs support it. Remove these keys fromskip_maven,image_pattern,ref, andkoncur_refso.github/workflows/global-ci-bundle.ymlcan parse the local action reference.Proposed fix
inputs: skip_maven: - type: boolean image_pattern: - type: string ref: - type: string koncur_ref: - type: string🤖 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 `@koncur-tackle-hub/action.yml` around lines 13 - 21, Remove the unsupported type keys from the action input definitions for skip_maven, image_pattern, ref, and koncur_ref in action.yml, while preserving their descriptions, required settings, defaults, and other input behavior.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@koncur-tackle-hub/action.yml`:
- Around line 13-21: Remove the unsupported type keys from the action input
definitions for skip_maven, image_pattern, ref, and koncur_ref in action.yml,
while preserving their descriptions, required settings, defaults, and other
input behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ebb2f52-4d7f-4869-9070-9ee35444b7c1
📒 Files selected for processing (6)
.github/workflows/e2e-hub-koncur.yaml.github/workflows/global-ci-bundle.yml.github/workflows/nightly-koncur-0.9.yaml.github/workflows/nightly-koncur.yamlkoncur-kantra/action.ymlkoncur-tackle-hub/action.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/nightly-koncur.yaml
7c8f4ac to
fd742a4
Compare
skip_maven reaches the koncur composite actions as a boolean when passed
as a YAML literal (nightly-koncur.yaml) but as a string when passed via
`${{ inputs.koncur_skip_maven }}` (global-ci-bundle.yml), because `${{ }}`
interpolation stringifies. `== 'false'` only matched the string form and
`!inputs.skip_maven` only worked for the boolean form, so one caller path
or the other always skipped maven setup and the maven tests failed.
Use `!fromJSON(inputs.skip_maven)`, which parses either form into a real
boolean, matching the existing fromJSON(inputs.skip_maven) usage in
koncur-kantra/action.yml.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Maayan Hadasi <mguetta@redhat.com>
Resolves #255
Summary by CodeRabbit
Improvements
Tests