Authorino manifests sync automatic worflow#328
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new GitHub Actions workflow is added for Authorino manifest synchronisation. It runs on dispatch or manual trigger, builds manifests, ignores ChangesAuthorino Manifests Sync Workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.github/workflows/sync-authorino-manifests.yaml (3)
44-47: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffMinor: Improve robustness of createdAt change detection.
The diff filter on line 47 uses
-I'^ createdAt:'to ignore timestamp-only changes. Whilst this works for standard YAML indentation, the pattern is fragile:
- Relies on exact 4-space indentation
- Matches
createdAt:anywhere in the diff, including in comments or nested structures- Would fail if YAML indentation changes or if createdAt appears at different depths
For robustness, consider using a dedicated diff tool or a more precise regex that accounts for YAML nesting (e.g., exclude diffs where only lines matching the pattern changed).
🤖 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 @.github/workflows/sync-authorino-manifests.yaml around lines 44 - 47, The git diff pattern `-I'^ createdAt:'` in the Check for meaningful changes step is fragile due to its reliance on exact indentation and inability to distinguish between actual changes and timestamp-only changes. Replace this overly simplistic pattern with a more robust approach that uses a proper regex to match createdAt fields accounting for YAML indentation variations, or consider using a dedicated filtering tool that can accurately detect whether the only changes are createdAt timestamp updates. The pattern should be anchored to YAML structure (accounting for variable indentation) and should ideally only consider diffs "meaningful" when non-createdAt lines actually differ from the original.
56-56: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMinor: Clarify author attribution for repository_dispatch triggers.
Line 56 uses
github.actorandgithub.actor_idto attribute commits. Forworkflow_dispatchevents (manual trigger), this correctly identifies the user. However, forrepository_dispatchevents (upstream automation),github.actoris the triggering workflow, not a real user, resulting in an incorrect email format.Consider explicitly handling the two trigger types:
- For
workflow_dispatch: use actual user actor- For
repository_dispatch: use a generic automation account (e.g.,kuadrant-automation@kuadrant.ioorgithub-actions[bot])This improves commit attribution clarity in the repository history.
🤖 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 @.github/workflows/sync-authorino-manifests.yaml at line 56, The author field on line 56 sets the git commit author using github.actor and github.actor_id directly, but this creates incorrect attribution for repository_dispatch events where github.actor is the triggering workflow name rather than a real user. Modify the author assignment to conditionally handle both trigger types: for workflow_dispatch events continue using the current github.actor format, but for repository_dispatch events use a generic automation account identifier such as kuadrant-automation@kuadrant.io or github-actions[bot] to accurately represent that the commit was created by an automated workflow.
42-42: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMinor: Validate authorinoVersion format before use.
The
authorinoVersioninput lacks format validation. Per the Makefile context, valid values are:
- Semantic version (X.Y.Z or X.Y.Z-prerelease)
- Git commit SHA
- Branch name
- Literal string
'latest'Invalid input (e.g., spaces, special characters) could cause make targets to fail silently or behave unexpectedly. Combine with the injection fix (above) to validate format:
^([0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?|[0-9a-f]{40}|[a-zA-Z0-9._-]+|latest)$.🤖 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 @.github/workflows/sync-authorino-manifests.yaml at line 42, The authorinoVersion input used in the make manifests bundle helm-build command lacks format validation, which could allow invalid values to be passed and cause silent failures. Add a validation step before the make command that checks if the authorinoVersion input (or 'latest' if not provided) matches the required format using the provided regex pattern: `^([0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?|[0-9a-f]{40}|[a-zA-Z0-9._-]+|latest)$`. If the format is invalid, the workflow should fail with a clear error message indicating what formats are accepted.
🤖 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/sync-authorino-manifests.yaml:
- Around line 40-43: The `run` step for the "Update manifests" job directly
interpolates the `github.event.inputs.authorinoVersion` into the shell command
without validation, creating a command injection vulnerability. Add input
validation before the make command to whitelist acceptable `authorinoVersion`
values (e.g., semantic version format or 'latest'), and reject any input that
contains special shell characters or unexpected patterns. Consider storing the
validated value in an environment variable with proper quoting before passing it
to the `make manifests bundle helm-build` command, or implement validation logic
that fails the workflow step if the input does not match the whitelist criteria.
- Around line 24-28: The checkout action in the workflow file is missing the
persist-credentials setting. Add persist-credentials: false to the with block of
the actions/checkout@v4 action to prevent the PAT token from being persisted in
the git config during the workflow session. This ensures the token is only used
for the specific checkout operation and reduces the risk of token leakage if the
workflow session is subsequently compromised.
- Line 25: The workflow uses semantic version tags for GitHub Actions which
creates supply-chain security risks since tags can be rewritten without warning.
Replace the three action references with their full commit SHA hashes: change
actions/checkout@v4 to pin to commit 34e114876b0b11c390a56381ad16ebd13914f8d5,
change actions/setup-go@v6 to pin to commit
4a3601121dd01d1626a1e23e37211e3254c1c06c, and change
peter-evans/create-pull-request@v7 to pin to commit
22a9089034f40e5a961c8808d113e2c98fb63676. This ensures the workflow always uses
the exact intended version of each action and prevents malicious or unintended
modifications.
---
Nitpick comments:
In @.github/workflows/sync-authorino-manifests.yaml:
- Around line 44-47: The git diff pattern `-I'^ createdAt:'` in the Check for
meaningful changes step is fragile due to its reliance on exact indentation and
inability to distinguish between actual changes and timestamp-only changes.
Replace this overly simplistic pattern with a more robust approach that uses a
proper regex to match createdAt fields accounting for YAML indentation
variations, or consider using a dedicated filtering tool that can accurately
detect whether the only changes are createdAt timestamp updates. The pattern
should be anchored to YAML structure (accounting for variable indentation) and
should ideally only consider diffs "meaningful" when non-createdAt lines
actually differ from the original.
- Line 56: The author field on line 56 sets the git commit author using
github.actor and github.actor_id directly, but this creates incorrect
attribution for repository_dispatch events where github.actor is the triggering
workflow name rather than a real user. Modify the author assignment to
conditionally handle both trigger types: for workflow_dispatch events continue
using the current github.actor format, but for repository_dispatch events use a
generic automation account identifier such as kuadrant-automation@kuadrant.io or
github-actions[bot] to accurately represent that the commit was created by an
automated workflow.
- Line 42: The authorinoVersion input used in the make manifests bundle
helm-build command lacks format validation, which could allow invalid values to
be passed and cause silent failures. Add a validation step before the make
command that checks if the authorinoVersion input (or 'latest' if not provided)
matches the required format using the provided regex pattern:
`^([0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?|[0-9a-f]{40}|[a-zA-Z0-9._-]+|latest)$`.
If the format is invalid, the workflow should fail with a clear error message
indicating what formats are accepted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e888807-c238-4b9e-b759-8d657c1b3072
📒 Files selected for processing (1)
.github/workflows/sync-authorino-manifests.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #328 +/- ##
=======================================
Coverage 57.20% 57.20%
=======================================
Files 13 13
Lines 1458 1458
=======================================
Hits 834 834
Misses 529 529
Partials 95 95
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| Synced by running `make manifests bundle helm-build AUTHORINO_VERSION=${{ github.event.inputs.authorinoVersion || 'latest' }}`. | ||
|
|
||
| [Recent Authorino manifest changes](https://github.com/Kuadrant/authorino/commits/main/install) |
There was a problem hiding this comment.
Wondering if we should use github.event.inputs.authorinoVersion here and avoid generating manifests for one version while pointing the link always to main.
Basically:
- if
github.event.inputs.authorinoVersion == 'latest'→git_ref = main - else →
git_ref = "v${{ github.event.inputs.authorinoVersion }}"
WDYT?
There was a problem hiding this comment.
It makes sense to change. However, I'm thinking now, if we should instead hard-code this action to work only with main branches instead. You think there is a case when merging manifests from an another authorino branch to the authorino-operator main could be useful?
There was a problem hiding this comment.
Other Authorino versions usually make sense in the release branches, where it's expected to be driven by the build.yaml file. IOW, only main branch is fine.
There was a problem hiding this comment.
It can only sync main branches now 👍
averevki
force-pushed
the
sync-manifests-workflow
branch
from
26fc993 to
2761bc3
Compare
Signed-off-by: averevki <sandyverevkin@gmail.com>
averevki
force-pushed
the
sync-manifests-workflow
branch
from
2761bc3 to
4747f37
Compare
3 participants
Adds a GitHub Actions workflow that automatically regenerates the AuthConfig CRD and related manifests when triggered by upstream changes in the authorino repo.
The workflow runs
make manifests bundle helm-buildand opens a PR with the updated files. It maintains a single rolling PR (sync/authorino-manifestsbranch), updating it on each trigger. Manifest updates with only acreatedAttimestamp change are skipped.Triggered by:
repository_dispatchfrom authorino sync-operator-manifests (Add authorino-operator manifests sync triggering workflow authorino#640) workflowworkflow_dispatchwith optionalauthorinoVersioninputSummary by CodeRabbit