feat: add MachineUser primitive XRD#7
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR introduces a complete MachineUser Crossplane composite resource for declarative Zitadel machine user provisioning with optional Personal Access Token generation and AWS Secrets Manager integration, including API definitions, orchestration functions, examples, tests, and CI updates. ChangesMachineUser Composite Resource Implementation
Sequence DiagramsequenceDiagram
participant User
participant MachineUserXR as MachineUser XR
participant StateInit as State Init (000)
participant ReadinessCalc as Readiness (010)
participant MachineUserMR as MachineUser MR (100)
participant PATMR as AccessToken MR (200)
participant AwsSmSecret as AWS SM Secret (300)
participant EsoPush as ESO PushSecret (310)
participant StatusOut as Status (999)
User->>MachineUserXR: Create with spec (orgId, userName, pat.enabled, pat.pushToAwsSm)
MachineUserXR->>StateInit: Pass spec
StateInit->>StateInit: Extract & default fields, build $state
StateInit->>ReadinessCalc: Provide $state
ReadinessCalc->>MachineUserMR: Ready for rendering
ReadinessCalc->>PATMR: Ready for rendering (if pat.enabled)
MachineUserMR->>User: Emit MachineUser MR
alt Pat Enabled
PATMR->>User: Emit AccessToken MR
PATMR->>ReadinessCalc: Provide token.id
alt Push to AWS SM
ReadinessCalc->>AwsSmSecret: Token observed, enable AWS SM Secret
AwsSmSecret->>User: Emit AWS SM Secret MR
AwsSmSecret->>EsoPush: AWS SM Secret ready
EsoPush->>User: Emit ESO PushSecret Object
EsoPush->>EsoPush: Push access_token to AWS SM
end
end
ReadinessCalc->>StatusOut: Aggregate readiness & IDs
StatusOut->>User: Render XR status (userId, pat.tokenId, ready)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 |
Published Crossplane PackageThe following Crossplane package was published as part of this PR: Package: ghcr.io/hops-ops/auth-stack:pr-7-bf0cc3241b444eadfec32e0a7d497a5cbf5f7f32 |
![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
🤖 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 `@apis/machineusers/definition.yaml`:
- Around line 138-147: Make providerConfigRef.name required and non-empty for
composed providers: update the providerConfigRef schema (the object with
properties name and kind used for provider-aws-secretsmanager) to include
required: ["name"] and add a constraint such as minLength: 1 on the name
property to prevent empty strings; apply the same change to the other
providerConfigRef block referenced (the second occurrence around the alternate
provider section).
- Around line 107-123: The CRD currently allows pushToAwsSm to be true without
ensuring pat.enabled is true or awsSm and its core fields exist; update
apis/machineusers/definition.yaml to add schema validation so that when
pushToAwsSm is true (use an openapi v3 if/then or oneOf pattern) it requires
pat.enabled = true and requires the awsSm object and its essential fields (e.g.,
the awsSm properties your controller needs to reconcile); reference the
pushToAwsSm boolean, the pat.enabled boolean, and the awsSm properties in the
conditional validation and mark those awsSm properties as required so invalid
specs are rejected at admission time.
In `@functions/machineuser/010-state-status.yaml.gotmpl`:
- Around line 46-64: The aggregate readiness ($ready) omits the PushSecret
(eso-pushsecret) status when PAT push is enabled, allowing readiness to become
true if only $awsSmReady is true; add a new boolean (e.g., $pushSecretReady)
initialized true, and when both $state.pat.enabled and $state.pat.pushToAwsSm
are set, compute $pushSecretReady by reading get $observed "eso-pushsecret"
(similar to how $smEntry/$smResource/$smConditions are read) and set it true
only if a condition with type "Ready" and status "True" exists; finally include
$pushSecretReady in the aggregate expression (e.g., set $ready := and $muReady
$patReady $awsSmReady $pushSecretReady or conditionally include it only when
pushToAwsSm is true).
In `@functions/machineuser/999-status.yaml.gotmpl`:
- Line 12: The template emits ready: using $state.status.ready without a
fallback; update the template to render a safe boolean default when the upstream
value is missing by wrapping $state.status.ready with the template default
function (use false as the fallback) so the ready field always outputs a valid
boolean; target the expression that references $state.status.ready in the
machineuser 999-status.yaml.gotmpl and replace it with a default-wrapped
version.
- Line 25: The template emits secretName from the pipeline $state.pat.awsSm.path
with no fallback; add a defensive default to that pipeline so secretName is
always valid YAML if the upstream state is missing. Modify the template
expression that produces secretName (the $state.pat.awsSm.path usage) to use the
template default operator with a safe fallback string (e.g., an empty string or
"unknown-secret") before quoting so the field always renders valid YAML.
🪄 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: 60a1b6d6-6bac-4531-aca4-110eaa7189d1
📒 Files selected for processing (22)
.github/workflows/on-pr.yaml.github/workflows/on-push-main.yamlMakefileREADME.mdapis/machineusers/composition.yamlapis/machineusers/definition.yamlexamples/machineusers/minimal.yamlexamples/machineusers/with-pat-push.yamlexamples/machineusers/with-pat.yamlfunctions/machineuser/000-state-init.yaml.gotmplfunctions/machineuser/010-state-status.yaml.gotmplfunctions/machineuser/100-machineuser.yaml.gotmplfunctions/machineuser/200-pat.yaml.gotmplfunctions/machineuser/300-aws-sm-secret.yaml.gotmplfunctions/machineuser/310-eso-pushsecret.yaml.gotmplfunctions/machineuser/999-status.yaml.gotmpltests/test-machineuser/kcl.modtests/test-machineuser/main.ktests/test-machineuser/modeltests/test-machineuser/observed/iter2-userid.yamltests/test-machineuser/observed/iter3-patid.yamlupbound.yaml
Comment on lines
+107
to
+123
| pushToAwsSm: | ||
| description: | | ||
| When true (and pat.enabled is true), composes an AWS | ||
| Secrets Manager Secret MR + an ESO PushSecret | ||
| Kubernetes Object that pushes the control-plane K8s | ||
| Secret's access_token property to AWS SM. Consumer | ||
| clusters then pull via ESO ExternalSecret. Uses the | ||
| push/<cluster>/<tenant>/<name> path convention from | ||
| [[reference-aws-sm-push-tag-convention]]. | ||
| type: boolean | ||
| default: false | ||
| awsSm: | ||
| description: | | ||
| AWS Secrets Manager push configuration. Required when | ||
| pushToAwsSm is true. | ||
| type: object | ||
| properties: |
There was a problem hiding this comment.
Enforce PAT/AWS push invariants in the CRD schema.
pushToAwsSm can be true without schema-enforced prerequisites (pat.enabled, awsSm presence and core fields), which admits invalid specs and causes late reconcile failures.
Suggested schema hardening
pat:
description: |
Personal Access Token generation. DISABLED by default —
@@
type: object
+ x-kubernetes-validations:
+ - rule: "self.pushToAwsSm == false || self.enabled == true"
+ message: "spec.pat.pushToAwsSm requires spec.pat.enabled=true"
+ - rule: "self.pushToAwsSm == false || has(self.awsSm)"
+ message: "spec.pat.awsSm is required when spec.pat.pushToAwsSm=true"
properties:
@@
awsSm:
@@
type: object
+ required:
+ - cluster
+ - region
+ - providerConfigRef
properties:🤖 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 `@apis/machineusers/definition.yaml` around lines 107 - 123, The CRD currently
allows pushToAwsSm to be true without ensuring pat.enabled is true or awsSm and
its core fields exist; update apis/machineusers/definition.yaml to add schema
validation so that when pushToAwsSm is true (use an openapi v3 if/then or oneOf
pattern) it requires pat.enabled = true and requires the awsSm object and its
essential fields (e.g., the awsSm properties your controller needs to
reconcile); reference the pushToAwsSm boolean, the pat.enabled boolean, and the
awsSm properties in the conditional validation and mark those awsSm properties
as required so invalid specs are rejected at admission time.
Comment on lines
+138
to
+147
| providerConfigRef: | ||
| description: | | ||
| ProviderConfig for provider-aws-secretsmanager. | ||
| type: object | ||
| properties: | ||
| name: | ||
| type: string | ||
| kind: | ||
| type: string | ||
| default: "ProviderConfig" |
There was a problem hiding this comment.
Require providerConfigRef.name for composed providers.
Both Zitadel and AWS provider refs allow empty name today. That permits invalid XR specs that render composed resources with unusable providerConfigRef.
Suggested required fields
providerConfigRef:
@@
type: object
+ required:
+ - name
properties:
name:
type: string
@@
providerConfigRef:
@@
type: object
+ required:
+ - name
properties:
name:
type: stringAlso applies to: 184-194
🤖 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 `@apis/machineusers/definition.yaml` around lines 138 - 147, Make
providerConfigRef.name required and non-empty for composed providers: update the
providerConfigRef schema (the object with properties name and kind used for
provider-aws-secretsmanager) to include required: ["name"] and add a constraint
such as minLength: 1 on the name property to prevent empty strings; apply the
same change to the other providerConfigRef block referenced (the second
occurrence around the alternate provider section).
Comment on lines
+46
to
+64
| # AWS SM + PushSecret observed state | ||
| {{- $awsSmArn := "" }} | ||
| {{- $awsSmReady := true }} | ||
| {{- if and $state.pat.enabled $state.pat.pushToAwsSm }} | ||
| {{- $awsSmReady = false }} | ||
| {{- $smEntry := get $observed "aws-sm-secret" | default dict }} | ||
| {{- $smResource := $smEntry.resource | default dict }} | ||
| {{- $smAtProvider := (($smResource.status | default dict).atProvider | default dict) }} | ||
| {{- $awsSmArn = $smAtProvider.arn | default "" }} | ||
| {{- $smConditions := (($smResource.status | default dict).conditions | default list) }} | ||
| {{- range $smConditions }} | ||
| {{- if and (eq .type "Ready") (eq .status "True") }} | ||
| {{- $awsSmReady = true }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
|
|
||
| # Aggregate | ||
| {{- $ready := and $muReady $patReady $awsSmReady }} |
There was a problem hiding this comment.
Include PushSecret readiness in aggregate ready.
When PAT push is enabled, readiness only checks aws-sm-secret. If eso-pushsecret fails, composite readiness can still become true prematurely.
Suggested readiness aggregation update
# AWS SM + PushSecret observed state
{{- $awsSmArn := "" }}
{{- $awsSmReady := true }}
+{{- $pushSecretReady := true }}
{{- if and $state.pat.enabled $state.pat.pushToAwsSm }}
{{- $awsSmReady = false }}
+ {{- $pushSecretReady = false }}
@@
{{- end }}
+
+ {{- $psEntry := get $observed "eso-pushsecret" | default dict }}
+ {{- $psResource := $psEntry.resource | default dict }}
+ {{- $psConditions := (($psResource.status | default dict).conditions | default list) }}
+ {{- range $psConditions }}
+ {{- if and (eq .type "Ready") (eq .status "True") }}
+ {{- $pushSecretReady = true }}
+ {{- end }}
+ {{- end }}
{{- end }}
# Aggregate
-{{- $ready := and $muReady $patReady $awsSmReady }}
+{{- $ready := and $muReady $patReady $awsSmReady $pushSecretReady }}🤖 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 `@functions/machineuser/010-state-status.yaml.gotmpl` around lines 46 - 64, The
aggregate readiness ($ready) omits the PushSecret (eso-pushsecret) status when
PAT push is enabled, allowing readiness to become true if only $awsSmReady is
true; add a new boolean (e.g., $pushSecretReady) initialized true, and when both
$state.pat.enabled and $state.pat.pushToAwsSm are set, compute $pushSecretReady
by reading get $observed "eso-pushsecret" (similar to how
$smEntry/$smResource/$smConditions are read) and set it true only if a condition
with type "Ready" and status "True" exists; finally include $pushSecretReady in
the aggregate expression (e.g., set $ready := and $muReady $patReady $awsSmReady
$pushSecretReady or conditionally include it only when pushToAwsSm is true).
| apiVersion: {{ $xr.apiVersion }} | ||
| kind: {{ $xr.kind }} | ||
| status: | ||
| ready: {{ $state.status.ready }} |
There was a problem hiding this comment.
Add default value for the ready field.
The ready field is output without a default value. If $state.status.ready is not set by upstream functions, this will output <no value> or similar, resulting in invalid YAML or unexpected boolean coercion.
🛡️ Proposed fix to add defensive default
- ready: {{ $state.status.ready }}
+ ready: {{ $state.status.ready | default false }}📝 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
| ready: {{ $state.status.ready }} | |
| ready: {{ $state.status.ready | default false }} |
🤖 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 `@functions/machineuser/999-status.yaml.gotmpl` at line 12, The template emits
ready: using $state.status.ready without a fallback; update the template to
render a safe boolean default when the upstream value is missing by wrapping
$state.status.ready with the template default function (use false as the
fallback) so the ready field always outputs a valid boolean; target the
expression that references $state.status.ready in the machineuser
999-status.yaml.gotmpl and replace it with a default-wrapped version.
| {{- if $state.pat.pushToAwsSm }} | ||
| awsSm: | ||
| secretArn: {{ $state.status.awsSmArn | default "" | quote }} | ||
| secretName: {{ $state.pat.awsSm.path | quote }} |
There was a problem hiding this comment.
Add default value for the AWS Secrets Manager secretName field.
The secretName field is output from $state.pat.awsSm.path without a default value. Although this is within the pushToAwsSm conditional where the path should be set, defensive coding suggests adding a default to prevent invalid YAML output if the upstream state initialization is incomplete.
🛡️ Proposed fix to add defensive default
- secretName: {{ $state.pat.awsSm.path | quote }}
+ secretName: {{ $state.pat.awsSm.path | default "" | quote }}📝 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
| secretName: {{ $state.pat.awsSm.path | quote }} | |
| secretName: {{ $state.pat.awsSm.path | default "" | quote }} |
🤖 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 `@functions/machineuser/999-status.yaml.gotmpl` at line 25, The template emits
secretName from the pipeline $state.pat.awsSm.path with no fallback; add a
defensive default to that pipeline so secretName is always valid YAML if the
upstream state is missing. Modify the template expression that produces
secretName (the $state.pat.awsSm.path usage) to use the template default
operator with a safe fallback string (e.g., an empty string or "unknown-secret")
before quoting so the field always renders valid YAML.
…eline The first auth-group primitive that survives the "is it just a single-MR wrapper?" test. Composes 1-4 underlying resources depending on opt-ins: 1. Always: Zitadel MachineUser MR (machineusers.user.zitadel.m.crossplane.io) 2. When spec.pat.enabled (default false): + Zitadel AccessToken MR (the PAT; connection secret on control-plane K8s under key access_token) 3. When spec.pat.pushToAwsSm (default false; requires pat.enabled): + AWS Secrets Manager Secret MR + ESO PushSecret (via provider-kubernetes Object) that pushes the control-plane K8s Secret's access_token into AWS SM at the canonical path push/<cluster>/<tenant>/<name> per reference_aws_sm_push_tag_convention. Consumer clusters pull via ESO ExternalSecret. Composition gating per feedback_crossplane_composition_gates: AccessToken emits once MachineUser.atProvider.id observed; AWS SM Secret + PushSecret emit once AccessToken.atProvider.id observed. Standard multi-iteration Crossplane convergence. PAT generation is opt-in by default — operators explicitly acknowledge minting a long-lived bearer token. Adoption via spec.machineUserId propagates as crossplane.io/external-name on the underlying MR. upbound.yaml gains provider-upjet-zitadel >=v0.1.1 and provider-aws-secretsmanager >=v2.5.0 deps (both required by the composition; aws-secretsmanager only matters when pushToAwsSm is on). 3 examples (minimal, with-pat, with-pat-push) — all render via up composition render. Multi-iter convergence verified via --observed-resources fixtures. 4 KCL CompositionTests covering: minimal-machineuser-only, pat-enabled- iter1-still-only-machineuser (gating verification), adopt-existing- machineuser-via-id, field-overrides. All 17 tests (13 AuthStack + 4 MachineUser) pass under `up test run`. Makefile EXAMPLES + CI workflows updated for the new examples. README "Auth-group primitives" table updated: MachineUser ✓, Grant TO WRITE (next); HumanUser / IDP / OrganizationSsoConfig dropped as thin wrappers — operators apply raw provider MRs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
patrickleet
force-pushed
the
feat/machineuser-xrd
branch
from
1d5aac3 to
4676a78
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Composes 1-4 resources depending on opt-ins. PAT off by default. See commit message for details.
Summary by CodeRabbit
New Features
MachineUsercomposite resource.Documentation