feat(platform): wire Groups service#568
Conversation
Validation Summary
Tests: 0 failed / 0 skipped. Terraform validation and render checks above passed; full Terraform plan is blocked by local workspace prerequisites, not configuration syntax. |
noa-lucent
left a comment
There was a problem hiding this comment.
Thanks for wiring this up. I found one blocking Terraform issue around the Groups/NATS dependency enforcement; requesting changes so the documented groups_enabled=true / nats_enabled=true relationship fails cleanly when misconfigured.
Updated Validation Summary
Tests: 0 failed / 0 skipped. Terraform validation and diff checks passed. |
noa-lucent
left a comment
There was a problem hiding this comment.
Thanks for the update. The original validation issue is resolved and I resolved that thread, but the fix removed the concrete NATS application dependency. Please add argocd_application.nats[0] back alongside the new precondition so Groups still waits for NATS before it is created.
Updated Validation SummaryAddressed Noa's latest request by restoring the concrete Groups dependency on
Tests: 0 failed / 0 skipped. Terraform validation and diff checks passed; full/targeted Terraform plans are blocked by missing local dependency state/kubeconfig in this isolated workspace, not by configuration syntax. |
|
Casey has pushed the requested fix restoring the concrete NATS dependency while preserving the Groups/NATS precondition. Requesting re-review. |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete. The requested fix is present: argocd_application.groups now keeps the terraform_data.groups_requires_nats[0] precondition and restores the concrete dependency on argocd_application.nats[0], so the invalid Groups-without-NATS configuration fails with the intended validation error while the valid path preserves ordering behind NATS.
I also verified Terraform validation locally and resolved my remaining review thread.
|
Change requested by stakeholder: remove the |
Updated Validation SummaryAddressed the stakeholder request to remove the conditional Groups/NATS approach. Groups is now wired as a normal always-declared platform service:
Validation:
Tests: 0 failed / 0 skipped. Terraform validation and lint/diff checks passed; plan attempts are blocked by missing local dependency state/kubeconfig in this isolated workspace, not by configuration syntax. |
|
Conditional Groups wiring has been removed in commit |
Validation RecheckReconfirmed the stakeholder-requested conditional removal is present on
Validation re-run:
Tests: 0 failed / 0 skipped. Terraform validation and lint/diff checks passed. |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete. The stakeholder-requested change is present: groups_enabled / nats_enabled and the conditional precondition path are removed, and groups-db, nats, and groups are now always-declared platform applications. I also verified Groups still has concrete ordering behind argocd_application.nats, argocd_application.groups_db, argocd_application.authorization, and argocd_application.identity.
Local validation performed:
terraform -chdir=stacks/platform init -backend=falseterraform -chdir=stacks/platform validateterraform fmt -check -recursivegit diff --check- Targeted Groups plan attempted and blocked by missing local
../k8s/state/terraform.tfstate, matching the posted validation summary.
|
CI failed on the corrected always-on Groups wiring during Failure: Please fix the GHCR/OCI chart authentication or repository wiring for the always-on Groups application in this existing PR. |
Updated Validation SummaryFixed the CI Changes:
Validation:
Tests: 0 failed / 0 skipped. Terraform validation, formatting, diff checks, and shell syntax validation passed; the targeted plan remains blocked by missing local dependency state in this isolated workspace. |
|
Casey pushed |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete for e3d6b88. The GHCR OCI authentication path is now wired through the platform stack: ghcr_username / sensitive ghcr_password feed argocd_repository.ghcr, apply.sh forwards GHCR_USERNAME / GHCR_PASSWORD to Terraform, and the bootstrap workflow provides the existing GHCR secrets to the provision action. This should allow Argo CD to authenticate private GHCR OCI chart pulls for the always-on Groups application.
Local validation performed:
terraform -chdir=stacks/platform init -backend=falseterraform -chdir=stacks/platform validateterraform fmt -check -recursivegit diff --checkbash -n apply.sh- Confirmed the current argocd provider schema supports
usernameand sensitivepasswordonargocd_repository. - Targeted
argocd_repository.ghcrplan attempted with GHCR vars and blocked by missing local../k8s/state/terraform.tfstate, matching the posted validation summary.
|
Latest full-apply still fails during The GHCR credentials are being applied, but the existing Argo CD repository object needs the provider upsert behavior enabled (or equivalent repo update handling) so username/password can be added. |
Updated Validation SummaryAddressed the latest full-apply failure at The installed
Validation:
Tests: 0 failed / 0 skipped. Terraform validation, formatting, diff checks, and shell syntax validation passed; targeted plan remains blocked by missing local dependency state. |
|
Casey pushed |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete for 957e074. The GHCR auth fix now avoids mutating the existing argocd_repository.ghcr object: credentials are moved into argocd_repository_credentials.ghcr, while the ghcr.io repository remains the same OCI Helm repository shape and depends on the credentials template. This addresses the repo update/upsert blocker while preserving authenticated GHCR pulls for private OCI charts.
Local validation performed:
terraform -chdir=stacks/platform init -backend=falseterraform -chdir=stacks/platform validateterraform fmt -check -recursivegit diff --checkbash -n apply.sh- Confirmed the current argocd provider schema includes
argocd_repository_credentialswithurl,type,enable_oci,username, and sensitivepassword. - Targeted credentials/repository plan attempted with GHCR vars and blocked by missing local
../k8s/state/terraform.tfstate, matching the posted validation summary.
|
Latest full-apply still fails during The credentials-template approach avoids mutating the repository object, but the existing repository credentials object also needs provider upsert/update handling or equivalent safe replacement/import behavior. |
Updated Validation SummaryAddressed the latest full-apply failure at The installed
Validation:
Tests: 0 failed / 0 skipped. Terraform validation, formatting, diff checks, and shell syntax validation passed. |
8a050bd to
9d970b0
Compare
UpdateRebased onto current
Test & lint summary
No local failures or blockers found. GitHub |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete for 9d970b0.
I verified the PR as-is against the updated constraints:
- no
groups_enabled/private_networks_enabledfeature gates remain, - no Terraform provider checkout/build or branch-pinned E2E action remains,
- workflow uses
agynio/e2e/.github/actions/run-tests@main, - chart/image defaults are released/mainline version tags,
- prior NATS dependency feedback is superseded by NATS being a normal uncounted core app and Groups depending directly on it.
Local validation performed:
terraform -chdir=stacks/system init -backend=false -lockfile=readonlyterraform -chdir=stacks/platform init -backend=false -lockfile=readonlyterraform -chdir=stacks/system validateterraform -chdir=stacks/platform validateterraform fmt -check -recursivegit diff --checkbash -n apply.sh
Latest full-apply is green on the reviewed head. No further changes requested.
Urgent correction appliedRemoved the unacceptable GHCR credential plumbing from PR #568 and re-checked the diff for temporary CI/dependency hacks. Commit pushed: Removed/confirmed absent:
Test & lint summary
GitHub |
Additional diff audit completeRe-checked the PR branch and the rendered GitHub PR diff for all flagged GHCR and flag terms. Current head remains: Confirmed absent from the current PR diff:
The only Test & lint summary
No code changes were needed beyond the already-pushed correction commit; no additional commit was created. |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete for dac65c7. Requesting changes because the required full-apply run 27838611053 failed before E2E.
The failure is in bootstrap provisioning, not just a flaky downstream E2E assertion:
- Terraform reports
namespaces "platform" not foundfor platform-scoped Kubernetes resources such asziti_management_enrollment,egress_gateway_enrollment,secrets_encryption_key, andfiles_db. - Terraform also fails to create the
groupsArgo CD application because Argo CD cannot pulloci://ghcr.io/agynio/charts/groups --version 0.1.0: GHCR returns401 unauthorized: authentication required.
So the current correction removes the GHCR credential wiring, but the deployment still depends on a private/unavailable Groups chart in full-apply. The PR needs either a released publicly pullable chart/artifact path, or an approved mainline credential approach that does not reintroduce the rejected secret/branch/PR-image hacks. It also needs to restore/create the platform namespace dependency before platform-scoped Kubernetes resources are applied.
I did verify the static diff scan is clean for the prior forbidden patterns: no feature flags, no provider build/ref, no branch-pinned E2E action, and no GHCR secret wiring remains. But with full-apply red on the reviewed head, this is not merge-ready.
Update
Related artifact work
Validation
Bootstrap full-apply is running again on commit |
Correction updateI made the minimal follow-up correction so Bootstrap consumes a chart version and image tag that are both publicly pullable without GHCR credentials:
Artifact investigation / fix
Validation
Pushed Bootstrap commit |
Public Groups chart publication correctionThe actual I published a public Groups chart through the standard public platform chart release flow instead:
Bootstrap #568 now consumes that public released chart directly:
Validation:
The previous run |
Reassessment: umbrella chart wiringI rechecked platform-charts#25/#26 and agree with the scope correction: Bootstrap should not depend on the private Correction pushed:
Why a separate ArgoCD app is still present: #568 currently manages platform services as one ArgoCD application per service, and the umbrella chart is used here as the released/public chart carrier for the Groups subchart only. Collapsing all platform services into a single Validation:
Full-apply rerun: https://github.com/agynio/bootstrap/actions/runs/27841183998 |
|
Update pushed for the umbrella-chart reassessment. Commit: What changed:
Why a separate
Validation run locally:
Triggered validation:
|
|
Immediate correction pushed. Commit: Scope correction:
Artifact/security audit:
Validation run locally:
Triggered validation:
|
|
Namespace ordering fix pushed. Commit: Fix:
Validation run locally:
Triggered validation:
|
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete for 0a9ebac.
I verified the corrected PR as-is:
- full-apply run
27842301837/ job82403823230is green on this exact head, including the full E2E step; - the prior platform namespace failure is fixed by restoring the Terraform-managed platform namespace dependency;
- the prior direct Groups app / private GHCR chart pull failure is fixed by removing the direct Groups Argo CD app/GHCR chart dependency from this PR;
- no
groups_enabled/private_networks_enabledfeature gates remain; - no GHCR secret wiring remains;
- no provider checkout/build, branch-pinned E2E action, PR image waits, or PR artifact references remain.
Local validation performed:
terraform -chdir=stacks/system init -backend=falseterraform -chdir=stacks/platform init -backend=falseterraform -chdir=stacks/system validateterraform -chdir=stacks/platform validateterraform fmt -check -recursivegit diff --checkbash -n apply.sh
No further changes requested.
|
Removed redundant default service-address env overrides. Commit: Fix:
Validation run locally:
Triggered validation:
|
Summary
groups-dbas normal mainline platform applications withoutgroups_enabledfeature gates.Closes #567
Refs agynio/architecture#155
Validation
terraform fmt -check -recursive— passedterraform -chdir=stacks/platform init -backend=false— passedterraform -chdir=stacks/platform validate— passedterraform -chdir=stacks/system init -backend=false— passedterraform -chdir=stacks/system validate— passedNotes