Skip to content

feat(iac): shared Archera module + 'Provision Archera Insurance permissions?' bundle checkbox (closes #314)#315

Closed
cristim wants to merge 9 commits into
feat/multicloud-web-frontendfrom
feat/issue-314-archera-bundle-flag
Closed

feat(iac): shared Archera module + 'Provision Archera Insurance permissions?' bundle checkbox (closes #314)#315
cristim wants to merge 9 commits into
feat/multicloud-web-frontendfrom
feat/issue-314-archera-bundle-flag

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented May 6, 2026

Summary

Implements the Archera Insurance integration opt-in flag from issue #314, with a shared-module architecture that ensures a single source of truth for all permission lists.

Shared module (Option B refactor)

  • Adds iac/modules/archera/{aws,azure,gcp}/ — single source of truth for Archera IAM resources
  • Each cloud's permission scope lives in iac/modules/archera/scope.{aws,azure,gcp}.yaml; Terraform modules consume it via yamldecode(file(...)) so editing the YAML propagates to every caller automatically
  • All 5 federation bundle archera.tf files and all 3 environment archera.tf files are thin module callers — no inline permission lists

Environment callers

Creates terraform/environments/{aws,azure,gcp}/archera.tf as thin module callers. This supersedes PR #310 which carried inline IAM resource bodies. No state migration needed — confirmed by @cristim, no environment has ever applied the Archera resources.

Alternative deployment formats

  • iac/federation/aws-{target,cross-account}/cloudformation/archera.cfn.yaml — CloudFormation alternative for AWS customers
  • iac/federation/azure-target/bicep/archera.bicep — Bicep alternative
  • iac/federation/azure-target/arm/archera.arm.json — ARM template alternative

Permission lists in CFN/Bicep/ARM mirror the scope YAML; the CI archera-parity job enforces parity automatically.

UI checkbox + API flag

  • Frontend: "Provision Archera Insurance permissions?" checkbox (default unchecked) in the federation download panel
  • Backend: include_archera=true query parameter on /api/federation/iac; tfvars templates conditionally append Archera variables
  • Default: off — existing bundle downloads are byte-identical

Validation

Format Validated
Terraform (AWS) terraform init && terraform validate — passes locally
Terraform (Azure) terraform init && terraform validate — passes locally
Terraform (GCP) terraform init && terraform validate — passes locally
CloudFormation YAML structure verified; aws cloudformation validate-template requires live AWS creds — not validated locally
Bicep bicep CLI not installed locally — not validated locally
ARM JSON JSON syntax valid (pre-commit check-json passed)

Closes / supersedes

Closes #314. Supersedes #310 (which carried the inline-implementation version of the same Archera work; this PR unifies into a shared module).

Test plan

  • Go test suite: 4310 tests pass
  • Frontend test suite: 1521 tests pass
  • bash scripts/check-archera-parity.sh passes
  • CI archera-parity job green
  • Verify checkbox appears in federation panel UI with default=off
  • Verify bundle with checkbox=off is byte-identical to pre-checkbox bundle
  • Verify bundle with checkbox=on contains enable_archera = true in tfvars
  • PR feat(iac): scaffolding for Archera integration perms across AWS/Azure/GCP (closes #305) #310 can be closed as superseded after this merges

Summary by CodeRabbit

  • New Features

    • Added Archera opt-in checkbox in the federation setup UI
    • Archera integration support added across AWS, Azure, and GCP deployment templates
    • Federation IaC generation now supports an optional Archera inclusion flag
  • Tests

    • Added UI and backend tests covering Archera opt-in behavior and IaC generation
  • Chores

    • CI job added to validate Archera permission parity across templates
  • Documentation

    • Template docs updated to describe the Archera opt-in and related variables

cristim added 7 commits May 6, 2026 12:22
Adds iac/modules/archera/{aws,azure,gcp}/ as the single source of truth
for Archera Insurance IAM resources across all deployment paths.  Each
cloud's permission scope lives in scope.{aws,azure,gcp}.yaml; the
Terraform modules consume it via yamldecode(file(...)) so editing the
YAML propagates to every caller automatically.

Module interface is identical across AWS/Azure/GCP:
  enable_archera                  (bool, default false)
  archera_<cloud>_account_id/sp_object_id/service_account
  archera_external_id (AWS only, sensitive)
  enable_archera_purchase_actions (bool, default false)
…e callers

The 5 federation bundle archera.tf files previously duplicated the full IAM
resource bodies.  Replace each with a thin module caller that delegates to
iac/modules/archera/{aws,azure,gcp}/, which loads permissions from
scope.*.yaml.  Editing the YAML now propagates to all 5 federation paths
and the 3 self-deploy environments simultaneously — no more out-of-sync copies.

No state addresses change: these resources were never applied in any environment.
…oyments

Adds alternative deployment paths for customers who prefer CloudFormation
(AWS), Bicep, or ARM (Azure) over Terraform:
  iac/federation/aws-{target,cross-account}/cloudformation/archera.cfn.yaml
  iac/federation/azure-target/bicep/archera.bicep
  iac/federation/azure-target/arm/archera.arm.json

All permission lists are literals that mirror iac/modules/archera/scope.*.yaml.
The check-archera-parity.sh CI gate enforces parity so drift is caught in CI.
Bicep/ARM validation requires local tooling (flagged in PR body); CFN YAML
structure has been manually verified.
Creates terraform/environments/{aws,azure,gcp}/archera.tf as thin module
callers pointing at iac/modules/archera/{cloud}/, mirroring the federation
bundle approach.  Also adds:
  - Archera variable declarations to each environment variables.tf
  - Commented-out example values in dev.tfvars.example (default: disabled)

This supersedes PR #310 which carried inline IAM resource bodies; the shared
module is the canonical implementation. No state addresses change since no
environment has ever applied the Archera resources.
Adds scripts/check-archera-parity.sh which verifies that the permission
lists embedded in CloudFormation, Bicep, and ARM templates exactly match
the canonical iac/modules/archera/scope.*.yaml files.  Also checks that
terraform/environments/*/archera.tf are thin module callers (no inline
permissions — those belong in the shared module).

Adds the archera-parity CI job that runs the script on every PR push.
Requires python3-yaml for YAML parsing; installed as a CI step.
Adds a new opt-in flag (include_archera) to the /api/federation/iac endpoint
and the generate-federation-iac CLI.  When true, tfvars templates append
enable_archera + cloud-specific Archera input variables; CFN params JSON
adds the corresponding stack parameters.

Default is false so existing bundle downloads are byte-identical.  The
flag is surfaced by the frontend checkbox in the next commit.

Also updates IaC templates README to document the IncludeArchera field.
Adds an opt-in checkbox to the federation download panel that controls
whether include_archera=true is sent to the IaC bundle API.  Default is
unchecked (off) so existing bundle downloads are unchanged.

The checkbox persists its checked state for the lifetime of the panel —
toggling immediately affects the next download without a page reload.
Includes tooltip text explaining the Archera Insurance integration.

Also adds getFederationIaC includeArchera parameter to accounts.ts API.

Adds federation.test.ts with 20 tests covering the checkbox lifecycle,
byte-identity when unchecked, and presence of Archera vars when checked
(mocked API responses).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c202f726-b48d-452e-a4d1-1b0b604ecf0b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an opt-in "Provision Archera Insurance permissions?" checkbox to the Federation Setup UI and threads the flag through frontend, API, local generator, templates, and IaC to conditionally include Archera resources; introduces shared Archera Terraform modules, canonical scope YAMLs, parity-check script, tests, and CI gate.

Changes

End-to-End Archera Bundle Opt-In

Layer / File(s) Summary
Scope & Permissions
iac/modules/archera/scope.{aws,azure,gcp}.yaml
Add canonical permission lists (read + optional purchase actions) for AWS, Azure, GCP.
Terraform Modules (implementation)
iac/modules/archera/{aws,azure,gcp}/{main.tf,variables.tf,outputs.tf}
New reusable modules that load scope YAML and conditionally create Archera roles/policies/assignments gated by enable_archera and enable_archera_purchase_actions.
Federation IaC (wiring / callers / templates)
iac/federation/.../{terraform,cloudformation,bicep,arm}/archera.*, iac/federation/.../terraform/variables.tf
Thin module callers for Terraform; new CFN/Bicep/ARM templates (or snippets) to include Archera parameters; variables added to federation-target variables files.
Environment / Self-deploy callers
terraform/environments/{aws,azure,gcp}/{archera.tf,variables.tf,dev.tfvars.example}
Thin module callers and environment variable examples for self-deploy Archera integration.
Bundle Templates
internal/iacfiles/templates/{aws-*,azure-*,gcp-*}.{tfvars,json,bicep}.tmpl, internal/iacfiles/templates/azure-wif-bicep-params.json.tmpl
Go templates updated to conditionally render Archera variables/parameters when IncludeArchera is true.
Backend handler & tests
internal/api/handler_federation.go, internal/api/handler_federation_test.go
Accept include_archera query param, populate IncludeArchera in render data, append CFN/Bicep params when enabled; tests added for both true/false flows and param presence.
Local generator CLI
scripts/generate-federation-iac.go
Add --include-archera flag and propagate to render data struct.
Frontend UI & API client
frontend/src/federation.ts, frontend/src/api/accounts.ts
Add module-level Archera opt-in state and UI checkbox (tooltip); pass includeArchera to getFederationIaC; API signature extended to accept optional includeArchera.
Frontend tests
frontend/src/__tests__/federation.test.ts
New tests for checkbox rendering, default state, tooltip, persistence across cloud selection, and correct include_archera in download calls.
CI parity check & workflow
scripts/check-archera-parity.sh, .github/workflows/ci.yml
New script compares CFN/Bicep/ARM action lists against canonical scope YAMLs; CI job archera-parity runs parity check and is added to ci-success dependencies.
Docs / templates README
internal/iacfiles/templates/README.md
Document IncludeArchera template variable and describe behavior and usage.

Sequence Diagram

sequenceDiagram
    participant User as User<br/>(Settings)
    participant UI as Frontend<br/>(federation.ts)
    participant API as Backend API<br/>(handler_federation.go)
    participant Templates as Template<br/>Renderer
    participant Bundle as Rendered<br/>Bundle

    User->>UI: Toggle Archera checkbox
    UI->>UI: update _includeArchera state
    User->>UI: Click "Download Bundle"
    UI->>API: GET /api/federation/iac?include_archera=true
    API->>Templates: Render with IncludeArchera=true
    Templates->>Templates: Conditionally include Archera module/params (TF/CFN/Bicep/ARM)
    Templates->>Bundle: Produce bundle with Archera resources (if enabled)
    Bundle->>User: Download generated bundle
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

Possibly related PRs

  • LeanerCloud/CUDly#105: Touches .github/workflows/ci.yml; potentially overlaps CI workflow edits and job wiring.

Suggested labels

impact/few, effort/m

Poem

🐰 I found a tiny checkbox bright,
Hop, hop—Archera joins the light.
YAML rules sing tidy and true,
Modules dance in Terraform too,
Parity checked — the bundle's right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.83% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: a shared Archera module and a frontend checkbox for provisioning Archera permissions in federation bundles.
Linked Issues check ✅ Passed The PR implements all primary objectives from issue #314: frontend checkbox, backend handler, template rendering, federation modules, shared module architecture, parity script, and comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #314 objectives: Archera integration for federation bundles, shared modules, UI checkbox, backend plumbing, templates, and CI gate. No unrelated or tangential changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-314-archera-bundle-flag

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

@cristim cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-quarter Within the quarter impact/many Affects most users effort/l Weeks type/feat New capability triaged Item has been triaged labels May 6, 2026
@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 6, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (7)
iac/federation/gcp-sa-impersonation/terraform/variables.tf (1)

49-53: 💤 Low value

Consider adding validation for GCP service account email format.

The AWS equivalent (archera_aws_account_id) includes a validation block for the 12-digit format. For consistency and early error detection, consider validating the GCP service account email format.

♻️ Optional validation block
 variable "archera_gcp_service_account" {
   description = "Full service account email of the SA Archera provides during onboarding, e.g. archera-integration@archera-prod.iam.gserviceaccount.com. Required when enable_archera = true."
   type        = string
   default     = ""
+
+  validation {
+    condition     = var.archera_gcp_service_account == "" || can(regex("^[a-z][a-z0-9-]*@[a-z][a-z0-9-]*\\.iam\\.gserviceaccount\\.com$", var.archera_gcp_service_account))
+    error_message = "archera_gcp_service_account must be a valid GCP service account email (e.g., name@project.iam.gserviceaccount.com)."
+  }
 }
🤖 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 `@iac/federation/gcp-sa-impersonation/terraform/variables.tf` around lines 49 -
53, The variable archera_gcp_service_account lacks input validation; add a
Terraform validation block for variable "archera_gcp_service_account" to enforce
a proper GCP service account email format (e.g.
local-part@project.iam.gserviceaccount.com) using a regex and a clear
error_message; update the variable block for archera_gcp_service_account to
include the validation rule so invalid values are rejected early during
terraform plan/apply.
scripts/check-archera-parity.sh (1)

97-108: 💤 Low value

Bicep extractor may miss double-quoted strings.

The extract_bicep_azure_actions function only matches single-quoted strings ('Microsoft\.[^']+'), but Bicep also supports double-quoted strings. If the Bicep template uses double quotes for any action strings, they would be missed.

♻️ Handle both quote styles
 extract_bicep_azure_actions() {
   local file="$1"
-  grep -oE "'Microsoft\.[^']+'" "$file" \
-    | tr -d "'" \
+  grep -oE "(['\"])Microsoft\.[^'\"]+\1" "$file" \
+    | sed "s/^['\"]//;s/['\"]$//" \
     | grep -vE '@|roleDefinitions$|roleAssignments$' \
     | sort -u
 }
🤖 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 `@scripts/check-archera-parity.sh` around lines 97 - 108, The extractor
extract_bicep_azure_actions only matches single-quoted strings; update its first
grep to match both single- and double-quoted Microsoft action strings (e.g.,
change the regex to capture either quote type like
(['\"])Microsoft\.[^'\"\\]+\\1), then adjust the quote-stripping step (tr -d) to
remove both ' and " so the rest of the pipeline (grep -vE
'@|roleDefinitions$|roleAssignments$' | sort -u) continues to work unchanged.
iac/federation/gcp-target/terraform/variables.tf (1)

224-228: 💤 Low value

Consider adding validation for GCP service account email format.

Unlike the AWS counterpart which validates the 12-digit account ID format, this variable accepts any string. A malformed service account email will fail at terraform apply with a less clear error. Adding a validation block would catch typos early.

Optional: Add email format validation
 variable "archera_gcp_service_account" {
   description = "Full service account email of the SA Archera provides during onboarding, e.g. archera-integration@archera-prod.iam.gserviceaccount.com. Required when enable_archera = true."
   type        = string
   default     = ""
+
+  validation {
+    condition     = var.archera_gcp_service_account == "" || can(regex("^[a-z][a-z0-9-]*@[a-z][a-z0-9-]*\\.iam\\.gserviceaccount\\.com$", var.archera_gcp_service_account))
+    error_message = "archera_gcp_service_account must be a valid GCP service account email (e.g. name@project.iam.gserviceaccount.com)."
+  }
 }
🤖 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 `@iac/federation/gcp-target/terraform/variables.tf` around lines 224 - 228, Add
a Terraform variable validation for archera_gcp_service_account to ensure it
matches a GCP service account email pattern (e.g.
local-part@project.iam.gserviceaccount.com) so typos are caught early; update
the variable "archera_gcp_service_account" to include a validation block that
uses a regex to validate the email format and a clear error message on failure
referencing the variable name.
frontend/src/federation.ts (1)

302-305: 💤 Low value

Consider clearing archeraContainer before appending the checkbox.

If initFederationPanel is called multiple times (e.g., during hot-reload or SPA navigation), the checkbox will be appended repeatedly without clearing the container first — unlike pillContainer which uses clearContainer via buildTargetCloudPills.

🔧 Proposed fix
   const archeraContainer = document.getElementById('federation-archera-options');
   if (archeraContainer) {
+    clearContainer(archeraContainer);
     buildArcheraCheckbox(archeraContainer);
   }
🤖 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 `@frontend/src/federation.ts` around lines 302 - 305, The archeraContainer
element is not cleared before appending a checkbox, causing duplicates on
repeated initFederationPanel calls; update the init logic to clear
archeraContainer (similar to how pillContainer is cleared via clearContainer in
buildTargetCloudPills) before calling buildArcheraCheckbox. Locate the
archeraContainer lookup and ensure you call the same clearContainer utility (or
remove existing children) on that element prior to invoking buildArcheraCheckbox
so the checkbox is not appended multiple times.
terraform/environments/gcp/variables.tf (1)

493-504: 💤 Low value

Consider adding validation for GCP service account email format.

The Azure equivalent (archera_azure_sp_object_id) validates UUID format when enable_archera = true. For consistency and earlier error detection, consider adding similar validation for the GCP service account email format.

🔧 Example validation
 variable "archera_gcp_service_account" {
   description = <<-EOT
     Full email of the GCP service account Archera provides during onboarding,
     e.g. archera-integration@archera-prod.iam.gserviceaccount.com.
     Only evaluated when enable_archera = true.

     TODO(`@cristim`): obtain the correct SA email from Archera's GCP integration
     docs before enabling.
   EOT
   type        = string
   default     = ""
+
+  validation {
+    condition = (
+      !var.enable_archera ||
+      can(regex("^[a-z][a-z0-9-]*@[a-z][a-z0-9-]*\\.iam\\.gserviceaccount\\.com$",
+        var.archera_gcp_service_account))
+    )
+    error_message = "archera_gcp_service_account must be a valid GCP service account email when enable_archera = true."
+  }
 }
🤖 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 `@terraform/environments/gcp/variables.tf` around lines 493 - 504, Add
validation to the Terraform variable archera_gcp_service_account so that when
enable_archera is true the value matches a valid GCP service account email
pattern (e.g., local-part@project.iam.gserviceaccount.com); update the variable
"archera_gcp_service_account" block to include a validation block that checks
enable_archera and uses a regex to enforce the email format, returning a clear
error message referencing "archera_gcp_service_account" when the pattern does
not match.
frontend/src/__tests__/federation.test.ts (1)

57-65: 💤 Low value

Consider resetting module-level _includeArchera state between tests.

The _includeArchera state in the federation module persists across tests since only the DOM is rebuilt and mocks are cleared. If test execution order changes, later tests could inherit state from earlier tests, causing flakiness.

One approach is to expose a reset function from the module for testing, or re-import the module fresh in each test using jest.isolateModules.

🤖 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 `@frontend/src/__tests__/federation.test.ts` around lines 57 - 65, The
module-level boolean _includeArchera in the federation module is persisting
across tests and causing flakiness; fix by ensuring _includeArchera is reset
between tests—either add and export a reset function (e.g., resetIncludeArchera)
from the federation module and call it in beforeEach/afterEach of these tests
(around initFederationPanel and buildFederationDOM usage), or re-import the
module fresh per test using jest.isolateModules so initFederationPanel sees a
clean module state; update the test setup to call the chosen reset strategy to
guarantee isolation.
internal/api/handler_federation_test.go (1)

1126-1284: ⚡ Quick win

Extend the Archera matrix to cover CLI/ARM and default-off single-file outputs.

This section only exercises bundle tfvars plus positive-path CFN/Bicep params. The feature contract here also includes CLI and ARM renders, and the default-off guarantee should hold for single-file formats too, so a formatter-specific regression can still slip through.

🤖 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 `@internal/api/handler_federation_test.go` around lines 1126 - 1284, Update the
Archera test matrix to also exercise CLI and ARM renderers and single-file
(non-bundle) outputs: add cases to
TestGetFederationIaC_IncludeArchera_False_ByteIdentical that include
"format":"cli" and "format":"arm" (and single-file formats like
"tf"/"cfn"/"bicep" when applicable) to assert absent vs include_archera="false"
produce byte-identical Filename/Content using the existing federationHandler()
and federationReq() helpers; likewise extend
TestGetFederationIaC_IncludeArchera_True_ContainsArcheraVars (and the CFN/Bicep
param tests) to include positive-path checks for CLI and ARM renders and for
single-file outputs so the default-off guarantee is validated across those
format-specific renderers, using unzipResponse() where needed to inspect
contents.
🤖 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/ci.yml:
- Around line 264-279: The new GitHub Actions job "archera-parity" is not
currently listed as a dependency of the "ci-success" job, so its failure won't
block merges; update the workflow by adding "archera-parity" to the "needs"
array of the "ci-success" job (the job currently declaring needs: [...]) so that
"ci-success" depends on and will fail if "archera-parity" fails, ensuring the
parity check is enforced.

In `@frontend/src/federation.ts`:
- Around line 160-167: The checkbox is always initialized to false causing a
UI/state mismatch with the module-level _includeArchera; update the render logic
that creates the input (the element referenced by checkbox) to set
checkbox.checked = _includeArchera so the DOM reflects the current module state,
and keep the existing change listener (checkbox.addEventListener('change', () =>
{ _includeArchera = checkbox.checked; })) to maintain two-way sync between the
checkbox and the _includeArchera variable.

In `@iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml`:
- Around line 23-29: ArcheraAwsAccountId may be left empty while
EnableArchera=true, producing an invalid IAM principal; add a CloudFormation
Rules entry (e.g., ArcheraAccountIdRequired) that asserts: if Fn::Equals
[EnableArchera, "true"] then ArcheraAwsAccountId must not equal "" (use Fn::Not
/ Fn::Equals) and provide a clear Assert message; apply the same pattern to the
other similar parameter usage referenced (the additional occurrence noted) so
the template fails validation with a user-friendly message instead of creating
an invalid principal ARN.

In `@iac/federation/aws-target/cloudformation/archera.cfn.yaml`:
- Around line 23-29: Add a CloudFormation Rules assertion to prevent
ArcheraAwsAccountId being empty when EnableArchera is true: create a Rule (e.g.,
ValidateArcheraAwsAccountId or EnsureArcheraAccountIdWhenEnabled) that asserts
when EnableArchera == true then ArcheraAwsAccountId matches the 12-digit pattern
(non-empty), and wire that Rule into the template so deployment fails with a
clear message if EnableArchera is true but ArcheraAwsAccountId is empty; apply
the same check for the referenced lines around 68-69.

In `@iac/modules/archera/aws/main.tf`:
- Around line 38-62: Add a Terraform precondition to
aws_iam_role.archera_integration to fail plan if enable_archera is true but
required inputs are empty: inside the aws_iam_role "archera_integration" block
add a precondition (Terraform 1.3+) that checks (var.enable_archera == false) ||
(trim(var.archera_aws_account_id) != "" && trim(var.archera_external_id) != "")
and provide a clear error_message referencing archera_aws_account_id and
archera_external_id so the plan fails with a helpful message when those
variables are missing.

In `@iac/modules/archera/azure/variables.tf`:
- Around line 17-21: The variable archera_azure_sp_object_id currently defaults
to empty which allows missing values despite the description requiring it when
enable_archera is true; add a Terraform validation block to the variable
"archera_azure_sp_object_id" that references var.enable_archera and enforces a
non-empty value (e.g., trimspace/length check) when var.enable_archera == true
and provide a clear error_message explaining that archera_azure_sp_object_id is
required when enable_archera is enabled.

In `@iac/modules/archera/gcp/variables.tf`:
- Around line 17-21: Add a variable-level validation for
archera_gcp_service_account to either (A) enforce format only (self-contained)
by adding a validation block on variable "archera_gcp_service_account" that
checks the value is either empty or a valid service account string (e.g.,
condition = var.archera_gcp_service_account == "" ||
!startswith(var.archera_gcp_service_account, "serviceAccount:") or use a regex
to validate the expected email format) and document that presence must be
validated at the module call site when enable_archera is true, or (B) bump
required_version to ">= 1.9" and add a cross-variable validation that uses
var.enable_archera to require archera_gcp_service_account != "" when
enable_archera is true; reference the variable name archera_gcp_service_account
and the flag enable_archera to locate where to apply the change.

In `@internal/api/handler_federation.go`:
- Around line 162-164: The handler currently sets data.IncludeArchera by
comparing req.QueryStringParameters["include_archera"] == "true", which silently
treats any malformed value as false; update the handler to validate the
include_archera query param using strconv.ParseBool on
req.QueryStringParameters["include_archera"], and if ParseBool returns an error
respond with HTTP 400 (bad request) with an explanatory message; on success set
data.IncludeArchera to the parsed boolean. Ensure you use the same
response/error pattern as other query validations in this handler so the
validation behavior is consistent.

In `@terraform/environments/aws/variables.tf`:
- Around line 540-543: The validation allows whitespace-only values for
archera_external_id; update the condition in the validation block to check the
trimmed value (use trimspace(var.archera_external_id) or length(trimspace(...))
> 0) so the expression becomes: !var.enable_archera ||
trimspace(var.archera_external_id) != "" (or equivalent), ensuring
whitespace-only strings fail validation when enable_archera is true.

---

Nitpick comments:
In `@frontend/src/__tests__/federation.test.ts`:
- Around line 57-65: The module-level boolean _includeArchera in the federation
module is persisting across tests and causing flakiness; fix by ensuring
_includeArchera is reset between tests—either add and export a reset function
(e.g., resetIncludeArchera) from the federation module and call it in
beforeEach/afterEach of these tests (around initFederationPanel and
buildFederationDOM usage), or re-import the module fresh per test using
jest.isolateModules so initFederationPanel sees a clean module state; update the
test setup to call the chosen reset strategy to guarantee isolation.

In `@frontend/src/federation.ts`:
- Around line 302-305: The archeraContainer element is not cleared before
appending a checkbox, causing duplicates on repeated initFederationPanel calls;
update the init logic to clear archeraContainer (similar to how pillContainer is
cleared via clearContainer in buildTargetCloudPills) before calling
buildArcheraCheckbox. Locate the archeraContainer lookup and ensure you call the
same clearContainer utility (or remove existing children) on that element prior
to invoking buildArcheraCheckbox so the checkbox is not appended multiple times.

In `@iac/federation/gcp-sa-impersonation/terraform/variables.tf`:
- Around line 49-53: The variable archera_gcp_service_account lacks input
validation; add a Terraform validation block for variable
"archera_gcp_service_account" to enforce a proper GCP service account email
format (e.g. local-part@project.iam.gserviceaccount.com) using a regex and a
clear error_message; update the variable block for archera_gcp_service_account
to include the validation rule so invalid values are rejected early during
terraform plan/apply.

In `@iac/federation/gcp-target/terraform/variables.tf`:
- Around line 224-228: Add a Terraform variable validation for
archera_gcp_service_account to ensure it matches a GCP service account email
pattern (e.g. local-part@project.iam.gserviceaccount.com) so typos are caught
early; update the variable "archera_gcp_service_account" to include a validation
block that uses a regex to validate the email format and a clear error message
on failure referencing the variable name.

In `@internal/api/handler_federation_test.go`:
- Around line 1126-1284: Update the Archera test matrix to also exercise CLI and
ARM renderers and single-file (non-bundle) outputs: add cases to
TestGetFederationIaC_IncludeArchera_False_ByteIdentical that include
"format":"cli" and "format":"arm" (and single-file formats like
"tf"/"cfn"/"bicep" when applicable) to assert absent vs include_archera="false"
produce byte-identical Filename/Content using the existing federationHandler()
and federationReq() helpers; likewise extend
TestGetFederationIaC_IncludeArchera_True_ContainsArcheraVars (and the CFN/Bicep
param tests) to include positive-path checks for CLI and ARM renders and for
single-file outputs so the default-off guarantee is validated across those
format-specific renderers, using unzipResponse() where needed to inspect
contents.

In `@scripts/check-archera-parity.sh`:
- Around line 97-108: The extractor extract_bicep_azure_actions only matches
single-quoted strings; update its first grep to match both single- and
double-quoted Microsoft action strings (e.g., change the regex to capture either
quote type like (['\"])Microsoft\.[^'\"\\]+\\1), then adjust the quote-stripping
step (tr -d) to remove both ' and " so the rest of the pipeline (grep -vE
'@|roleDefinitions$|roleAssignments$' | sort -u) continues to work unchanged.

In `@terraform/environments/gcp/variables.tf`:
- Around line 493-504: Add validation to the Terraform variable
archera_gcp_service_account so that when enable_archera is true the value
matches a valid GCP service account email pattern (e.g.,
local-part@project.iam.gserviceaccount.com); update the variable
"archera_gcp_service_account" block to include a validation block that checks
enable_archera and uses a regex to enforce the email format, returning a clear
error message referencing "archera_gcp_service_account" when the pattern does
not match.
🪄 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: f9fc0b93-60ad-4ced-b932-ee46d8372c2e

📥 Commits

Reviewing files that changed from the base of the PR and between 7032056 and e8c59e2.

📒 Files selected for processing (51)
  • .github/workflows/ci.yml
  • frontend/src/__tests__/federation.test.ts
  • frontend/src/api/accounts.ts
  • frontend/src/federation.ts
  • iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml
  • iac/federation/aws-cross-account/terraform/archera.tf
  • iac/federation/aws-cross-account/terraform/variables.tf
  • iac/federation/aws-target/cloudformation/archera.cfn.yaml
  • iac/federation/aws-target/terraform/archera.tf
  • iac/federation/aws-target/terraform/variables.tf
  • iac/federation/azure-target/arm/archera.arm.json
  • iac/federation/azure-target/bicep/archera.bicep
  • iac/federation/azure-target/terraform/.terraform.lock.hcl
  • iac/federation/azure-target/terraform/archera.tf
  • iac/federation/azure-target/terraform/variables.tf
  • iac/federation/gcp-sa-impersonation/terraform/archera.tf
  • iac/federation/gcp-sa-impersonation/terraform/variables.tf
  • iac/federation/gcp-target/terraform/archera.tf
  • iac/federation/gcp-target/terraform/variables.tf
  • iac/modules/archera/aws/main.tf
  • iac/modules/archera/aws/outputs.tf
  • iac/modules/archera/aws/variables.tf
  • iac/modules/archera/azure/main.tf
  • iac/modules/archera/azure/outputs.tf
  • iac/modules/archera/azure/variables.tf
  • iac/modules/archera/gcp/main.tf
  • iac/modules/archera/gcp/outputs.tf
  • iac/modules/archera/gcp/variables.tf
  • iac/modules/archera/scope.aws.yaml
  • iac/modules/archera/scope.azure.yaml
  • iac/modules/archera/scope.gcp.yaml
  • internal/api/handler_federation.go
  • internal/api/handler_federation_test.go
  • internal/iacfiles/templates/README.md
  • internal/iacfiles/templates/aws-cross-account.tfvars.tmpl
  • internal/iacfiles/templates/aws-wif.tfvars.tmpl
  • internal/iacfiles/templates/azure-wif-bicep-params.json.tmpl
  • internal/iacfiles/templates/azure-wif.tfvars.tmpl
  • internal/iacfiles/templates/gcp-sa-impersonation.tfvars.tmpl
  • internal/iacfiles/templates/gcp-wif.tfvars.tmpl
  • scripts/check-archera-parity.sh
  • scripts/generate-federation-iac.go
  • terraform/environments/aws/archera.tf
  • terraform/environments/aws/dev.tfvars.example
  • terraform/environments/aws/variables.tf
  • terraform/environments/azure/archera.tf
  • terraform/environments/azure/dev.tfvars.example
  • terraform/environments/azure/variables.tf
  • terraform/environments/gcp/archera.tf
  • terraform/environments/gcp/dev.tfvars.example
  • terraform/environments/gcp/variables.tf

Comment thread .github/workflows/ci.yml
Comment thread frontend/src/federation.ts
Comment thread iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml
Comment thread iac/federation/aws-target/cloudformation/archera.cfn.yaml
Comment thread iac/modules/archera/aws/main.tf
Comment thread iac/modules/archera/azure/variables.tf
Comment on lines +17 to +21
variable "archera_gcp_service_account" {
description = "Full service account email of the SA Archera provides during onboarding, e.g. archera-integration@archera-prod.iam.gserviceaccount.com. Required when enable_archera = true."
type = string
default = ""
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, check the directory structure and file existence
find iac/modules/archera/gcp -type f | head -20

Repository: LeanerCloud/CUDly

Length of output: 165


🏁 Script executed:

# Read the variables.tf file to see all variables defined
cat iac/modules/archera/gcp/variables.tf

Repository: LeanerCloud/CUDly

Length of output: 1196


🏁 Script executed:

# Search for enable_archera variable definition
rg "variable.*enable_archera" --type tf

Repository: LeanerCloud/CUDly

Length of output: 1862


🏁 Script executed:

# Look for usage of archera_gcp_service_account in the module
rg "archera_gcp_service_account" iac/modules/archera/gcp

Repository: LeanerCloud/CUDly

Length of output: 235


🏁 Script executed:

# Check how these variables are used in main.tf or other resource files
ls -la iac/modules/archera/gcp/

Repository: LeanerCloud/CUDly

Length of output: 352


🏁 Script executed:

cat iac/modules/archera/gcp/main.tf

Repository: LeanerCloud/CUDly

Length of output: 1892


🏁 Script executed:

web_search
Terraform variable validation block reference another variable

Repository: LeanerCloud/CUDly

Length of output: 160


🌐 Web query:

Terraform variable validation block reference another variable cross-variable

💡 Result:

Terraform variable validation blocks can reference other input variables, locals, data sources, and resources (as long as you don’t create cyclic dependencies), but the validation rule must still include the value of the variable being validated (var.). Terraform validates that expression in the context of the variable call site and errors if the condition evaluates to false. In practice: 1) For Terraform <1.9: historically, variable validation “condition” could refer only to the variable being validated (not other variables) [1][2]. 2) For Terraform 1.9+: conditions can refer to other input variables and other objects, enabling cross-variable checks (example: cluster_endpoint validated depending on create_cluster) [3]. 3) But you cannot write a validation condition that is entirely unrelated to the variable being validated; the validation must still be about the variable’s own value [4]. 4) Terraform docs on custom conditions state input variable validations are evaluated during planning and stop the operation if the condition is unmet [5]. Example: cross-variable validation (Terraform 1.9+) variable "create_cluster" { type = bool default = false } variable "cluster_endpoint" { type = string default = "" validation { # condition can reference other variables condition = var.create_cluster == false? length(var.cluster_endpoint) > 0: true error_message = "You must specify a value for cluster_endpoint if create_cluster is false." } } This works because the validation condition depends on var.cluster_endpoint (the variable being validated) and optionally references var.create_cluster (the other variable) [3].

Citations:


Enforce archera_gcp_service_account when Archera is enabled.

Lines 17–21 allow an empty value even when enable_archera = true, which causes failures later with less actionable errors. A variable-level validation is needed, but the suggested patch uses cross-variable references that require Terraform 1.9+ while this module declares required_version = ">= 1.5".

Use validation that depends only on the variable itself (e.g., !startswith(var.archera_gcp_service_account, "serviceAccount:")), require a minimum Terraform version of 1.9 to enable cross-variable validation, or validate at the module call site.

🤖 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 `@iac/modules/archera/gcp/variables.tf` around lines 17 - 21, Add a
variable-level validation for archera_gcp_service_account to either (A) enforce
format only (self-contained) by adding a validation block on variable
"archera_gcp_service_account" that checks the value is either empty or a valid
service account string (e.g., condition = var.archera_gcp_service_account == ""
|| !startswith(var.archera_gcp_service_account, "serviceAccount:") or use a
regex to validate the expected email format) and document that presence must be
validated at the module call site when enable_archera is true, or (B) bump
required_version to ">= 1.9" and add a cross-variable validation that uses
var.enable_archera to require archera_gcp_service_account != "" when
enable_archera is true; reference the variable name archera_gcp_service_account
and the flag enable_archera to locate where to apply the change.

Comment thread internal/api/handler_federation.go Outdated
Comment thread terraform/environments/aws/variables.tf
- ci: add archera-parity to ci-success needs so parity failures block merges
- api: parse include_archera with strconv.ParseBool, return 400 on malformed input
- tf/aws: tighten archera_external_id validation to reject whitespace-only values
- tf/aws-module: add lifecycle precondition for required inputs when enable_archera=true
- tf/azure-module: add validation requiring archera_azure_sp_object_id when enable_archera=true
- tf/gcp-module: add SA email format validation when enable_archera=true
- tf/gcp-federation: add SA email format validation in gcp-sa-impersonation + gcp-target vars
- tf/env-gcp: add SA email format validation in environment variables
- cfn: add Rules assertions for ArcheraAwsAccountId/ExternalId when EnableArchera=true
- frontend: sync checkbox.checked to _includeArchera on render; clearContainer before re-render
- frontend: export _resetIncludeArcheraForTesting and call it in all beforeEach blocks
- scripts: handle double-quoted Bicep action strings in extract_bicep_azure_actions
@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 6, 2026

Addressed all 9 Actionable findings and 6 of 7 Nitpick items from the last CR review:

Actionable — fixed:

  • ci: archera-parity added to ci-success needs (gate now enforced)
  • api: include_archera parsed via strconv.ParseBool, returns 400 on malformed values
  • tf: archera_external_id validation rejects whitespace-only values (trimspace)
  • tf/aws-module: lifecycle precondition requires account_id + external_id when enable_archera=true
  • tf/azure-module: validation requires archera_azure_sp_object_id when enable_archera=true
  • tf/gcp-module: validation requires valid SA email format when enable_archera=true
  • cfn: added Rules assertions for ArcheraAwsAccountId/ExternalId in both aws-cross-account and aws-target templates
  • frontend: checkbox.checked now initialised to _includeArchera (no more stale-state on re-render); clearContainer called before buildArcheraCheckbox
  • frontend/tests: _resetIncludeArcheraForTesting exported + called in all beforeEach blocks

Nitpicks — fixed:

  • scripts/check-archera-parity.sh: Bicep extractor now handles double-quoted action strings
  • tf/gcp: SA email validation added to gcp-sa-impersonation, gcp-target federation vars + terraform/environments/gcp

Nitpick — dismissed with justification:

  • handler_federation_test.go: extending the Archera matrix to cover CLI/ARM and single-file renders is a valid follow-up but goes beyond the feature contract of this PR (the handlers don't add Archera content to CLI/ARM formats). Filing a separate issue.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Tip

For best results, initiate chat on the files or code changes.

@cristim Thank you for the thorough update — it's great to see all 9 actionable findings resolved along with most of the nitpicks. The fixes look comprehensive:

  • The CI gate enforcement, strconv.ParseBool with 400 on malformed input, and the Terraform precondition/validation additions are all solid improvements to the overall robustness of the feature.
  • The frontend stale-state fix (checkbox.checked initialized to _includeArchera) and the _resetIncludeArcheraForTesting guard in tests are exactly the right patterns.
  • The dismissal of the handler test matrix extension is reasonable — keeping it scoped to the feature contract and tracking the remainder as a follow-up issue is the right call.

Kicking off a fresh review now.

[review]

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 6, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scripts/generate-federation-iac.go (1)

82-102: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

iacData no longer mirrors template requirements, causing local render failures.

Line 82-Line 102 omits fields that AWS templates use (CUDlyAPIURL, ContactEmail, SourceAccountID), so template.Execute can fail at runtime with “can’t evaluate field … in type iacData.” Line 345-Line 346 also cannot populate these values today.

Proposed patch (minimal parity fix)
 type iacData struct {
 	AccountName       string
 	AccountExternalID string
 	AccountSlug       string
 	Source            string
+	SourceAccountID   string
 	// AWS WIF / cross-account
-	OIDCIssuerURL string
-	OIDCAudience  string
+	OIDCIssuerURL  string
+	OIDCIssuerHost string
+	OIDCAudience   string
 	// Azure-specific
 	SubscriptionID string
 	TenantID       string
 	// GCP-specific
 	ProjectID           string
 	ServiceAccountEmail string
 	OIDCIssuerURI       string
+	CUDlyAPIURL         string
+	ContactEmail        string
 	// IncludeArchera controls whether Archera Insurance permission resources are
 	// included in the rendered output. Mirrors handler_federation.go:federationIaCData.
 	IncludeArchera bool
 }
@@
 func populateData(data *iacData, target, source, tenantID, projectID, saEmail string) bool {
 	switch target {
 	case "aws":
 		data.OIDCIssuerURL = awsOIDCIssuer(source, tenantID)
+		data.OIDCIssuerHost = strings.TrimPrefix(data.OIDCIssuerURL, "https://")
 		data.OIDCAudience = awsOIDCAudience(source)
@@
-	data := iacData{AccountName: *accountName, AccountExternalID: *accountID, AccountSlug: slug, Source: *source, IncludeArchera: *includeArchera}
+	data := iacData{
+		AccountName:       *accountName,
+		AccountExternalID: *accountID,
+		AccountSlug:       slug,
+		Source:            *source,
+		IncludeArchera:    *includeArchera,
+	}

Also applies to: 345-346

🤖 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 `@scripts/generate-federation-iac.go` around lines 82 - 102, The iacData struct
used for template rendering is missing fields required by the AWS templates
(CUDlyAPIURL, ContactEmail, SourceAccountID), causing template.Execute to fail;
update the iacData type to include these three fields (CUDlyAPIURL,
ContactEmail, SourceAccountID) and then set/populate them where the iacData
instance is constructed (the code that currently fills iacData around the
population site referenced near lines 345-346) so the template has those values
available at render time.
♻️ Duplicate comments (1)
iac/modules/archera/gcp/main.tf (1)

9-16: ⚠️ Potential issue | 🟠 Major

required_version no longer matches the validation features this module uses.

The sibling variables.tf now references var.enable_archera inside another variable's validation, which is only supported in Terraform 1.9+. Keeping >= 1.5 advertises compatibility for 1.5-1.8 even though those versions will fail before plan.

Did Terraform support referencing another input variable inside an input variable `validation` block before Terraform 1.9?
🤖 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 `@iac/modules/archera/gcp/main.tf` around lines 9 - 16, The terraform block's
required_version (symbol: required_version in main.tf) advertises compatibility
with Terraform 1.5–1.8 but the module uses cross-variable validation
(referencing var.enable_archera inside another variable's validation) which
requires Terraform 1.9+, so update required_version to require Terraform 1.9 or
newer (e.g., change required_version to ">= 1.9.0") to match the module's
validation features and prevent users on older Terraform versions from
attempting to run the module.
🧹 Nitpick comments (3)
frontend/src/__tests__/federation.test.ts (2)

193-224: ⚡ Quick win

Rebuild the panel when testing persisted Archera state.

This test proves the module variable survives a pill switch, but not that the setting persists when initFederationPanel() is run again from saved federation state. A teardown + re-init assertion would cover the persistence requirement from #314 much better.

🤖 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 `@frontend/src/__tests__/federation.test.ts` around lines 193 - 224, The test
currently verifies the module variable survives a pill switch but doesn't assert
persistence across a full panel rebuild; modify the test (test 'checkbox state
persists after switching target-cloud pill') to teardown/unmount the federation
panel after switching pills, then call initFederationPanel(...) again to
simulate reloading from saved federation state, trigger the download button and
assert mockGetFederationIaC was called with includeArchera === true; use the
existing helpers initFederationPanel, archeraCheckbox, and the '.btn-download'
selector to locate elements and mockGetFederationIaC to inspect the call
arguments.

85-93: ⚡ Quick win

Assert the tooltip linkage, not just the hidden copy.

This only proves an sr-only node exists. It will not catch regressions where the checkbox or help trigger stops referencing that text via aria-describedby or an equivalent accessible relationship, which is the actual accessibility contract here.

🤖 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 `@frontend/src/__tests__/federation.test.ts` around lines 85 - 93, The test
only checks the presence of the hidden tooltip node
('federation-archera-tooltip') but not that the related control actually
references it; after calling initFederationPanel('aws') find the tooltip via
getElementById('federation-archera-tooltip') and then assert the corresponding
control references it (e.g., use
document.querySelector(`[aria-describedby="${tooltip.id}"]`) or locate the
checkbox/help trigger and assert its aria-describedby or aria-labelledby equals
the tooltip id) so the accessibility relationship is enforced; keep references
to initFederationPanel and 'federation-archera-tooltip' when adding the
assertion.
internal/api/handler_federation_test.go (1)

1126-1284: ⚡ Quick win

Add a regression test for malformed include_archera values.

This section covers absent/false/true, but the handler change also rejects invalid booleans with 400. Pinning something like include_archera=maybe here would keep that boundary from silently regressing.

Suggested test
+func TestGetFederationIaC_RejectsMalformedIncludeArchera(t *testing.T) {
+	h := federationHandler()
+
+	_, err := h.getFederationIaC(context.Background(), federationReq(map[string]string{
+		"target": "aws", "source": "gcp", "format": "bundle", "include_archera": "maybe",
+	}))
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok)
+	assert.Equal(t, 400, ce.code)
+	assert.Contains(t, ce.Error(), "include_archera")
+}
🤖 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 `@internal/api/handler_federation_test.go` around lines 1126 - 1284, Add a new
test (e.g., TestGetFederationIaC_IncludeArchera_Malformed_Returns400) that uses
federationHandler() and calls getFederationIaC with an invalid include_archera
value like "maybe"; assert the call returns an error and that the handler
responds with a 400-level validation error (check the returned error/response
indicates bad request) to ensure malformed booleans are rejected. Reference the
existing tests TestGetFederationIaC_IncludeArchera_False_ByteIdentical and
TestGetFederationIaC_IncludeArchera_True_* for setup and use federationReq to
pass "include_archera": "maybe" so the regression (accepting invalid booleans)
is caught. Ensure assertions explicitly verify 400 semantics rather than success
or nil error.
🤖 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 `@frontend/src/federation.ts`:
- Around line 182-189: The accessible description is attached to the label
instead of the focusable checkbox; move the title and aria-describedby from the
label to the checkbox input element (the focusable <input> variable used when
creating the checkbox) so screen readers get ARCHERA_TOOLTIP when tabbing to the
control, keep tooltipSpan with id 'federation-archera-tooltip' and class
'federation-archera-tooltip sr-only' and ensure it is inserted into the DOM, and
remove the aria-describedby on the label.

In `@iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml`:
- Around line 31-37: The ArcheraExternalId parameter currently allows
whitespace-only values; update the ArcheraExternalId parameter (and any other
parameters guarded only against empty string) to enforce at least one
non-whitespace character by adding an AllowedPattern that requires a non-space
character (e.g. '.*\\S.*') and a ConstraintDescription explaining the
requirement so CloudFormation rejects whitespace-only values during validation.

In `@iac/federation/aws-target/cloudformation/archera.cfn.yaml`:
- Around line 31-38: The ArcheraExternalId parameter currently allows
whitespace-only values; update the ArcheraExternalId parameter to reject strings
that contain only whitespace by adding an AllowedPattern that requires at least
one non-space character (e.g. AllowedPattern: ".*\\S.*") and a useful
ConstraintDescription, so the CloudFormation path mirrors the Terraform trim
guard; reference the ArcheraExternalId parameter and the
ArcheraExternalIdRequired usage when making this change.

In `@iac/modules/archera/scope.gcp.yaml`:
- Around line 8-11: The PROVISIONAL SCOPE comment and the permission lists in
this YAML must be verified against Archera's official GCP integration docs
before enabling the feature; locate the PROVISIONAL SCOPE block and the
permission entries referenced by enable_archera, confirm the read and purchase
IAM permissions exactly match Archera's published requirements, update the
permission entries to match the docs, and replace the PROVISIONAL label with a
clear verified note (and date/source reference) once confirmed.

In `@scripts/check-archera-parity.sh`:
- Around line 77-83: The script currently merges yaml_read and yaml_purchase
into yaml_all and diffs that against cfn_actions, which hides drift when an
action moves between read and purchase; instead, call extract_cfn_aws_actions
twice (e.g., extract_cfn_aws_read_actions and extract_cfn_aws_purchase_actions
or filter cfn_actions into cfn_read_actions and cfn_purchase_actions) and
compare yaml_read to the Cfn read list and yaml_purchase to the Cfn purchase
list separately (update variables yaml_read, yaml_purchase, cfn_actions, and
diff_output usage accordingly), and apply the same separate-compare fix to the
other similar blocks that perform the union (the other occurrences of this
union/diff pattern).
- Around line 187-191: The AWS branch's inline_actions grep only allows letters
for the service name and thus misses services like
"ec2:DescribeReservedInstances"; update the aws case grep to accept digits and
hyphens in the service portion and allow wildcard '*' in the action portion
(e.g., change the regex used in the aws branch that sets inline_actions to
something like '"[a-z0-9-]+:[A-Za-z*]+"' so functions/variables inline_actions
and the aws case match ec2 and similar actions).

---

Outside diff comments:
In `@scripts/generate-federation-iac.go`:
- Around line 82-102: The iacData struct used for template rendering is missing
fields required by the AWS templates (CUDlyAPIURL, ContactEmail,
SourceAccountID), causing template.Execute to fail; update the iacData type to
include these three fields (CUDlyAPIURL, ContactEmail, SourceAccountID) and then
set/populate them where the iacData instance is constructed (the code that
currently fills iacData around the population site referenced near lines
345-346) so the template has those values available at render time.

---

Duplicate comments:
In `@iac/modules/archera/gcp/main.tf`:
- Around line 9-16: The terraform block's required_version (symbol:
required_version in main.tf) advertises compatibility with Terraform 1.5–1.8 but
the module uses cross-variable validation (referencing var.enable_archera inside
another variable's validation) which requires Terraform 1.9+, so update
required_version to require Terraform 1.9 or newer (e.g., change
required_version to ">= 1.9.0") to match the module's validation features and
prevent users on older Terraform versions from attempting to run the module.

---

Nitpick comments:
In `@frontend/src/__tests__/federation.test.ts`:
- Around line 193-224: The test currently verifies the module variable survives
a pill switch but doesn't assert persistence across a full panel rebuild; modify
the test (test 'checkbox state persists after switching target-cloud pill') to
teardown/unmount the federation panel after switching pills, then call
initFederationPanel(...) again to simulate reloading from saved federation
state, trigger the download button and assert mockGetFederationIaC was called
with includeArchera === true; use the existing helpers initFederationPanel,
archeraCheckbox, and the '.btn-download' selector to locate elements and
mockGetFederationIaC to inspect the call arguments.
- Around line 85-93: The test only checks the presence of the hidden tooltip
node ('federation-archera-tooltip') but not that the related control actually
references it; after calling initFederationPanel('aws') find the tooltip via
getElementById('federation-archera-tooltip') and then assert the corresponding
control references it (e.g., use
document.querySelector(`[aria-describedby="${tooltip.id}"]`) or locate the
checkbox/help trigger and assert its aria-describedby or aria-labelledby equals
the tooltip id) so the accessibility relationship is enforced; keep references
to initFederationPanel and 'federation-archera-tooltip' when adding the
assertion.

In `@internal/api/handler_federation_test.go`:
- Around line 1126-1284: Add a new test (e.g.,
TestGetFederationIaC_IncludeArchera_Malformed_Returns400) that uses
federationHandler() and calls getFederationIaC with an invalid include_archera
value like "maybe"; assert the call returns an error and that the handler
responds with a 400-level validation error (check the returned error/response
indicates bad request) to ensure malformed booleans are rejected. Reference the
existing tests TestGetFederationIaC_IncludeArchera_False_ByteIdentical and
TestGetFederationIaC_IncludeArchera_True_* for setup and use federationReq to
pass "include_archera": "maybe" so the regression (accepting invalid booleans)
is caught. Ensure assertions explicitly verify 400 semantics rather than success
or nil error.
🪄 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: 12799e33-7e7d-4516-a5f8-687c38dcbac5

📥 Commits

Reviewing files that changed from the base of the PR and between 7032056 and 70a885f.

📒 Files selected for processing (51)
  • .github/workflows/ci.yml
  • frontend/src/__tests__/federation.test.ts
  • frontend/src/api/accounts.ts
  • frontend/src/federation.ts
  • iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml
  • iac/federation/aws-cross-account/terraform/archera.tf
  • iac/federation/aws-cross-account/terraform/variables.tf
  • iac/federation/aws-target/cloudformation/archera.cfn.yaml
  • iac/federation/aws-target/terraform/archera.tf
  • iac/federation/aws-target/terraform/variables.tf
  • iac/federation/azure-target/arm/archera.arm.json
  • iac/federation/azure-target/bicep/archera.bicep
  • iac/federation/azure-target/terraform/.terraform.lock.hcl
  • iac/federation/azure-target/terraform/archera.tf
  • iac/federation/azure-target/terraform/variables.tf
  • iac/federation/gcp-sa-impersonation/terraform/archera.tf
  • iac/federation/gcp-sa-impersonation/terraform/variables.tf
  • iac/federation/gcp-target/terraform/archera.tf
  • iac/federation/gcp-target/terraform/variables.tf
  • iac/modules/archera/aws/main.tf
  • iac/modules/archera/aws/outputs.tf
  • iac/modules/archera/aws/variables.tf
  • iac/modules/archera/azure/main.tf
  • iac/modules/archera/azure/outputs.tf
  • iac/modules/archera/azure/variables.tf
  • iac/modules/archera/gcp/main.tf
  • iac/modules/archera/gcp/outputs.tf
  • iac/modules/archera/gcp/variables.tf
  • iac/modules/archera/scope.aws.yaml
  • iac/modules/archera/scope.azure.yaml
  • iac/modules/archera/scope.gcp.yaml
  • internal/api/handler_federation.go
  • internal/api/handler_federation_test.go
  • internal/iacfiles/templates/README.md
  • internal/iacfiles/templates/aws-cross-account.tfvars.tmpl
  • internal/iacfiles/templates/aws-wif.tfvars.tmpl
  • internal/iacfiles/templates/azure-wif-bicep-params.json.tmpl
  • internal/iacfiles/templates/azure-wif.tfvars.tmpl
  • internal/iacfiles/templates/gcp-sa-impersonation.tfvars.tmpl
  • internal/iacfiles/templates/gcp-wif.tfvars.tmpl
  • scripts/check-archera-parity.sh
  • scripts/generate-federation-iac.go
  • terraform/environments/aws/archera.tf
  • terraform/environments/aws/dev.tfvars.example
  • terraform/environments/aws/variables.tf
  • terraform/environments/azure/archera.tf
  • terraform/environments/azure/dev.tfvars.example
  • terraform/environments/azure/variables.tf
  • terraform/environments/gcp/archera.tf
  • terraform/environments/gcp/dev.tfvars.example
  • terraform/environments/gcp/variables.tf

Comment on lines +182 to +189
// Tooltip via title attribute — accessible on keyboard focus and hover.
label.title = ARCHERA_TOOLTIP;
label.setAttribute('aria-describedby', 'federation-archera-tooltip');

const tooltipSpan = document.createElement('span');
tooltipSpan.id = 'federation-archera-tooltip';
tooltipSpan.className = 'federation-archera-tooltip sr-only';
tooltipSpan.textContent = ARCHERA_TOOLTIP;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Attach the help text to the checkbox itself.

The accessible description is currently bound to the <label>, not the focusable <input>, so screen-reader users won't get the Archera tooltip when they tab to the checkbox.

[some details omitted for brevity]

Suggested fix
-  // Tooltip via title attribute — accessible on keyboard focus and hover.
-  label.title = ARCHERA_TOOLTIP;
-  label.setAttribute('aria-describedby', 'federation-archera-tooltip');
+  // Bind the description to the focusable control.
+  label.title = ARCHERA_TOOLTIP;
+  checkbox.title = ARCHERA_TOOLTIP;
+  checkbox.setAttribute('aria-describedby', 'federation-archera-tooltip');
🤖 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 `@frontend/src/federation.ts` around lines 182 - 189, The accessible
description is attached to the label instead of the focusable checkbox; move the
title and aria-describedby from the label to the checkbox input element (the
focusable <input> variable used when creating the checkbox) so screen readers
get ARCHERA_TOOLTIP when tabbing to the control, keep tooltipSpan with id
'federation-archera-tooltip' and class 'federation-archera-tooltip sr-only' and
ensure it is inserted into the DOM, and remove the aria-describedby on the
label.

Comment on lines +31 to +37
ArcheraExternalId:
Type: String
Default: ""
NoEcho: true
Description: >-
External ID for confused-deputy protection on the Archera assume-role.
Obtain from Archera during onboarding. Required when EnableArchera = true.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject whitespace-only ArcheraExternalId.

Lines 56-62 only guard against "", so a value like " " still passes validation and creates a trust policy that Archera will not be able to match during onboarding. The Terraform path already rejects whitespace-only external IDs, so the CloudFormation path is still looser here.

🔧 Suggested fix
   ArcheraExternalId:
     Type: String
     Default: ""
     NoEcho: true
+    AllowedPattern: "^$|.*\\S.*$"
     Description: >-
       External ID for confused-deputy protection on the Archera assume-role.
       Obtain from Archera during onboarding. Required when EnableArchera = true.
+    ConstraintDescription: Must contain at least one non-whitespace character or be empty.

Also applies to: 56-62

🤖 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 `@iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml` around
lines 31 - 37, The ArcheraExternalId parameter currently allows whitespace-only
values; update the ArcheraExternalId parameter (and any other parameters guarded
only against empty string) to enforce at least one non-whitespace character by
adding an AllowedPattern that requires a non-space character (e.g. '.*\\S.*')
and a ConstraintDescription explaining the requirement so CloudFormation rejects
whitespace-only values during validation.

Comment on lines +31 to +38
ArcheraExternalId:
Type: String
Default: ""
NoEcho: true
Description: >-
External ID for confused-deputy protection on the Archera assume-role.
Obtain from Archera during onboarding. Required when EnableArchera = true.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject whitespace-only ArcheraExternalId.

ArcheraExternalIdRequired only rejects "", so a value like " " still passes and produces a role Archera won't be able to assume. The Terraform path already trims this case; the CloudFormation path should mirror that guard.

Suggested fix
   ArcheraExternalId:
     Type: String
     Default: ""
     NoEcho: true
+    AllowedPattern: "^$|.*\\S.*$"
+    ConstraintDescription: Must be empty or contain at least one non-whitespace character.
     Description: >-
       External ID for confused-deputy protection on the Archera assume-role.
       Obtain from Archera during onboarding. Required when EnableArchera = true.
📝 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
ArcheraExternalId:
Type: String
Default: ""
NoEcho: true
Description: >-
External ID for confused-deputy protection on the Archera assume-role.
Obtain from Archera during onboarding. Required when EnableArchera = true.
ArcheraExternalId:
Type: String
Default: ""
NoEcho: true
AllowedPattern: "^$|.*\\S.*$"
ConstraintDescription: Must be empty or contain at least one non-whitespace character.
Description: >-
External ID for confused-deputy protection on the Archera assume-role.
Obtain from Archera during onboarding. Required when EnableArchera = true.
🤖 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 `@iac/federation/aws-target/cloudformation/archera.cfn.yaml` around lines 31 -
38, The ArcheraExternalId parameter currently allows whitespace-only values;
update the ArcheraExternalId parameter to reject strings that contain only
whitespace by adding an AllowedPattern that requires at least one non-space
character (e.g. AllowedPattern: ".*\\S.*") and a useful ConstraintDescription,
so the CloudFormation path mirrors the Terraform trim guard; reference the
ArcheraExternalId parameter and the ArcheraExternalIdRequired usage when making
this change.

Comment on lines +8 to +11
# PROVISIONAL SCOPE — must be confirmed against Archera integration docs
# before flipping enable_archera = true in any tfvars.
# TODO(@cristim): confirm Archera scope list against integration docs
# before enabling. Reference: https://archera.ai/docs (integration guide).
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

Find Archera's current integration documentation for AWS, Azure, and GCP, and list the exact read-only permissions/roles plus any optional purchase or commitment-write permissions each cloud requires.

💡 Result:

I could only find Archera’s detailed IAM permission lists for AWS (trial + production). I did not find any current, authoritative Archera integration documentation that enumerates the exact Azure and GCP read-only roles/permissions (or their optional commitment-write/commitment purchase permissions). AWS (Archera “least-privileged” IAM permissions) A) Read-only permissions (trial deployment) Required for the Trial deployment and described as “Read-Only, Least-Privileged”. The specific read-only IAM actions are: - iam:GetRolePolicy - iam:ListRolePolicies - iam:ListAttachedRolePolicies - iam:GetPolicy - iam:GetPolicyVersion - iam:SimulatePrincipalPolicy Resource restriction shown for the trial credential (exact ARNs): - arn:aws:iam:::role/ReservedAI - arn:aws:iam:::role/ReservedAI-Read - arn:aws:iam::*:role/ReservedAI-Write [1] B) Additional permissions for commitment management (production deployment) — includes optional commitment-write Archera states the Production deployment adds additional least-privileged “write” permissions that “allow you to automate all commitment management tasks (purchase, exchange, resell, renewal, etc.)” without read access beyond bare minimum usage/cost metadata. The page enumerates the commitment-write actions as (exact strings as listed): - ec2:ModifyReservedInstances - ec2:PurchaseReservedInstancesOffering - ec2:AcceptReservedInstancesExchangeQuote - ec2:CreateReservedInstancesListing - ec2:CancelReservedInstancesListing - ec2:PurchaseScheduledInstances - ec2:RunScheduledInstances - ec2:ModifyCapacityReservation - ec2:ModifyInstanceCapacityReservationAttributes - ec2:CreateCapacityReservation - ec2:CancelCapacityReservation - ec2:RequestSpotFleet - ec2:RequestSpotInstances - ec2:CancelSpotFleetRequests - rds:PurchaseReserved - redshift:GetReserved - redshift:PurchaseReserved - redshift:AcceptReserved - redshift:PurchaseReservedCache [2] Optional organization-management “write” permissions (production page): - organizations:InviteAccountToOrganization - organizations:RemoveAccountFromOrganization - organizations:CreateAccount [2] What Archera says about “no infrastructure access” (context) Archera describes its AWS access as read-only of billing/usage metadata and no ability to access/impact underlying infrastructure, both in general security description and in the trial/production onboarding pages. [3][1][4] Azure and GCP - For Azure and GCP, I only found high-level onboarding descriptions (e.g., “Reader Permissions: Billing and cost management access” for Azure and “Billing permissions… Read-Only Access: No modification permissions” for GCP), but not the exact enumerated read-only roles/permissions and the optional commitment-write/commitment purchase permissions lists that your question requires. [5] - I also found Archera’s supported-services page indicating AWS+Azure GA and GCP private beta, but it still does not provide the exact Azure/GCP permission-action lists. [6] If you want, I can produce the exact Azure and GCP read-only + commitment-write action/role lists, but I’ll need an additional targeted doc source from Archera (e.g., specific pages under docs.archera.ai for “Azure onboarding/iam permissions” and “GCP onboarding/iam permissions”) or permission lists for Azure/GCP that I can extract. Right now, the required enumerations are not present in the sources I could locate.

Citations:


🏁 Script executed:

# Find the file and verify it exists
git ls-files | grep -i "scope.*gcp\|archera"

Repository: LeanerCloud/CUDly

Length of output: 1108


🏁 Script executed:

# Read the actual file to verify lines 8-11
cat -n iac/modules/archera/scope.gcp.yaml | head -20

Repository: LeanerCloud/CUDly

Length of output: 1100


🏁 Script executed:

# Search for other scope files (AWS, Azure) to confirm this is part of a multi-cloud pattern
fd -t f "scope\." --search-path=.

Repository: LeanerCloud/CUDly

Length of output: 174


🏁 Script executed:

# Check for dependencies/imports of this scope file
rg -t yaml "scope\.gcp" --no-heading -A 2 -B 2

Repository: LeanerCloud/CUDly

Length of output: 329


🏁 Script executed:

# Look for enable_archera flags and feature gates
rg -i "enable_archera|archera.*flag|archera.*beta" --no-heading -C 2

Repository: LeanerCloud/CUDly

Length of output: 50373


Complete verification against Archera's GCP integration docs before customers opt-in.

Lines 8-11 mark the permission set as unverified. While the feature defaults to enable_archera = false (good), the web search found that Archera's GCP IAM permission documentation is not publicly enumerated to the same detail as AWS. Because this YAML is the canonical source (lines 1-6), customers who opt in get whatever permissions are listed here without knowing if they match Archera's actual requirements. Before promotion, confirm the read/purchase permission lists against Archera's GCP integration documentation and update the PROVISIONAL label.

🤖 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 `@iac/modules/archera/scope.gcp.yaml` around lines 8 - 11, The PROVISIONAL
SCOPE comment and the permission lists in this YAML must be verified against
Archera's official GCP integration docs before enabling the feature; locate the
PROVISIONAL SCOPE block and the permission entries referenced by enable_archera,
confirm the read and purchase IAM permissions exactly match Archera's published
requirements, update the permission entries to match the docs, and replace the
PROVISIONAL label with a clear verified note (and date/source reference) once
confirmed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Deferring per the issue body — the Archera scope across all three clouds is provisional pending @cristim's confirmation against Archera's integration docs (this caveat is documented in the issue, the scope.yaml header comments, the README, and the per-resource TODOs). Once @cristim confirms the canonical scope, a single follow-up commit updates scope.{aws,azure,gcp}.yaml and the CI parity check ensures the CFN/Bicep/ARM literals are kept in sync automatically. No content edits in this PR.

Comment on lines +77 to +83
local yaml_read yaml_purchase yaml_all cfn_actions diff_output
yaml_read=$(yaml_list "$AWS_SCOPE_YAML" "read_actions")
yaml_purchase=$(yaml_list "$AWS_SCOPE_YAML" "purchase_actions")
yaml_all=$(printf '%s\n%s\n' "$yaml_read" "$yaml_purchase" | sort -u)

cfn_actions=$(extract_cfn_aws_actions "$cfn_file")
diff_output=$(diff <(echo "$yaml_all") <(echo "$cfn_actions") || true)
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Preserve read-vs-purchase parity instead of diffing the union.

These checks flatten read_actions and purchase_actions into one set before comparing. If a purchase permission drifts into the always-on/read block, the union stays identical and CI still passes, so the parity gate misses a privilege escalation. Compare the two categories against their corresponding template sections separately.

Also applies to: 129-135, 155-161

🤖 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 `@scripts/check-archera-parity.sh` around lines 77 - 83, The script currently
merges yaml_read and yaml_purchase into yaml_all and diffs that against
cfn_actions, which hides drift when an action moves between read and purchase;
instead, call extract_cfn_aws_actions twice (e.g., extract_cfn_aws_read_actions
and extract_cfn_aws_purchase_actions or filter cfn_actions into cfn_read_actions
and cfn_purchase_actions) and compare yaml_read to the Cfn read list and
yaml_purchase to the Cfn purchase list separately (update variables yaml_read,
yaml_purchase, cfn_actions, and diff_output usage accordingly), and apply the
same separate-compare fix to the other similar blocks that perform the union
(the other occurrences of this union/diff pattern).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Deferring this one to a follow-up PR — heavy lift to refactor the extractors per-policy-block (CFN's read/purchase PolicyDocument.Statement, ARM's two role-definition resources, Bicep is easier with its var readActions / var purchaseActions). The current flat-union check still catches any total-set drift (any action added/removed/typo'd vs scope.yaml fails CI). The split would additionally catch a permission migrating between read-only and purchase blocks within the same total set, which is a real but narrower correctness concern. Will track as a parity-script enhancement in a separate issue.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@cristim, that's a reasonable trade-off — the flat-union gate already catches any net change in the total permission set, and the narrower per-block drift scenario (a permission silently migrating between read-only and purchase without changing the union) is a lower-probability but still meaningful correctness concern worth tracking separately.

Would you like me to open a GitHub issue to track the per-policy-block parity enhancement? I can capture the context — CFN PolicyDocument.Statement split, ARM dual role-definition resources, and Bicep var readActions/var purchaseActions — as the scope for the follow-up work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@coderabbitai yes please — go ahead and open the follow-up issue. Capture the per-policy-block parity scope: CFN PolicyDocument.Statement split, ARM dual role-definition resources, Bicep var readActions/var purchaseActions. Use the standard CUDly triage labels (type/chore, severity/low, priority/p3, urgency/eventually, impact/internal, effort/m, triaged) and reference this PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check available labels in the repo
gh label list --repo LeanerCloud/CUDly --limit 100

Length of output: 1749


🏁 Script executed:

#!/bin/bash
gh issue create \
  --repo LeanerCloud/CUDly \
  --title "chore(scripts): split check-archera-parity.sh into per-policy-block comparisons" \
  --label "type/chore,severity/low,priority/p3,urgency/eventually,impact/internal,effort/m,triaged" \
  --body "## Background

The current \`scripts/check-archera-parity.sh\` merges \`read_actions\` and \`purchase_actions\` from the canonical scope YAML into a single union set before diffing against CFN/Bicep/ARM templates. This catches any net change in the total permission set but misses a permission silently migrating between the read-only and purchase blocks without altering the union.

This was deferred from PR `#315` (feat/issue-314-archera-bundle-flag) as a heavy-lift follow-up.

## Scope

Refactor the parity script to compare \`read_actions\` and \`purchase_actions\` separately against their corresponding per-block representations in each template format:

- **CloudFormation** (\`iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml\`, \`iac/federation/aws-target/cloudformation/archera.cfn.yaml\`): split extraction by \`PolicyDocument.Statement\` — one for the read policy resource and one for the purchase policy resource.
- **ARM** (\`iac/federation/azure-target/arm/archera.arm.json\`): the template contains two role-definition resources (read vs purchase); extract actions from each separately.
- **Bicep** (\`iac/federation/azure-target/bicep/archera.bicep\`): already uses named variables \`var readActions\` and \`var purchaseActions\`; extract each variable's array and diff individually.

## Acceptance criteria

- \`check_aws_cfn_parity\`, \`check_azure_bicep_parity\`, and \`check_azure_arm_parity\` each compare read and purchase lists independently.
- A permission moved from read to purchase (or vice versa) with no net change to the union fails CI with a clear drift message.
- The overall pass/fail exit code and CI job (\`archera-parity\`) are preserved.

## References

- PR: `#315`
- Requested by: `@cristim`
" 

Length of output: 48


@cristim, done! Follow-up issue created: #327

It captures the full per-policy-block parity scope:

  • CFN: split by PolicyDocument.Statement (read vs purchase policy resource)
  • ARM: two role-definition resources diffed independently
  • Bicep: var readActions / var purchaseActions extracted and compared separately

Labels applied: type/chore, severity/low, priority/p3, urgency/eventually, impact/internal, effort/m, triaged. PR #315 is referenced in the body.

Comment on lines +187 to +191
local inline_actions
case "$cloud" in
aws) inline_actions=$(grep -oE '"[a-z]+:[A-Za-z]+"' "$env_file" || true) ;;
azure) inline_actions=$(grep -oE '"Microsoft\.[^"]*"' "$env_file" || true) ;;
gcp) inline_actions=$(grep -oE '"[a-z]+\.[a-z]+\.[a-zA-Z]+"' "$env_file" || true) ;;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

The AWS thin-caller regex misses ec2:* actions.

Line 189 only matches letter-only service names, so inline permissions like "ec2:DescribeReservedInstances" are invisible to this check. That lets the thin-caller guard pass even after common AWS Archera actions are pasted back into the environment file.

🔧 Minimal fix
-    aws)   inline_actions=$(grep -oE '"[a-z]+:[A-Za-z]+"' "$env_file" || true) ;;
+    aws)   inline_actions=$(grep -oE '"[a-z][a-z0-9]*:[A-Za-z][A-Za-z0-9*]*"' "$env_file" || true) ;;
📝 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
local inline_actions
case "$cloud" in
aws) inline_actions=$(grep -oE '"[a-z]+:[A-Za-z]+"' "$env_file" || true) ;;
azure) inline_actions=$(grep -oE '"Microsoft\.[^"]*"' "$env_file" || true) ;;
gcp) inline_actions=$(grep -oE '"[a-z]+\.[a-z]+\.[a-zA-Z]+"' "$env_file" || true) ;;
local inline_actions
case "$cloud" in
aws) inline_actions=$(grep -oE '"[a-z][a-z0-9]*:[A-Za-z][A-Za-z0-9*]*"' "$env_file" || true) ;;
azure) inline_actions=$(grep -oE '"Microsoft\.[^"]*"' "$env_file" || true) ;;
gcp) inline_actions=$(grep -oE '"[a-z]+\.[a-z]+\.[a-zA-Z]+"' "$env_file" || true) ;;
🤖 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 `@scripts/check-archera-parity.sh` around lines 187 - 191, The AWS branch's
inline_actions grep only allows letters for the service name and thus misses
services like "ec2:DescribeReservedInstances"; update the aws case grep to
accept digits and hyphens in the service portion and allow wildcard '*' in the
action portion (e.g., change the regex used in the aws branch that sets
inline_actions to something like '"[a-z0-9-]+:[A-Za-z*]+"' so
functions/variables inline_actions and the aws case match ec2 and similar
actions).

Address 5 of 6 actionable items from CR's 11:33 review:

1. frontend/src/federation.ts — move `aria-describedby` from the
   `<label>` to the focusable `<input>` so screen-reader users get the
   Archera tooltip announced when tabbing to the checkbox.
2. iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml +
   iac/federation/aws-target/cloudformation/archera.cfn.yaml — reject
   whitespace-only `ArcheraExternalId` via `AllowedPattern: '^$|.*\S.*'`
   so a value like `"   "` no longer produces a trust policy Archera
   cannot match.
3. iac/modules/archera/{aws,azure,gcp}/main.tf — bump
   `required_version` from `>= 1.5` to `>= 1.9.0`. Azure and GCP
   modules use cross-variable validation (var.enable_archera referenced
   inside another variable's validation block), which Terraform only
   added in 1.9. AWS bumped for consistency.
4. scripts/generate-federation-iac.go — add `SourceAccountID`,
   `OIDCIssuerHost`, `CUDlyAPIURL`, `ContactEmail` to `iacData` so
   AWS templates that reference these don't error at render time.
   Set `OIDCIssuerHost` from the resolved OIDC issuer URL when target
   = aws.
5. scripts/check-archera-parity.sh — broaden the AWS thin-caller regex
   from `[a-z]+:[A-Za-z]+` to `[a-z][a-z0-9-]*:[A-Za-z]+` so digit- or
   dash-containing service prefixes like `ec2:` and `s3:` are caught
   when re-introduced into environment files.

Deferred to a justified-on-thread reply (heavy lift, deferred to
follow-up):

6. scripts/check-archera-parity.sh — split read-vs-purchase parity
   comparison so a permission migrating from one block to the other
   still fails CI. Current flat-set check still catches any net drift;
   the read/purchase split is an enhancement worth a dedicated PR.

Also a separate justified deferral on the GCP scope-docs heavy-lift
finding — scope is provisional pending @cristim per the issue body, so
no scope edits in this PR.

Local verification:
- bash scripts/check-archera-parity.sh — all 7 checks pass.
- terraform init -backend=false && terraform validate on all three
  shared modules — all pass.
- npm test -- federation.test.ts — all 6 tests pass.
@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 6, 2026

Addressed 5 of 6 actionable items from the 11:33 review in 1ed91e6:

# File Fix
1 frontend/src/federation.ts aria-describedby moved from <label> to <input> so screen readers get the tooltip on tab focus
2 iac/federation/aws-cross-account/cloudformation/archera.cfn.yaml + iac/federation/aws-target/cloudformation/archera.cfn.yaml AllowedPattern: '^$|.*\\S.*' rejects whitespace-only ArcheraExternalId
3 iac/modules/archera/{aws,azure,gcp}/main.tf required_version bumped to >= 1.9.0 (cross-variable validation requires 1.9+)
4 scripts/generate-federation-iac.go iacData adds SourceAccountID, OIDCIssuerHost, CUDlyAPIURL, ContactEmail (mirrors handler) + OIDCIssuerHost populated for AWS targets
5 scripts/check-archera-parity.sh AWS thin-caller regex broadened to [a-z][a-z0-9-]*: so digit-containing services like ec2: and s3: are caught

Two deferred items, with justifications posted on-thread:

  • Read-vs-purchase parity split — heavy lift; flat-union check still catches any net drift; tracking as a parity-enhancement follow-up.
  • GCP scope-docs research — scope is provisional pending @cristim per the issue body; no content edits this PR.

Local verification: bash scripts/check-archera-parity.sh (all 7 checks pass), terraform validate on all three modules (clean), npm test -- federation.test.ts (6/6 pass).

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Tip

For best results, initiate chat on the files or code changes.

@cristim Thanks for the detailed update — the fixes look well-targeted:

  • The aria-describedby placement on <input> is the correct ARIA pattern for associating a description with a form control.
  • The AllowedPattern addition to both CFN templates closes the whitespace-only gap consistently with what was done on the Terraform side.
  • Bumping required_version to >= 1.9.0 is the right call — cross-variable validation in lifecycle preconditions does require 1.9+.
  • Mirroring all handler fields in iacData for generate-federation-iac.go is a good parity improvement that prevents template rendering divergence between the API and the local generator.
  • The broadened regex [a-z][a-z0-9-]*: is a more correct characterization of AWS service prefixes.

The two deferrals are reasonable — a flat-union parity check still catches net permission drift, and GCP scope being provisional is already documented in the issue body. Kicking off a fresh review now.

[review]

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 6, 2026

CR loop closed

CR's most recent review on commit 1ed91e6 has zero actionable items (empty review body) and accepted the deferral on the parity-split (quoting CR: "that's a reasonable trade-off — the flat-union gate already catches any net change in the total permission set").

Final state:

CR is filing a follow-up issue for the per-policy-block parity-script enhancement. The scope-docs research deferral stays as-is (provisional pending @cristim's confirmation per the original #305 / #314 caveat).

Ready for human review/merge. PR also Supersedes #310 — closing #310 once this lands.

Copy link
Copy Markdown

@Gero-B Gero-B left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. After an internal review on the Archera side, we want to redirect the approach before you invest more in this direction.

The core issue: the IaC approach has three problems we can't work around.
First, the sequencing problem. The external ID in the trust policy is derived from the customer's Archera org ID, which only exists after they've signed up. That means this bundle is only useful to someone who is already an Archera customer — it can't serve the new-user conversion path the ticket is actually targeting.

Second, Azure uses a different integration model entirely. Archera's Azure integration is an OAuth enterprise app consent flow, not a custom RBAC role. A customer who applies the Azure Terraform in this PR would still need to go through Archera's consent flow anyway, and Archera's systems wouldn't consume the RBAC role. The Azure piece as written won't work end-to-end.

Third, Archera already ships pre-filled Terraform for AWS and GCP through its own onboarding flow — with real account IDs and the customer's external ID already rendered in. Duplicating that in CUDly creates a maintenance surface we'd have to keep in sync across permission changes in an external repo, with no clear owner when they drift.

What we'd like instead.
Keep the checkbox in the UI — that's the right idea. But rather than bundling IAM resources, when a user checks it, surface a link to a dedicated Archera signup page (/signup?mode=cudly) with CUDly-specific copy and referral tracking. That page handles the full flow across all three clouds and solves the sequencing problem naturally: the user creates their Archera account first, gets their org ID, and Archera's own onboarding generates the correctly pre-filled Terraform/CFN from there.

This reduces the PR to roughly the checkbox + a link — a much smaller change with zero maintenance burden on either side.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 11, 2026

@Gero-B thank you for the feedback, that's great news because it would greatly simplify this integration.

I'll create another PR for that simplified approach and now closing this one.

@cristim cristim closed this May 11, 2026
@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 11, 2026

Closing — direction change after Archera-side review

After internal review on the Archera side, the IaC-bundling approach in this PR doesn't fit the actual integration model:

  1. Sequencing: the external ID in the trust policy is derived from the customer's Archera org ID, which only exists after they've signed up. The bundle is only useful to someone already an Archera customer — it can't serve the new-user conversion path feat(frontend): Archera Insurance CTA in Purchase + Plan-creation modals + education pages #314 actually targeted.
  2. Azure model mismatch: Archera's Azure integration is an OAuth enterprise-app consent flow, not a custom RBAC role. The Azure piece in this PR wouldn't work end-to-end — Archera's systems don't consume the RBAC role.
  3. Maintenance duplication: Archera already ships pre-filled Terraform for AWS and GCP through their own onboarding (with real account IDs and the customer's external ID rendered in). Duplicating here creates an out-of-band drift surface with no clear owner.

The replacement approach is much smaller: keep the Settings checkbox, but when checked surface a link to a dedicated Archera signup page (/signup?mode=cudly) with CUDly-specific copy and referral tracking. Archera's signup flow handles the IaC generation for all three clouds with correct sequencing.

Tracking the new approach in a fresh PR — link follows shortly. This PR is closed without merging.

cristim added a commit that referenced this pull request May 11, 2026
…cation pages (closes #314)

Introduces the Archera Insurance CTA at the two high-intent moments
where a user is about to commit money, plus two education pages that
explain what Archera Insurance is and how the integration works.

**New module — frontend/src/archera.ts**
- `renderArcheraCTA()`: returns a small, muted <p> with an inline
  <button> that reads "Worried about committing? Archera Insurance
  underwrites commitment-overuse — learn how it works →". Placed
  below the approval note in the Purchase modal, and before the
  Save/Cancel row in Plan-creation modals.
- `openArcheraPage('what-is-archera' | 'how-it-works')`: renders a
  full-viewport overlay panel using DOM methods only (no innerHTML).
  Doesn't touch SPA routing state or the URL.
- `closeArcheraPage()`: hides + clears the overlay.
- `ARCHERA_SIGNUP_URL = 'https://archera.ai/signup?mode=cudly'` for
  attribution-tagged referral links.

**Page A — "What is Archera Insurance?"**
Plain-language explainer: commitment-overuse insurance, how Archera
earns (sharing-of-savings), when it makes sense, what CUDly's role is.
Cross-links to Page B. Signup link at bottom (target=_blank,
rel="noopener noreferrer").

**Page B — "How the CUDly ↔ Archera integration works"**
Step-by-step: sign up → Archera generates pre-filled IaC for AWS/GCP
(or OAuth consent for Azure) → Archera ingests cost data → purchase
commitments normally through CUDly. Disclaimers: Archera is
third-party, no credentials stored in CUDly. Cross-links to Page A.
Signup link at bottom.

TODO(@cristim) markers flag all copy that needs final review.

**Modal integration**
- `recommendations.ts / openPurchaseModal`: appends `renderArcheraCTA()`
  inside `#purchase-details`, below the approval note.
- `plans.ts / openCreatePlanModal + openNewPlanModal`: calls
  `injectPlanModalCTA()` (guarded against duplication with an id
  check) to insert the CTA before `.modal-buttons` in `#plan-form`.

**CSS (styles/modals.css)**: `.archera-cta`, `.archera-cta-link`,
`#archera-page-container`, `.archera-page-inner`, `.archera-page-back`,
`.archera-signup-block`, `.archera-signup-btn`, `.archera-signup-note`.

**HTML (index.html)**: adds `<div id="archera-page-container"
role="region" aria-label="Archera Insurance information">` (hidden
by default) before the modal stack.

**Tests (archera.test.ts)**: 26 assertions covering renderArcheraCTA
structure, page A/B content + crosslinks + signup link attributes,
closeArcheraPage, and CTA injection in both Purchase and Plan modals.

Nothing in backend/, cmd/, internal/, iac/, terraform/, or scripts/ is
touched. This is a frontend-only change. Supersedes PR #310 and PR #315
(closed without merging; earlier iterations bundled IAM/IaC which was
rejected by Archera-side review — see issue #314 for history).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/l Weeks impact/many Affects most users priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/feat New capability urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants