Use build.yaml for release configuration in CI workflows#325
Use build.yaml for release configuration in CI workflows#325guicassolato wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe PR adds Changesbuild.yaml-driven build configuration
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: 6
🤖 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/build-images-branches.yaml:
- Line 53: Replace the blanket `secrets: inherit` statement with explicit secret
mapping that only passes the specific secrets required by the reusable workflow.
Instead of inheriting all available secrets, use the `secrets` key to map
individual secrets by name (such as `SECRET_NAME: ${{ secrets.SECRET_NAME }}`)
for each secret that the downstream workflow actually needs, following the
principle of least privilege.
- Around line 11-13: The detect-config job lacks an explicit permissions block,
which causes it to inherit broad default token scopes. Add a permissions: block
at the job level under detect-config that specifies only the minimal required
permissions for this workflow's operations. Review what actions and steps the
detect-config job performs to determine which specific permissions it actually
needs (such as contents, id-token, etc.) and declare only those in the
permissions block, following the principle of least privilege.
- Around line 21-22: The actions/checkout step is using a version tag reference
without a commit SHA, which weakens supply-chain security. Update the checkout
action to pin to a specific commit SHA instead of `@v4`, and add the
persist-credentials option set to false since this step only performs a
read-only checkout operation that does not require credential persistence.
- Around line 33-34: The CHANNELS and DEFAULT_CHANNEL environment variables in
the build-images-branches.yaml workflow are hardcoded to "stable" instead of
being dynamically read from the build.yaml configuration file. To fix this,
replace the hardcoded string values for CHANNELS and DEFAULT_CHANNEL with
dynamic values that are parsed and extracted from the build.yaml configuration
file, ensuring that channel configuration is actually driven by the config file
rather than bypassing it with static values.
- Around line 30-32: The grep and sed pipeline in the OPERATOR_VERSION,
AUTHORINO_VERSION, and AUTHORINO_IMAGE variable assignments succeeds with empty
output when fields are missing, preventing the fallback values from being
applied. Restructure each variable assignment to explicitly check if the
extracted value is empty and use the fallback only when the result is empty. For
example, use parameter expansion or a conditional check to ensure that when grep
finds no matching version key, authorinoVersion key, or authorinoImage key in
the BUILD_FILE, the corresponding default values (0.0.0, latest, and
quay.io/kuadrant/authorino:latest) are used instead of empty strings.
In `@Makefile`:
- Around line 35-36: The VERSION variable is being assigned on line 35 even when
BUILD_CONFIG_VERSION is empty, which prevents the fallback to git SHA on line 36
from executing since VERSION is already set. Replace the two separate VERSION
assignments with a single assignment that conditionally uses
BUILD_CONFIG_VERSION if it is not empty, otherwise falls back to the git SHA
using make's conditional syntax such as the if function, for example: VERSION ?=
$(if $(BUILD_CONFIG_VERSION),$(BUILD_CONFIG_VERSION),$(shell git rev-parse
HEAD)).
🪄 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: ad1a5602-d98a-4597-a687-bb1ceb100d80
📒 Files selected for processing (2)
.github/workflows/build-images-branches.yamlMakefile
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #325 +/- ##
=======================================
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:
|
guicassolato
force-pushed
the
use-build-yaml-for-release-config
branch
from
b5fcb9e to
be0c3ab
Compare
Replaces the make/release.mk approach with reading configuration from build.yaml. This makes build.yaml the single source of truth for release configuration. Changes: - Adds 'version' and 'authorinoVersion' fields to build.yaml - Makefile reads VERSION, AUTHORINO_VERSION, and CHANNELS from build.yaml if present - Uses simple grep/sed parsing to avoid yq dependency at variable definition time - Removes make/release.mk (no longer needed) - Falls back to defaults (git SHA, 'latest') if build.yaml doesn't exist This allows release branches to automatically use the correct versions from build.yaml for 'make verify-manifests' and 'make verify-bundle' in CI, while dev branches without build.yaml continue to work with default values. Signed-off-by: Guilherme Cassolato <guicassolato@gmail.com>
- Fix VERSION fallback logic in Makefile to properly fall back to git SHA when build.yaml is missing - Add permissions blocks for least privilege access (contents: read) - Pin actions/checkout to commit SHA (v4.2.2) and disable persist-credentials - Fix fallback logic for missing fields in build.yaml using parameter expansion - Read channels and defaultChannel from build.yaml instead of hardcoding to 'stable' - Replace 'secrets: inherit' with explicit secret mapping (IMG_REGISTRY_USERNAME, IMG_REGISTRY_TOKEN) - Add comment explaining why only registry secrets are passed Signed-off-by: Guilherme Cassolato <guicassolato@gmail.com>
guicassolato
force-pushed
the
use-build-yaml-for-release-config
branch
from
be0c3ab to
7a17cac
Compare
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/build-images-branches.yaml:
- Around line 69-71: The reusable workflow build-images-base.yaml is missing the
secret declarations for IMG_REGISTRY_USERNAME and IMG_REGISTRY_TOKEN that are
being passed from build-images-branches.yaml. Add a secrets section under
on.workflow_call in build-images-base.yaml that declares both
IMG_REGISTRY_USERNAME and IMG_REGISTRY_TOKEN, marking each as required: true, so
the workflow properly validates and accepts these secrets from the caller.
🪄 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: 32770a71-896a-4059-b2ae-7f8be3c8a5be
📒 Files selected for processing (2)
.github/workflows/build-images-branches.yamlMakefile
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
| secrets: | ||
| IMG_REGISTRY_USERNAME: ${{ secrets.IMG_REGISTRY_USERNAME }} | ||
| IMG_REGISTRY_TOKEN: ${{ secrets.IMG_REGISTRY_TOKEN }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking reusable workflow secret declarations..."
fd -a "build-images-base.yaml" .github/workflows --exec sed -n '1,220p' {}
echo
echo "Looking for workflow_call secret declarations..."
rg -n -C3 'workflow_call|secrets:|IMG_REGISTRY_USERNAME|IMG_REGISTRY_TOKEN' .github/workflows/build-images-base.yamlRepository: Kuadrant/authorino-operator
Length of output: 9680
Reusable-workflow secret contract is broken.
The build-images-base.yaml workflow uses IMG_REGISTRY_USERNAME and IMG_REGISTRY_TOKEN secrets in its jobs, but does not declare them under on.workflow_call.secrets. When the caller passes these secrets (lines 69–71), the workflow will fail validation or at runtime.
Add the following to build-images-base.yaml under on.workflow_call:
Suggested fix
on:
workflow_call:
inputs:
# ... existing inputs ...
secrets:
IMG_REGISTRY_USERNAME:
required: true
IMG_REGISTRY_TOKEN:
required: true🧰 Tools
🪛 actionlint (1.7.12)
[error] 70-70: secret "IMG_REGISTRY_USERNAME" is not defined in "./.github/workflows/build-images-base.yaml" reusable workflow. no secret is defined
(workflow-call)
[error] 71-71: secret "IMG_REGISTRY_TOKEN" is not defined in "./.github/workflows/build-images-base.yaml" reusable workflow. no secret is defined
(workflow-call)
🤖 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/build-images-branches.yaml around lines 69 - 71, The
reusable workflow build-images-base.yaml is missing the secret declarations for
IMG_REGISTRY_USERNAME and IMG_REGISTRY_TOKEN that are being passed from
build-images-branches.yaml. Add a secrets section under on.workflow_call in
build-images-base.yaml that declares both IMG_REGISTRY_USERNAME and
IMG_REGISTRY_TOKEN, marking each as required: true, so the workflow properly
validates and accepts these secrets from the caller.
Source: Linters/SAST tools
|
I am going to say hold off on this for a little bit. I like the idea, and there is currently a RFC looking to standardize this across the different repo. It is planned to be talked about in the community call on June 23rd. Here is a link to the RFC Kuadrant/limitador#494 , and here is a link to a draft PR I have in autorino, Kuadrant/authorino#634, it still needs a bit of testing, but the layout would be the same. |
|
This is partially covering #329. |
5 participants
Problem
The
build-images-branches.yamlworkflow currently passes hardcoded values (VERSION=0.0.0,AUTHORINO_VERSION=latest) to the base workflow. This causes issues for branches targeting release branches (e.g., backport branches for patch releases) because:make verify-manifestsandmake verify-bundlefail in CI because manifests are generated with different versions than what's committed0.0.0andlatestSolution
Enable the Makefile and CI workflows to read release configuration from
build.yamlwhen present:build.yaml): Falls back to defaults (0.0.0,latest) - no change in behaviorbuild.yaml): Automatically uses the correct versions frombuild.yamlChanges
Makefile
VERSION,AUTHORINO_VERSION, andCHANNELSfrombuild.yamlif file existsbuild.yamldoesn't existGitHub Workflow (build-images-branches.yaml)
detect-configjob to extract configuration frombuild.yamlbuild-images-baseworkflow0.0.0/latestwhenbuild.yamlnot foundAlternative Considered
Commit
build.yamlin main with default values (VERSION=0.0.0,AUTHORINO_VERSION=latest, etc.) and simplify the Makefile to always read from the build file without fallback logic.This alternative was discarded for now to make the change less disruptive -
build.yamlcontinues to only exist in release branches. However, we're open to revisiting this decision in the future if having a committedbuild.yamlin main proves beneficial.Benefits
build.yamlto set correct versions automaticallyTesting
build.yamldoesn't existDeveloped while preparing patch releases v0.23.3, v0.24.1, and v0.25.1
Contains commits of #326, which should be merged first for a clean history.
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
build.yamlwhen present, including versioning and release channel defaults, with sensible fallbacks when the file is missing.build.yaml, defaulting channels tostablewhen unset.