chore: multi-API repo cleanup + provider-upjet-zitadel dep#6
Conversation
📝 WalkthroughWalkthroughModernizes CI and build tooling: upgrades reusable workflows to hops-ops v3.0.0 (input shape changes), refactors Makefile to render/validate per-example API dirs, consolidates example secrets, updates docs, and converts composition tests to untyped xr inputs. ChangesAuth Stack Project Modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
patrickleet
force-pushed
the
feat/multi-api-cleanup
branch
from
a812c39 to
569d9b4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
.github/workflows/on-pr.yaml (1)
51-51: ⚡ Quick winScope secrets explicitly instead of unconditional inheritance.
secrets: inheritpasses all available secrets to the called workflow. Prefer explicit secret mapping for least privilege.🤖 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/on-pr.yaml at line 51, Replace the unconditional "secrets: inherit" in the workflow with an explicit secrets mapping: identify which pipeline secrets are required by the called workflow (e.g., API keys, tokens used by the job) and replace "secrets: inherit" with a mapping that passes only those, using the secrets context (for example map SECRET_NAME: ${{ secrets.SECRET_NAME }} for each needed secret) so only the minimal required secrets are forwarded to the called workflow.upbound.yaml (1)
22-22: ⚡ Quick winConsider bounding the provider version range for reproducibility.
>=v0.1.1allows unbounded upgrades. Prefer a bounded compatible range (or exact pin) to avoid accidental breaking changes during resolution.🤖 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 `@upbound.yaml` at line 22, The provider version field "version" in upbound.yaml is unbounded (">=v0.1.1"); change it to a bounded compatible range or exact pin to prevent accidental upgrades — e.g., use a caret/compatible or upper-bounded range such as ">=v0.1.1 <v1.0.0" or pin to "v0.1.1" so resolution is reproducible; update the version value in upbound.yaml accordingly.
🤖 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/on-pr.yaml:
- Line 30: Replace the floating tag references to the reusable workflows with
immutable commit SHAs: for each uses:
hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0 (and the
other occurrences at the same file), update the `@v3.0.0` suffix to the specific
commit SHA of the target repository; optionally add a trailing comment
indicating the intended tag (e.g., // intended-tag: v3.0.0) for clarity. Locate
the three uses lines in .github/workflows/on-pr.yaml and swap the tag for the
full commit SHA so the workflow points to an immutable ref.
In @.github/workflows/on-push-main.yaml:
- Line 26: The workflow uses: references are tag-pinned
("hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0") and
must be locked to immutable commit SHAs; for each occurrence (the three uses:
the one shown and the ones at the other reported lines) replace the tag ref
(`@v3.0.0`) with the exact commit SHA from the upstream repository (e.g.,
`@abcdef1234567890`...), fetching the canonical commit ID from the
hops-ops/workflows-crossplane repo and updating the three uses: entries so they
reference that SHA instead of the tag.
In `@Makefile`:
- Line 102: The Makefile currently only marks "render" and "validate" as .PHONY
which risks file-name collisions; update the .PHONY declaration to include all
operational targets — add clean, build, test, e2e, and publish alongside render
and validate — so the .PHONY line contains: clean, build, test, e2e, publish,
render, validate (refer to the .PHONY declaration and the target names clean,
build, test, e2e, publish, render, validate when making the change).
In `@README.md`:
- Line 3: Update the README top-level description to match the later section
that says these primitives are not wrapped here: change the sentence that
currently begins "Installs Zitadel into a Kubernetes cluster..." which lists
"Organization, Project, HumanUser, MachineUser, Grant, IDP" as being hosted, to
explicitly state that Zitadel is installed as the platform IdP and that the
primitive XRDs for Organization and Project (and the other listed primitives)
are NOT wrapped/hosted in this repo but are composed/consumed against the
installed Zitadel; ensure the wording mirrors the later section's phrasing about
not wrapping Organization/Project so the two descriptions are consistent.
In `@upbound.yaml`:
- Around line 25-26: Update the project description in upbound.yaml to match the
README’s primitive scope by removing or correcting the mention of Organization
and Project (which are not wrapped here); ensure the description only lists the
actual wrapped primitives (e.g., HumanUser, MachineUser, Grant, IDP or the set
documented in README) so the text accurately reflects the implemented XRDs.
---
Nitpick comments:
In @.github/workflows/on-pr.yaml:
- Line 51: Replace the unconditional "secrets: inherit" in the workflow with an
explicit secrets mapping: identify which pipeline secrets are required by the
called workflow (e.g., API keys, tokens used by the job) and replace "secrets:
inherit" with a mapping that passes only those, using the secrets context (for
example map SECRET_NAME: ${{ secrets.SECRET_NAME }} for each needed secret) so
only the minimal required secrets are forwarded to the called workflow.
In `@upbound.yaml`:
- Line 22: The provider version field "version" in upbound.yaml is unbounded
(">=v0.1.1"); change it to a bounded compatible range or exact pin to prevent
accidental upgrades — e.g., use a caret/compatible or upper-bounded range such
as ">=v0.1.1 <v1.0.0" or pin to "v0.1.1" so resolution is reproducible; update
the version value in upbound.yaml accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 522c9e06-de44-4531-b7b4-adabf3380abf
📒 Files selected for processing (6)
.github/workflows/on-pr.yaml.github/workflows/on-push-main.yamlMakefileREADME.mdapis/authstacks/configuration.yamlupbound.yaml
💤 Files with no reviewable changes (1)
- apis/authstacks/configuration.yaml
| jobs: | ||
| validate: | ||
| uses: unbounded-tech/workflows-crossplane/.github/workflows/validate.yaml@v2.20.0 | ||
| uses: hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/on-pr.yamlRepository: hops-ops/auth-stack
Length of output: 1729
Pin reusable workflow references to immutable commit SHAs.
Using @v3.0.0 leaves these jobs vulnerable to tag retargeting. Pin to full commit SHAs and optionally annotate the intended tag in a comment.
🔒 Suggested hardening
- uses: hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0
+ uses: hops-ops/workflows-crossplane/.github/workflows/validate.yaml@<sha-for-v3.0.0> # v3.0.0
...
- uses: hops-ops/workflows-crossplane/.github/workflows/test.yaml@v3.0.0
+ uses: hops-ops/workflows-crossplane/.github/workflows/test.yaml@<sha-for-v3.0.0> # v3.0.0
...
- uses: hops-ops/workflows-crossplane/.github/workflows/e2e.yaml@v3.0.0
+ uses: hops-ops/workflows-crossplane/.github/workflows/e2e.yaml@<sha-for-v3.0.0> # v3.0.0
...
- uses: hops-ops/workflows-crossplane/.github/workflows/publish.yaml@v3.0.0
+ uses: hops-ops/workflows-crossplane/.github/workflows/publish.yaml@<sha-for-v3.0.0> # v3.0.0Also applies to: 41-41, 44-44, 50-50
🧰 Tools
🪛 zizmor (1.25.2)
[error] 30-30: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/on-pr.yaml at line 30, Replace the floating tag references
to the reusable workflows with immutable commit SHAs: for each uses:
hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0 (and the
other occurrences at the same file), update the `@v3.0.0` suffix to the specific
commit SHA of the target repository; optionally add a trailing comment
indicating the intended tag (e.g., // intended-tag: v3.0.0) for clarity. Locate
the three uses lines in .github/workflows/on-pr.yaml and swap the tag for the
full commit SHA so the workflow points to an immutable ref.
| jobs: | ||
| validate: | ||
| uses: unbounded-tech/workflows-crossplane/.github/workflows/validate.yaml@v2.20.0 | ||
| uses: hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files ".github/workflows/*.yaml" | head -20Repository: hops-ops/auth-stack
Length of output: 169
🏁 Script executed:
cat -n .github/workflows/on-push-main.yamlRepository: hops-ops/auth-stack
Length of output: 1741
🏁 Script executed:
# Check if hops-ops/workflows-crossplane is a real repo and what tags/commits are available
web_search
GitHub Actions best practices: commit SHA vs tag pinningRepository: hops-ops/auth-stack
Length of output: 159
🌐 Web query:
GitHub Actions security best practices tag vs commit SHA pinning 2024 2025
💡 Result:
In the context of GitHub Actions security, pinning to a full-length commit SHA is the industry-standard best practice for ensuring immutability and mitigating supply chain attacks [1][2][3]. Key differences and security implications: Commit SHA Pinning (Recommended) - Immutability: A commit SHA uniquely identifies a specific state of the repository, ensuring that the exact same code runs every time [2][4][3]. - Security: It protects against tag hijacking, where an attacker compromises a repository or maintainer account to move a version tag (e.g., v1) to a malicious commit [5][6][3]. - Implementation: Use the full-length commit SHA (e.g., actions/checkout@b4ffde6) [1][2]. - Maintenance: To maintain readability and facilitate updates, it is best practice to include the version tag as a comment (e.g., uses: actions/checkout@b4ffde6 # v4.1.1) [5][7][4]. Tag-Based Pinning (Convenient but Risky) - Mutability: Version tags (e.g., v1, v1.2) are mutable references in Git [3][8]. They can be deleted and re-created or force-pushed to point to different commits, making them vulnerable to supply chain compromise [6][3][9]. - Trust: Pinning to a tag should only be done if you fully trust the creator [1]. Even then, it remains susceptible to account takeovers or repository compromises [5][1]. Best Practices for 2025-2026 - Enforce SHA Pinning: Organizations can now configure policies at the repository or organization level to require that all actions be pinned to a full-length commit SHA [1][4][10]. - Automated Updates: Use tools like Dependabot or Renovate to manage updates for pinned actions [5][7][11]. These tools can automatically open pull requests to update SHAs when new versions are released, balancing security with maintenance [4][3]. - Blocklist Compromised Actions: GitHub allows organizations to block specific actions or versions using the! prefix (e.g.,!compromised-org/action) to respond rapidly to security incidents [4][10]. - Defense in Depth: Pinning is one layer of security. Other critical practices include minimizing secret exposure, using OIDC for authentication, auditing high-privilege workflow triggers (e.g., pull_request_target), and restricting self-hosted runners [12][13][10].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 3: https://blog.rafaelgss.dev/why-you-should-pin-actions-by-commit-hash
- 4: https://dev.to/ciberconscientes/github-actions-why-you-should-pin-your-actions-to-a-specific-version-1513
- 5: Pin GitHub Actions to commit SHAs in release workflow github/gh-aw#10021
- 6: https://gbhackers.com/compromised-github-action/
- 7: Pin all GitHub Actions to immutable SHA commits for supply chain security github/gh-aw#5691
- 8: https://endjin.com/blog/2020/10/the-importance-of-specific-and-immutable-dependency-versions
- 9: https://dev.to/nkuziev-sec/why-fixed-container-image-versions-matter-lessons-from-the-trivy-supply-chain-attack-29ke
- 10: https://www.wiz.io/blog/github-actions-security-guide
- 11: https://docs.github.com/actions/learn-github-actions/security-hardening-for-github-actions
- 12: https://www.aikido.dev/blog/checklist-github-actions
- 13: https://docs.github.com/en/actions/concepts/security/compromised-runners
Pin reusable workflows by commit SHA.
These uses: refs are tag-pinned, not commit-pinned. Switch to immutable SHAs to prevent tag hijacking and supply chain attacks.
Also applies to: 37-37, 40-40
🧰 Tools
🪛 zizmor (1.25.2)
[error] 26-26: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/on-push-main.yaml at line 26, The workflow uses:
references are tag-pinned
("hops-ops/workflows-crossplane/.github/workflows/validate.yaml@v3.0.0") and
must be locked to immutable commit SHAs; for each occurrence (the three uses:
the one shown and the ones at the other reported lines) replace the tag ref
(`@v3.0.0`) with the exact commit SHA from the upstream repository (e.g.,
`@abcdef1234567890`...), fetching the canonical commit ID from the
hops-ops/workflows-crossplane repo and updating the three uses: entries so they
reference that SHA instead of the tag.
569d9b4 to
b087ee1
Compare
Published Crossplane PackageThe following Crossplane package was published as part of this PR: Package: ghcr.io/hops-ops/auth-stack:pr-6-9b274f53e440eba0508640c1673f440d1b5288db |
patrickleet
force-pushed
the
feat/multi-api-cleanup
branch
from
b087ee1 to
501b7e2
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/on-pr.yaml:
- Line 51: Replace the broad "secrets: inherit" usage in the reusable workflow
call with a least-privilege mapping: either set "secrets: {}" to pass no
secrets, or explicitly map only the needed secret(s) (e.g., map GH_PAT only when
ghcr_user / other relevant inputs are provided). Update the call site that
currently includes "secrets: inherit" so it conditionally supplies GH_PAT (or an
explicit empty object) based on the publish job inputs, ensuring no other
repository secrets are forwarded.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88a27443-9289-4ede-aa22-878d45082d13
📒 Files selected for processing (8)
.github/workflows/on-pr.yaml.github/workflows/on-push-main.yamlMakefileREADME.mdapis/authstacks/configuration.yamlexamples/authstacks/local-colima.yamltests/test-render/main.ktests/test-render/model
💤 Files with no reviewable changes (1)
- apis/authstacks/configuration.yaml
✅ Files skipped from review due to trivial changes (1)
- tests/test-render/model
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Three structural changes preparing auth-stack to host additional XRDs in the future, plus three fixes for pre-existing CI failures. Structural: - Makefile + CI workflows refactored to multi-API. Mirror psql-stack's api-dir macro and hops-ops/workflows-crossplane@v3.0.0 per-example api_path. render:all / validate:all derive the api dir from each example path; single-XRD targets still work for examples/authstacks/. - Drop apis/authstacks/configuration.yaml from git — already matched by .gitignore's apis/**/configuration.yaml; the tracked copy was a stale leftover from initial commit. Build regenerates it. - README rewrites the "Auth-group primitives" section to describe the intended scope (HumanUser, MachineUser, Grant, IDP planned; Organization + Project intentionally not wrapped — Tenant composes Zitadel MRs directly). Fixes (pre-existing on main, not regressions from this PR): - examples/authstacks/local-colima.yaml: spec.externalSecrets.masterkey was the old nested-per-property shape from before b61b74b ("collapse externalSecrets to a single secretPath"). Replaced with the current flat shape (single secretPath into an AWS SM JSON blob containing masterkey + admin-password). - tests/test-render/: was missing the `model` symlink to ../../.up/kcl/models per the test-model-symlinks convention. Without it, KCL ran into a goroutine-stack overflow on schema dep resolution. Adding the symlink fixes it (model symlink is gitignored per .gitignore's `.up/` rule; the symlink target itself is tracked). - tests/test-render/main.k: rewrote 13 typed-import xr literals to dict literals (matches observe-stack/crossplane-stack convention). Also added missing metadata.namespace = "zitadel" to psql-cluster-ref-pins-existing-database test — the test asserts the Postgres Host string uses zitadel ns, and psqlClusterRef.namespace defaults to metadata.namespace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
patrickleet
force-pushed
the
feat/multi-api-cleanup
branch
from
501b7e2 to
9729d7c
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)
Makefile (1)
42-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
set -euo pipefailto background subshells in bothrender:allandvalidate:alltargets.Failures inside background subshells are masked because the subshells end with
echo ""without enabling fail-fast. Both theup composition rendercommands (line 42-52) and the validation pipelines (lines 75-89) can fail silently, causing the Makefile to report success even when rendering or validation actually failed.Suggested fix
@@ ( \ + set -euo pipefail; \ if [ -n "$$observed" ]; then \ @@ - fi; \ - echo "" \ + fi; \ + echo ""; \ ) > "$$outfile" 2>&1 & \Apply to both locations: lines 42-52 (
render:all) and lines 75-89 (validate:all).🤖 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 `@Makefile` around lines 42 - 52, The background subshells used by the render:all and validate:all targets can hide failures because they don't enable fail-fast; update the subshells that run the up composition render commands (the block around up composition render --xrd=... and the conditional using $$observed) and the validation pipeline subshells to start with a strict shell option (e.g., add set -euo pipefail at the top of each parenthesized subshell) so any command failure causes the subshell to exit non‑zero and surface the error to make the Makefile fail; ensure you add this to both the render:all and validate:all background subshell blocks.
🤖 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 `@Makefile`:
- Around line 42-52: The background subshells used by the render:all and
validate:all targets can hide failures because they don't enable fail-fast;
update the subshells that run the up composition render commands (the block
around up composition render --xrd=... and the conditional using $$observed) and
the validation pipeline subshells to start with a strict shell option (e.g., add
set -euo pipefail at the top of each parenthesized subshell) so any command
failure causes the subshell to exit non‑zero and surface the error to make the
Makefile fail; ensure you add this to both the render:all and validate:all
background subshell blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce00478e-4ddf-4d2b-96dd-52972740afb0
📒 Files selected for processing (9)
.github/workflows/on-pr.yaml.github/workflows/on-push-main.yamlMakefileREADME.mdapis/authstacks/configuration.yamlexamples/authstacks/local-colima.yamltests/test-render/main.ktests/test-render/modelupbound.yaml
💤 Files with no reviewable changes (1)
- apis/authstacks/configuration.yaml
✅ Files skipped from review due to trivial changes (2)
- README.md
- tests/test-render/model
1 participant
Summary
Three small structural changes to prepare auth-stack to host additional auth-group primitive XRDs (HumanUser, MachineUser, Grant, IDP) alongside the existing AuthStack:
api-dirmacro andhops-ops/workflows-crossplane@v3.0.0per-exampleapi_path.render:all/validate:allnow derive the api dir from each example path; single-XRD targets still work forexamples/authstacks/.apis/authstacks/configuration.yamlfrom git — already matched by.gitignore'sapis/**/configuration.yaml; the tracked copy was a stale leftover from initial commit. Build regenerates it.provider-upjet-zitadel >=v0.1.1as a project dependency. Not used by AuthStack today but required by the auth-group primitive XRDs this repo will host next.README also rewrites the "Auth-group primitives" section to enumerate the four planned XRDs (all TO WRITE) and explicitly note that Organization + Project intentionally aren't wrapped here — the Tenant business kind in tenant-stack composes the underlying Zitadel Org + Project + Role MRs directly during initial scaffolding.
Test plan
up project buildsucceedsmake render:allpasses for all 3 authstacks examples🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features
Changes
Chores