fix(api): pre-flight validation for federation IaC bundles (target/source + Azure/GCP source-identity)#67
Conversation
…urce + Azure/GCP source-identity) Adds two pre-flight validation guards to the federation IaC handler so operator misconfigurations fail loudly at download time instead of silently shipping broken bundles that fail at `terraform apply`. #42 — validateFederationTargetSource (HTTP 400, fail fast) Reject impossible target/source combinations early — before bundle construction. Today the only impossible-by-construction combo is target=aws-cross-account (target=aws + source=aws) from a non-AWS CUDly deployment: the rendered trust policy needs CUDly's AWS account ID in the principal ARN, which a CUDly running on Azure/GCP cannot supply. Returns NewClientError(400) naming the actual deployment cloud so the caller knows why. #41 — validateSourceIdentity (HTTP 500, operator misconfig) Extends the existing AWS-only fail-loud guard in populateSourceAccountID to cover Azure and GCP source clouds. When CUDly runs on Azure but AZURE_SUBSCRIPTION_ID or AZURE_TENANT_ID is unset (or on GCP without GCP_PROJECT_ID), the bundle previously shipped with empty identity fields and customers hit a cryptic `terraform apply` error with a blank client_id / missing project. Now returns a 500-class error naming the missing env var so the operator knows exactly what to fix. Test seam: introduces mockSourceIdentity + defaultTestSourceIdentity in handler_federation_test.go so tests can pre-warm the cached sourceIdentity without going through STS / env vars. The existing TestGetFederationIaC_FailsLoudOnEmptySourceAccountID is simplified to use the new seam instead of clearing every AWS_* env var. The AWS-cross-account tests (which previously relied on the default CUDLY_SOURCE_CLOUD=gcp test fixture and the absence of the new target/source check) now correctly set CUDLY_SOURCE_CLOUD=aws. New test coverage: - 3 Azure subtests (missing-subscription-id, missing-tenant-id, all-populated positive control) - 1 GCP test (missing-project-id) - 3 target/source consistency subtests (azure→reject, gcp→reject, aws→allow regression guard) Closes #41 Closes #42
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 7 seconds. ⌛ 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 (2)
✨ 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Adds two pre-flight validation guards to the federation IaC handler so operator misconfigurations fail loudly at download time instead of silently shipping broken bundles that fail at
terraform apply.Combines fixes for two related issues that touch the same code path (
internal/api/handler_federation.go::getFederationIaCand the surrounding helpers), so reviewers don't have to look at the same function twice.Closes #41
Closes #42
Changes
#42 —
validateFederationTargetSource(HTTP 400, fail fast)Reject impossible target/source combinations before bundle construction. Today the only impossible-by-construction combo is
target=aws-cross-account(target=aws+source=aws) from a non-AWS CUDly deployment: the rendered trust policy needs CUDly's AWS account ID in the principal ARN, which a CUDly running on Azure/GCP cannot supply. ReturnsNewClientError(400)naming the actual deployment cloud so the caller sees:instead of a downloadable bundle that later fails with a cryptic
invalid principal ARNIAM error.#41 —
validateSourceIdentity(HTTP 500, operator misconfig)Extends the existing AWS-only fail-loud guard in
populateSourceAccountIDto cover Azure and GCP source clouds. When CUDly runs on Azure butAZURE_SUBSCRIPTION_IDorAZURE_TENANT_IDis unset (or on GCP withoutGCP_PROJECT_ID), the bundle previously shipped with empty identity fields. Customers ranterraform applyand got a blankclient_id/ missing project. Now the handler returns a 500-class error naming the missing env var so the operator knows exactly what to fix.Test seam
Introduces
mockSourceIdentity+defaultTestSourceIdentityinhandler_federation_test.goso tests can pre-warm the cachedsourceIdentitywithout going through STS / env vars. The existingTestGetFederationIaC_FailsLoudOnEmptySourceAccountIDis simplified to use the new seam instead of clearing everyAWS_*env var. The AWS-cross-account tests (which previously relied on the defaultCUDLY_SOURCE_CLOUD=gcptest fixture and the absence of the new target/source check) now correctly setCUDLY_SOURCE_CLOUD=aws.Files changed
internal/api/handler_federation.go— +66 LOC (two new helpers + two new call sites ingetFederationIaC)internal/api/handler_federation_test.go— +258 / -19 (new test seam, 7 new test cases across 3 test functions, updates to existing AWS-cross-account tests)Test plan
go build ./...cleango vet ./...cleango test ./...clean (full repo suite)TestGetFederationIaC_FailsLoudOnEmptyAzureSourceIdentity— 3 subtests (missing-subscription-id, missing-tenant-id, all-populated positive control)TestGetFederationIaC_FailsLoudOnEmptyGCPSourceIdentity— missing GCP_PROJECT_IDTestGetFederationIaC_RejectsImpossibleTargetSourceCombo— 3 subtests (azure→reject, gcp→reject, aws→allow regression guard)Notes
feat/multicloud-web-frontend) is not the repo's default branch — that's expected; the@coderabbitai reviewcomment is still posted per project convention.