Skip to content

[ci] Get REGISTRY from vars, not secrets - #1423

Merged
Timofei Larkin (lllamnyp) merged 1 commit into
mainfrom
ci/use-var-for-registry
Sep 16, 2025
Merged

Timofei Larkin (lllamnyp) merged 1 commit into
mainfrom
ci/use-var-for-registry

Conversation

@lllamnyp

@lllamnyp Timofei Larkin (lllamnyp) commented Sep 16, 2025 •

Copy link
Copy Markdown
Member

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

[ci] Source the REGISTRY env var from actions' variables, not secrets,
so external pull requests can work.

Summary by CodeRabbit

  • Chores
    • Updated CI configuration to source the container registry setting from organization variables instead of secrets, improving maintainability and visibility of build settings.
    • No impact to application features, functionality, or performance; builds and deployments continue to operate as before.
    • No action required from users or admins; this is an internal workflow refinement.

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]>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented Sep 16, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Changed 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

Cohort / File(s) Summary
CI workflow config
\.github/workflows/pull-requests.yaml
Updated REGISTRY env source from secrets.OCIR_REPO to vars.OCIR_REPO; no control-flow or structural changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

I twitch my nose at YAML’s tune,
A tiny tweak beneath the moon—
From secrets’ burrow to vars we hop,
Same carrots, different veggie shop.
Pipelines purr, no steps askew,
A tidy hop, then we’re through. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "[ci] Get REGISTRY from vars, not secrets" succinctly and accurately summarizes the primary change in this PR: updating the CI workflow to source the REGISTRY env var from Actions variables instead of secrets. It is specific to the CI change, clear to reviewers, and directly matches the PR objectives and the workflow file modification. A teammate scanning history will understand the main intent without needing additional context.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ci/use-var-for-registry

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 PRs

make 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 ?= 1 and set env: PUSH: '0' (or run make 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

📥 Commits

Reviewing files that changed from the base of the PR and between 87b2316 and 07384c3.

📒 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

Comment on lines 3 to +4
env:
REGISTRY: ${{ secrets.OCIR_REPO }}
REGISTRY: ${{ vars.OCIR_REPO }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 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
+          fi

To 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"
fi

Length 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.

Suggested change
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.

@lllamnyp
Timofei Larkin (lllamnyp) deleted the ci/use-var-for-registry branch September 16, 2025 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants