Repository navigation
[ci] Get REGISTRY from vars, not secrets - #1423
Conversation
This patch sources the REGISTRY env var from GitHub actions variables instead of secrets, so pull requests from forked repos work correctly. ```release-note [ci] Source the REGISTRY env var from actions' variables, not secrets, so external pull requests can work. ``` Signed-off-by: Timofei Larkin <[email protected]>
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
WalkthroughChanged the REGISTRY environment variable in .github/workflows/pull-requests.yaml to reference vars.OCIR_REPO instead of secrets.OCIR_REPO. No other workflow steps, jobs, or logic were modified. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pull-requests.yaml (1)
51-58: Prevent image pushes on forked PRsmake build invokes package image targets that tag to
$(REGISTRY) and CI defaults to pushing (scripts/common-envs.mk sets REGISTRY ?= ghcr.io/cozystack/cozystack and PUSH := 1). packages/core/installer/Makefile performs a skopeo copy to docker://$ (REGISTRY) (line 38).
- Action: Gate the Build step in .github/workflows/pull-requests.yaml (lines 51–58) with if: ${{ !github.event.pull_request.head.repo.fork }} OR
- Action: Make pushes overridable and disable for PRs: change scripts/common-envs.mk
PUSH := 1→PUSH ?= 1and set env: PUSH: '0' (or runmake build PUSH=0) for PR runs.- Evidence to inspect: top-level Makefile build target (calls package image targets, Makefile lines ~10–28), scripts/common-envs.mk (lines 1,3), packages/core/installer/Makefile (skopeo copy at line 38).
🧹 Nitpick comments (1)
.github/workflows/pull-requests.yaml (1)
41-48: Rename step: it logs into OCIR, not GHCR.Title is misleading; registry is iad.ocir.io.
- - name: Login to GitHub Container Registry + - name: Login to Oracle Cloud Container Registry (OCIR)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-requests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
| env: | ||
| REGISTRY: ${{ secrets.OCIR_REPO }} | ||
| REGISTRY: ${{ vars.OCIR_REPO }} |
There was a problem hiding this comment.
🧩 Analysis chain
Switch to vars is correct; add guards for unset REGISTRY and a safe fork default.
Good move for forks. To prevent empty REGISTRY causing odd tags/push attempts, validate it for non‑fork runs and set a harmless default for forks.
Apply:
env:
- REGISTRY: ${{ vars.OCIR_REPO }}
+ REGISTRY: ${{ vars.OCIR_REPO }}
@@
build:
@@
steps:
- name: Checkout code
uses: actions/checkout@v4
@@
+ - name: Default REGISTRY for forks
+ if: ${{ github.event.pull_request.head.repo.fork }}
+ run: echo "REGISTRY=local.invalid" >> "$GITHUB_ENV"
+
+ - name: Validate REGISTRY for non‑fork runs
+ if: ${{ !github.event.pull_request.head.repo.fork }}
+ run: |
+ if [ -z "${REGISTRY}" ]; then
+ echo "REGISTRY is not set. Define repo/org variable OCIR_REPO."; exit 1
+ fiTo verify presence of the variable in the repo/org:
🏁 Script executed:
#!/bin/bash
# Check for OCIR_REPO at repo/org level
set -euo pipefail
echo "Repo variables:" >&2
gh api repos/${GITHUB_REPOSITORY}/actions/variables | jq -r '.variables[].name' | grep -qx OCIR_REPO || echo "OCIR_REPO not in repo vars"
echo "Org variables:" >&2
owner=$(cut -d/ -f1 <<< "${GITHUB_REPOSITORY}")
gh api orgs/${owner}/actions/variables | jq -r '.variables[].name' | grep -qx OCIR_REPO || echo "OCIR_REPO not in org vars"Length of output: 167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Determine repository (prefer GITHUB_REPOSITORY, fall back to git remote)
repo="${GITHUB_REPOSITORY-}"
if [ -z "$repo" ]; then
url=$(git remote get-url origin 2>/dev/null || true)
if [ -z "$url" ]; then
echo "ERROR: Cannot determine repository (GITHUB_REPOSITORY unset and no git remote origin)" >&2
exit 0
fi
if [[ "$url" =~ ^git@ ]]; then
repo=$(echo "$url" | sed -E 's|^[^:]+:(.+/[^/]+)(\.git)?$|\1|')
else
repo=$(echo "$url" | sed -E 's|https?://[^/]+/([^/]+/[^/]+)(\.git)?$|\1|')
fi
fi
echo "Repository: $repo"
owner=$(cut -d/ -f1 <<< "$repo")
repo_name=$(cut -d/ -f2 <<< "$repo")
# Ensure gh CLI available
if ! command -v gh >/dev/null 2>&1; then
echo "gh CLI not found; cannot query Actions variables from this environment." >&2
echo "Run locally:" >&2
echo " gh variable list -R ${owner}/${repo_name} | jq -r '.[] | .name' | grep -x OCIR_REPO || echo OCIR_REPO not in repo vars" >&2
echo " gh variable list --org ${owner} | jq -r '.[] | .name' | grep -x OCIR_REPO || echo OCIR_REPO not in org vars" >&2
exit 0
fi
# Check repo variables
echo "Repo variables:"
if gh variable list -R "${owner}/${repo_name}" --jq '.[] | .name' 2>/dev/null | grep -qx OCIR_REPO; then
echo "OCIR_REPO present in repo vars"
else
echo "OCIR_REPO not in repo vars"
fi
# Check org variables
echo "Org variables:"
if gh variable list --org "${owner}" --jq '.[] | .name' 2>/dev/null | grep -qx OCIR_REPO; then
echo "OCIR_REPO present in org vars"
else
echo "OCIR_REPO not in org vars"
fiLength of output: 119
Add guards for unset REGISTRY and default for forks — OCIR_REPO missing in repo/org
OCIR_REPO was verified absent at both repo and org levels; add a harmless fork default and fail-fast validation for non-fork runs.
File: .github/workflows/pull-requests.yaml
Apply:
env:
- REGISTRY: ${{ vars.OCIR_REPO }}
+ REGISTRY: ${{ vars.OCIR_REPO }}
@@
build:
@@
steps:
- name: Checkout code
uses: actions/checkout@v4
@@
+ - name: Default REGISTRY for forks
+ if: ${{ github.event.pull_request.head.repo.fork }}
+ run: echo "REGISTRY=local.invalid" >> "$GITHUB_ENV"
+
+ - name: Validate REGISTRY for non‑fork runs
+ if: ${{ !github.event.pull_request.head.repo.fork }}
+ run: |
+ if [ -z "${REGISTRY}" ]; then
+ echo "REGISTRY is not set. Define repo/org variable OCIR_REPO."; exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: | |
| REGISTRY: ${{ secrets.OCIR_REPO }} | |
| REGISTRY: ${{ vars.OCIR_REPO }} | |
| env: | |
| REGISTRY: ${{ vars.OCIR_REPO }} | |
| jobs: | |
| build: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Default REGISTRY for forks | |
| if: ${{ github.event.pull_request.head.repo.fork }} | |
| run: echo "REGISTRY=local.invalid" >> "$GITHUB_ENV" | |
| - name: Validate REGISTRY for non-fork runs | |
| if: ${{ !github.event.pull_request.head.repo.fork }} | |
| run: | | |
| if [ -z "${REGISTRY}" ]; then | |
| echo "REGISTRY is not set. Define repo/org variable OCIR_REPO." | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In .github/workflows/pull-requests.yaml around lines 3-4, the workflow assumes
REGISTRY is set from vars.OCIR_REPO which is missing; add a harmless default for
forked PRs and a fail-fast validation for non-fork runs: set REGISTRY to a safe
placeholder when vars.OCIR_REPO is empty for forked repositories (so forks don’t
break CI), and add an early workflow/job step that checks if the event is not
from a fork and REGISTRY is unset/placeholder, then fail with a clear error
instructing to set vars.OCIR_REPO or repository secret; ensure the check uses
github.event.pull_request.head.repo.fork to distinguish forks and aborts before
any publish steps.
What this PR does
This patch sources the REGISTRY env var from GitHub actions variables instead of secrets, so pull requests from forked repos work correctly.
Release note
Summary by CodeRabbit