diff --git a/.github/RELEASE_NOTES_TEMPLATE.md b/.github/RELEASE_NOTES_TEMPLATE.md index bc9f24b..a8c2189 100644 --- a/.github/RELEASE_NOTES_TEMPLATE.md +++ b/.github/RELEASE_NOTES_TEMPLATE.md @@ -2,13 +2,13 @@ ### Highlights -- Generated payload, manual, and audit files are now atomically replaced with owner-only `0600` permissions, including outputs that may contain static AWS credentials. -- Forward API reads, NQE queries, and full-state PATCH operations retry bounded transient `429`, `502`, `503`, and `504` responses. Non-idempotent create POSTs remain single-attempt. -- `awssync --version` now reports the release, source commit, and build date. -- CI now runs formatting, vet, tests, the race detector, and `govulncheck` with read-only repository permissions and commit-pinned actions. -- Release jobs use least-privilege permissions and continue to publish checksums and build-provenance attestations. -- The README now starts with the workflow decision diagram and routes detailed operator procedures to focused runbooks. -- Contribution guidance requires human attribution and excludes automation/tool identities from contributor metadata. +- `awssync external-id` can now target one or more `--account-id` values while preserving every unselected account. +- A reviewed `--external-id-file` CSV supports different values and explicit set/clear actions per setup and account. +- Normal NQE, webhook, and authoritative-manifest syncs preserve mixed per-account External IDs instead of flattening them to the first configured value. +- New accounts in a mixed-ID setup fail closed unless the plan receives an explicit per-account CSV assignment. +- CSV validation rejects malformed IDs, duplicates, implicit clears, wrong setups, and accounts outside the planned inventory before PATCH. +- Dry-run summaries report selected, changed, unchanged, set, and cleared account counts plus per-account change metadata. +- Release assets remain available for Linux and macOS on amd64 and arm64 with SHA-256 checksums and GitHub build-provenance attestations. ### Download and verify diff --git a/README.md b/README.md index 04a14d4..2ec1dd7 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ flowchart TD A[What are you doing?] -->|Update an existing setup| B{Complete Organizations inventory
is visible in Forward NQE?} A -->|Create a new setup| C{Can the customer use
AWS Organizations?} - A -->|Change External ID only| X[Run external-id dry plan
then apply once] + A -->|Change External ID only| X[Choose all accounts, selected IDs,
or a reviewed CSV; dry-run first] B -->|Yes| D[Use preflight and the default NQE sync] B -->|No or standalone GovCloud accounts| E[Use a reviewed authoritative manifest] @@ -33,6 +33,7 @@ The key choice is the inventory source. Use Forward NQE only when a current snap - Additions can be automated. Removals are blocked unless `--allow-removals` is explicit. - `--max-removals` and `--max-removal-percent` impose independent blast-radius ceilings. - Empty candidate inventory, stale snapshots, missing Organizations evidence, and unsafe GovCloud removal plans fail closed. +- Existing per-account External IDs are preserved. Adding accounts to a mixed-ID setup fails unless the new accounts have explicit CSV assignments. - Saved plans are revalidated against current Forward state before apply. - Generated payload and audit files are written atomically with owner-only `0600` permissions. - Transient API failures are retried only for idempotent reads and full-state updates; create operations are never automatically retried. @@ -118,7 +119,7 @@ Never remove an account only because collection fails. If it remains visible in ## Customer-Defined External ID -Changing an External ID is a separate, one-time workflow and works with an existing IAM-user/access-key setup. First review the Forward payload without changing anything: +Changing an External ID is a separate workflow and works with an existing IAM-user/access-key setup. With no `--account-id`, the command retains its setup-wide behavior. First review the Forward payload without changing anything: ```bash ./bin/awssync external-id \ @@ -140,6 +141,40 @@ Update the target-role trust policies to require the identical `sts:ExternalId`, Later syncs preserve the value. To roll back, first relax the AWS trust policies, verify role assumption, then run the same command with `--clear` instead of `--value`. +For a representative-account test or different values per account, scope the command with one or more account IDs: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --account-id 111111111111 \ + --value test-external-id \ + --output aws_external_id_test.json \ + --format human +``` + +For a reviewed batch, use CSV. `set` requires a non-empty value; `clear` requires an empty value. A blank cell by itself never means clear. + +```csv +setup_id,account_id,action,external_id +AWS-PROD,111111111111,set,account-one-value +AWS-PROD,222222222222,set,account-two-value +AWS-PROD,333333333333,clear, +``` + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --external-id-file external-ids.csv \ + --output aws_external_id_payload.json \ + --format human +``` + +Omitted accounts remain unchanged. Duplicate, malformed, wrong-setup, and unknown account rows stop before any PATCH. The generated payload still contains the complete current account list because Forward updates this field as full state. + +Scoped rollback uses the same `--account-id` selection. Dry-run `--clear` when the account previously had no External ID, or `--value PREVIOUS_VALUE` when restoring a prior non-null value; then repeat the reviewed command with `--apply --yes`. Record any prior non-null value before testing because the command reports whether a previous value was configured but does not retain that value as an automatic rollback artifact. Relax the matching AWS trust-policy condition before clearing or replacing the Forward value. + +Ordinary NQE, webhook, and `sync-accounts` runs preserve each existing account's value. If a mixed-ID setup discovers a new account, preflight and dry-run fail closed until that account is assigned in the same CSV passed with `--external-id-file`. + ## Onboarding and GovCloud | Environment | Inventory source | Recommended command or workflow | diff --git a/cmd/awssync/main.go b/cmd/awssync/main.go index 8547795..8ddfc05 100644 --- a/cmd/awssync/main.go +++ b/cmd/awssync/main.go @@ -92,6 +92,7 @@ func newRootCommand() *cobra.Command { AllowNoCandidates: flagBool(cmd, v, "allow-no-candidates"), AllowNoOrgEvidence: flagBool(cmd, v, "allow-no-org-evidence"), MaxSnapshotAge: flagDuration(cmd, v, "max-snapshot-age"), + ExternalIDFile: flagString(cmd, v, "external-id-file"), } previewSummary, err := app.Run(cmd.Context(), preview) if err != nil { @@ -126,6 +127,7 @@ func newRootCommand() *cobra.Command { AllowNoCandidates: flagBool(cmd, v, "allow-no-candidates"), AllowNoOrgEvidence: flagBool(cmd, v, "allow-no-org-evidence"), MaxSnapshotAge: flagDuration(cmd, v, "max-snapshot-age"), + ExternalIDFile: flagString(cmd, v, "external-id-file"), } summary, err := app.Run(cmd.Context(), cfg) if err != nil { @@ -171,6 +173,8 @@ func newExternalIDCommand(v *viper.Viper) *cobra.Command { setupID, _ := cmd.Flags().GetString("setup-id") value, _ := cmd.Flags().GetString("value") clearValue, _ := cmd.Flags().GetBool("clear") + accountIDs, _ := cmd.Flags().GetStringSlice("account-id") + externalIDFile, _ := cmd.Flags().GetString("external-id-file") apply, _ := cmd.Flags().GetBool("apply") yes, _ := cmd.Flags().GetBool("yes") if err := confirmApply(apply, yes, os.Stdin, os.Stderr); err != nil { @@ -178,18 +182,20 @@ func newExternalIDCommand(v *viper.Viper) *cobra.Command { } output, _ := cmd.Flags().GetString("output") summary, err := app.ChangeExternalID(cmd.Context(), app.ExternalIDConfig{ - Host: v.GetString("host"), - Username: v.GetString("username"), - Password: password, - NetworkID: networkID, - SetupID: setupID, - ExternalID: value, - Clear: clearValue, - Output: output, - APIPrefix: v.GetString("api-prefix"), - Insecure: v.GetBool("insecure"), - Timeout: v.GetDuration("timeout"), - Apply: apply, + Host: v.GetString("host"), + Username: v.GetString("username"), + Password: password, + NetworkID: networkID, + SetupID: setupID, + AccountIDs: accountIDs, + ExternalID: value, + Clear: clearValue, + ExternalIDFile: externalIDFile, + Output: output, + APIPrefix: v.GetString("api-prefix"), + Insecure: v.GetBool("insecure"), + Timeout: v.GetDuration("timeout"), + Apply: apply, }) if err != nil { return err @@ -201,6 +207,8 @@ func newExternalIDCommand(v *viper.Viper) *cobra.Command { cmd.Flags().String("setup-id", "", "existing Forward AWS setup ID") cmd.Flags().String("value", "", "customer-defined External ID to set") cmd.Flags().Bool("clear", false, "clear the External ID back to null") + cmd.Flags().StringSlice("account-id", nil, "optional AWS account ID to change; repeatable or comma-separated") + cmd.Flags().String("external-id-file", "", "CSV file of per-account External ID set/clear actions") cmd.Flags().String("output", "", "output JSON path for the generated PATCH payload") cmd.Flags().Bool("apply", false, "PATCH the generated setup payload into Forward") cmd.Flags().Bool("yes", false, "skip apply confirmation prompt") @@ -244,6 +252,7 @@ func bindPreflightFlags(v *viper.Viper, flags *pflag.FlagSet) { flags.Int("max-removals", 0, "maximum aggregate account removals allowed during apply; 0 disables the limit") flags.Float64("max-removal-percent", 0, "maximum removal percentage allowed per setup during apply; 0 disables the limit") flags.Duration("max-snapshot-age", 0, "fail if latest processed snapshot is older than this duration; 0 disables the check") + flags.String("external-id-file", "", "CSV file of explicit per-account External IDs for mixed-ID setups") mustBind(v, flags, "snapshot-id") mustBind(v, flags, "query-id") mustBind(v, flags, "query-setup-param") @@ -252,6 +261,7 @@ func bindPreflightFlags(v *viper.Viper, flags *pflag.FlagSet) { mustBind(v, flags, "max-removals") mustBind(v, flags, "max-removal-percent") mustBind(v, flags, "max-snapshot-age") + mustBind(v, flags, "external-id-file") } func bindProcessingFlags(v *viper.Viper, flags *pflag.FlagSet) { @@ -268,6 +278,7 @@ func bindProcessingFlags(v *viper.Viper, flags *pflag.FlagSet) { flags.Bool("allow-no-candidates", false, "allow removals when no uncollected candidate accounts are visible") flags.Bool("allow-no-org-evidence", false, "allow removals when no AWS Organizations evidence is visible in NQE") flags.Duration("max-snapshot-age", 0, "fail if latest processed snapshot is older than this duration; 0 disables the check") + flags.String("external-id-file", "", "CSV file of explicit per-account External IDs for mixed-ID setups") mustBind(v, flags, "query-id") mustBind(v, flags, "query-setup-param") mustBind(v, flags, "setup-id") @@ -281,6 +292,7 @@ func bindProcessingFlags(v *viper.Viper, flags *pflag.FlagSet) { mustBind(v, flags, "allow-no-candidates") mustBind(v, flags, "allow-no-org-evidence") mustBind(v, flags, "max-snapshot-age") + mustBind(v, flags, "external-id-file") } func newPreflightCommand(v *viper.Viper) *cobra.Command { @@ -318,6 +330,7 @@ func newPreflightCommand(v *viper.Viper) *cobra.Command { MaxRemovals: flagInt(cmd, v, "max-removals"), MaxRemovalPercent: flagFloat64(cmd, v, "max-removal-percent"), MaxSnapshotAge: flagDuration(cmd, v, "max-snapshot-age"), + ExternalIDFile: flagString(cmd, v, "external-id-file"), }) if err != nil { return err @@ -697,6 +710,7 @@ func newSyncAccountsCommand(v *viper.Viper) *cobra.Command { AllowRemovals: flagBool(cmd, v, "allow-removals"), MaxRemovals: flagInt(cmd, v, "max-removals"), MaxRemovalPercent: flagFloat64(cmd, v, "max-removal-percent"), + ExternalIDFile: flagString(cmd, v, "external-id-file"), }, accounts) if err != nil { return err @@ -714,7 +728,8 @@ func newSyncAccountsCommand(v *viper.Viper) *cobra.Command { cmd.Flags().Bool("allow-removals", false, "allow reviewed manifest entries to remove accounts from the setup") cmd.Flags().Int("max-removals", 0, "maximum aggregate account removals allowed; 0 disables the limit") cmd.Flags().Float64("max-removal-percent", 0, "maximum removal percentage allowed for the setup; 0 disables the limit") - for _, name := range []string{"accounts-file", "setup-id", "output", "manual-output", "apply", "yes", "allow-removals", "max-removals", "max-removal-percent"} { + cmd.Flags().String("external-id-file", "", "CSV file of explicit per-account External IDs") + for _, name := range []string{"accounts-file", "setup-id", "output", "manual-output", "apply", "yes", "allow-removals", "max-removals", "max-removal-percent", "external-id-file"} { mustBind(v, cmd.Flags(), name) } return cmd @@ -758,6 +773,7 @@ func newServeWebhookCommand(v *viper.Viper) *cobra.Command { AllowNoCandidates: flagBool(cmd, v, "allow-no-candidates"), AllowNoOrgEvidence: flagBool(cmd, v, "allow-no-org-evidence"), MaxSnapshotAge: flagDuration(cmd, v, "max-snapshot-age"), + ExternalIDFile: flagString(cmd, v, "external-id-file"), }, }) if err != nil { @@ -1182,17 +1198,15 @@ func emitResult(cmd *cobra.Command, v *viper.Viper, value any) error { } func emitExternalIDHuman(summary *app.ExternalIDSummary) error { - action := "set" - if !summary.TargetExternalIDConfigured { - action = "clear" - } fmt.Fprintln(os.Stdout, "External ID migration report") fmt.Fprintf(os.Stdout, " host: %s\n", summary.Host) fmt.Fprintf(os.Stdout, " network: %s\n", summary.NetworkID) fmt.Fprintf(os.Stdout, " setup: %s\n", summary.SetupID) - fmt.Fprintf(os.Stdout, " action: %s\n", action) - fmt.Fprintf(os.Stdout, " accounts: %d\n", summary.AccountCount) + fmt.Fprintf(os.Stdout, " mode: %s\n", summary.Mode) + fmt.Fprintf(os.Stdout, " accounts: %d total, %d selected, %d changed, %d unchanged\n", summary.AccountCount, summary.SelectedAccountCount, summary.ChangedAccountCount, summary.UnchangedAccountCount) + fmt.Fprintf(os.Stdout, " actions: %d set, %d clear\n", summary.SetAccountCount, summary.ClearedAccountCount) fmt.Fprintf(os.Stdout, " prior ID: configured=%t consistent=%t\n", summary.PreviousExternalIDConfigured, summary.PreviousExternalIDConsistent) + fmt.Fprintf(os.Stdout, " target ID: configured=%t consistent=%t\n", summary.TargetExternalIDConfigured, summary.TargetExternalIDConsistent) fmt.Fprintf(os.Stdout, " apply: %t\n", summary.Apply) fmt.Fprintf(os.Stdout, " patched: %t\n", summary.Patched) fmt.Fprintf(os.Stdout, " output: %s\n", summary.Output) diff --git a/docs/architecture-flow.md b/docs/architecture-flow.md index 71f9969..efb2d3d 100644 --- a/docs/architecture-flow.md +++ b/docs/architecture-flow.md @@ -65,6 +65,7 @@ flowchart TB subgraph plan_apply["awssync — plan and apply"] plan["plan / dry-run\nPOST /nqe + GET /cloudAccounts"] + external_ids["Per-account External ID merge\npreserve existing values\nexplicit CSV for ambiguous additions"] disk["payload.json\nwritten to disk before any change"] safety["Removal gates\nexplicit approval + count/% ceilings"] apply["--apply\nPATCH /cloudAccounts/{setupId}"] @@ -84,7 +85,7 @@ flowchart TB cron --> plan preflight -- "read-only" --> fwd - plan --> disk + plan --> external_ids --> disk disk --> safety --> apply disk --> apply_plan apply_plan --> safety @@ -99,11 +100,13 @@ flowchart TB classDef artifact fill:#FAEEDA,stroke:#854F0B,color:#412402; class cli,cron,preflight neutral; - class plan,safety,apply,apply_plan neutral; + class plan,external_ids,safety,apply,apply_plan neutral; class nqe,get_accts,patch_accts,get_snap fwdnode; class disk artifact; ``` +The account list is full-state, but External IDs are merged by AWS account ID. Existing mixed values are preserved. When a mixed-ID setup gains an account, planning stops unless `--external-id-file` explicitly supplies the new account's value; omitted existing accounts remain unchanged. + --- ## Mode 2 — Initial AWS Organizations Onboarding diff --git a/docs/aws-account-sync-procedure.md b/docs/aws-account-sync-procedure.md index a17b5a2..a706777 100644 --- a/docs/aws-account-sync-procedure.md +++ b/docs/aws-account-sync-procedure.md @@ -363,9 +363,9 @@ If `--manual-output` is used, also confirm that manual payload file by opening i This is a separate, one-time hardening change, not a prerequisite for AWS Organizations discovery. It is supported for an existing IAM user/access-key setup: Forward keeps using the stored IAM user credentials, but includes the configured External ID when it calls `sts:AssumeRole` for each target account. -Choose one customer-defined value per Forward AWS setup. Use the same value in Forward and in every target role trust policy for that setup. External IDs are not passwords, but use an unguessable, customer-specific value and do not reuse it across unrelated customers. +The simplest policy uses one customer-defined value per Forward AWS setup. Per-account values are also supported for staged testing or customer policy requirements. In every case, the value stored on an account's Forward `assumeRoleInfos` entry must exactly match that account's target-role trust policy. External IDs are not passwords, but use unguessable, customer-specific values and do not reuse them across unrelated customers. -Use the dedicated `external-id` command rather than the normal NQE synchronization path. It reads the existing Forward setup directly, preserves its account list, role ARNs, regions, and proxy settings, and changes only the External ID on each `assumeRoleInfos` entry. It does not depend on NQE account discovery or a new snapshot. +Use the dedicated `external-id` command rather than the normal NQE synchronization path for an isolated migration. It reads the existing Forward setup directly, preserves its account list, role ARNs, regions, and proxy settings, and changes only the selected External IDs. It does not depend on NQE account discovery or a new snapshot. First run a dry plan: @@ -377,7 +377,88 @@ First run a dry plan: --format human ``` -Review the prior-state fields and confirm every entry in `aws_external_id_payload.json` contains the intended `externalId`. The command requires exactly one setup and either `--value VALUE` or `--clear`; without `--apply`, it writes the payload but does not modify Forward. It does not change or expose the setup's stored IAM access key or secret. +Review the prior-state fields and confirm every entry in `aws_external_id_payload.json` contains the intended `externalId`. The command requires exactly one setup and either `--value VALUE`, `--clear`, or `--external-id-file FILE`; without `--apply`, it writes the payload but does not modify Forward. With no `--account-id`, `--value` and `--clear` apply to every account for backward compatibility. It does not change or expose the setup's stored IAM access key or secret. + +### Test one account or assign different values + +Repeat `--account-id` to apply one value or clear operation to a selected subset: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --account-id 111111111111 \ + --value representative-test-value \ + --output aws_external_id_test_payload.json \ + --format human +``` + +For different values or mixed set/clear actions, create a reviewed CSV: + +```csv +setup_id,account_id,action,external_id +AWS-PROD,111111111111,set,representative-test-value +AWS-PROD,222222222222,set,account-two-value +AWS-PROD,333333333333,clear, +``` + +The shorter `account_id,action,external_id` header is accepted when the command or sync selects exactly one setup. An explicit `clear` action is mandatory; an empty cell never clears by implication. + +Dry-run and then apply the same file: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --external-id-file external-ids.csv \ + --output aws_external_id_payload.json \ + --format human + +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --external-id-file external-ids.csv \ + --output aws_external_id_payload.json \ + --apply --yes +``` + +Omitted accounts are preserved. The command rejects duplicate, malformed, wrong-setup, and unknown account rows before writing or applying a PATCH. Review `selected_account_count`, `changed_account_count`, set/clear counts, and the per-account change list; values are visible only in the generated full-state payload. + +For a representative-account test, record that account's prior value during change review. Rollback is a second scoped dry-run and apply using the same account ID: + +```bash +# Restore the original null/no-External-ID state. +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --account-id 111111111111 \ + --clear \ + --output aws_external_id_test_revert.json \ + --format human + +# Or restore a prior non-null value. +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --account-id 111111111111 \ + --value PREVIOUS_VALUE \ + --output aws_external_id_test_revert.json \ + --format human +``` + +Review the payload, then repeat only the applicable command with `--apply --yes`. Unselected accounts retain their current fields and order in the generated `assumeRoleInfos` list. The summary intentionally records configured/not-configured state rather than retaining the prior value as an automatic rollback artifact. Before clearing or replacing Forward's value, relax or restore the selected account's AWS trust-policy condition and verify role assumption so collection is not interrupted. + +Normal NQE sync, webhook sync, and `sync-accounts` now preserve each existing account's External ID instead of copying the first value across the setup. If a setup already has mixed values and discovery adds an account, the plan fails closed because no safe value can be inferred. Supply an assignment for every new account to preflight and the eventual dry-run/apply: + +```bash +./bin/awssync preflight \ + --setup-id AWS-PROD \ + --external-id-file external-ids.csv \ + --max-snapshot-age 24h \ + --format human + +./bin/awssync \ + --setup-id AWS-PROD \ + --external-id-file external-ids.csv \ + --max-snapshot-age 24h \ + --output aws_sync_payload.json \ + --format human +``` Prepare the matching collection-role trust policy change for each target AWS account, but do not make the condition mandatory until the Forward payload has been applied and tested. For an IAM user in the connectivity account, the trust statement has this form: diff --git a/docs/quick-start.md b/docs/quick-start.md index e15c34c..9242887 100644 --- a/docs/quick-start.md +++ b/docs/quick-start.md @@ -98,6 +98,27 @@ Confirm the dry run reports the expected prior and target state and verify every Run the command once per Forward AWS setup if different setups require different values. It does not replace or expose the IAM user's stored access key or secret. After the migration PATCH, later syncs preserve the stored External ID without rerunning it. +To test one account, add `--account-id 111111111111`; repeat the flag for a subset. Unselected accounts remain unchanged. For different values per account, use a reviewed CSV: + +```csv +setup_id,account_id,action,external_id +AWS-PROD,111111111111,set,test-value +AWS-PROD,222222222222,set,account-specific-value +AWS-PROD,333333333333,clear, +``` + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --external-id-file external-ids.csv \ + --output aws_external_id_payload.json \ + --format human +``` + +Duplicate, malformed, or unknown accounts fail before PATCH. Normal sync preserves mixed per-account values. If a mixed-ID setup gains a new account, pass the same `--external-id-file` to `preflight` and the normal dry-run/apply so the new account has an explicit value. + +Scoped rollback uses the same `--account-id`: dry-run and apply `--clear` if the original value was null, or `--value PREVIOUS_VALUE` if it was non-null. Record the old non-null value before the test; the summary reports its configured state but does not save it as an automatic rollback value. Relax the selected account's AWS trust-policy condition before changing Forward back. All unselected accounts remain unchanged. + Rollback order matters: first relax or remove the mandatory `sts:ExternalId` condition from the target-role trust policies and confirm a representative role can still be assumed. Then replace `--value VALUE` with `--clear`, review the dry run, apply it, and test collection again. Clearing Forward first while AWS still requires the External ID will interrupt collection. ## Discover Before Onboarding diff --git a/internal/app/external_id.go b/internal/app/external_id.go index f3b27b6..4abcc7f 100644 --- a/internal/app/external_id.go +++ b/internal/app/external_id.go @@ -3,6 +3,7 @@ package app import ( "context" "fmt" + "sort" "strings" "time" @@ -10,18 +11,20 @@ import ( ) type ExternalIDConfig struct { - Host string - Username string - Password string - NetworkID string - SetupID string - ExternalID string - Clear bool - Output string - APIPrefix string - Insecure bool - Timeout time.Duration - Apply bool + Host string + Username string + Password string + NetworkID string + SetupID string + AccountIDs []string + ExternalID string + Clear bool + ExternalIDFile string + Output string + APIPrefix string + Insecure bool + Timeout time.Duration + Apply bool } type ExternalIDSummary struct { @@ -30,15 +33,32 @@ type ExternalIDSummary struct { SetupID string `json:"setup_id"` Apply bool `json:"apply"` Patched bool `json:"patched"` + Mode string `json:"mode"` AccountCount int `json:"account_count"` + SelectedAccountCount int `json:"selected_account_count"` + ChangedAccountCount int `json:"changed_account_count"` + SetAccountCount int `json:"set_account_count"` + ClearedAccountCount int `json:"cleared_account_count"` + UnchangedAccountCount int `json:"unchanged_account_count"` PreviousExternalIDConfigured bool `json:"previous_external_id_configured"` PreviousExternalIDConsistent bool `json:"previous_external_id_consistent"` TargetExternalIDConfigured bool `json:"target_external_id_configured"` + TargetExternalIDConsistent bool `json:"target_external_id_consistent"` + Changes []ExternalIDChange `json:"changes"` Output string `json:"output"` PayloadSHA256 string `json:"payload_sha256"` Payload ExternalIDPatchPayload `json:"payload"` } +type ExternalIDChange struct { + AccountID string `json:"account_id"` + AccountName string `json:"account_name,omitempty"` + Action string `json:"action"` + PreviousConfigured bool `json:"previous_configured"` + TargetConfigured bool `json:"target_configured"` + Changed bool `json:"changed"` +} + type ExternalIDPatchPayload struct { Type string `json:"type"` AssumeRoleInfos []api.AssumeRoleInfo `json:"assumeRoleInfos"` @@ -50,10 +70,27 @@ func ChangeExternalID(ctx context.Context, cfg ExternalIDConfig) (*ExternalIDSum if setupID == "" { return nil, fmt.Errorf("setup ID is required") } - if cfg.Clear == (externalID != "") { + externalIDFile := strings.TrimSpace(cfg.ExternalIDFile) + if externalIDFile != "" { + if externalID != "" || cfg.Clear || len(cfg.AccountIDs) > 0 { + return nil, fmt.Errorf("--external-id-file cannot be combined with --value, --clear, or --account-id") + } + } else if cfg.Clear == (externalID != "") { return nil, fmt.Errorf("specify exactly one of an external ID value or clear") } + assignments, err := loadExternalIDAssignments(externalIDFile, setupID) + if err != nil { + return nil, err + } + if externalIDFile != "" { + for assignmentSetupID := range assignments { + if assignmentSetupID != setupID { + return nil, fmt.Errorf("external ID file contains setup %s but command selected %s", assignmentSetupID, setupID) + } + } + } + client, err := api.NewClient( cfg.Host, cfg.APIPrefix, @@ -84,9 +121,88 @@ func ChangeExternalID(ctx context.Context, cfg ExternalIDConfig) (*ExternalIDSum previousConfigured, previousConsistent := externalIDState(account.AssumeRoleInfos) infos := make([]api.AssumeRoleInfo, len(account.AssumeRoleInfos)) copy(infos, account.AssumeRoleInfos) + seenCurrent := make(map[string]bool, len(infos)) + for _, info := range infos { + accountID := assumeRoleAccountID(info) + if accountID == "" { + return nil, fmt.Errorf("AWS setup %s contains an assumeRoleInfos entry without an account ID", setupID) + } + if seenCurrent[accountID] { + return nil, fmt.Errorf("AWS setup %s contains duplicate account %s", setupID, accountID) + } + seenCurrent[accountID] = true + } + mode := "all" + selected := make(map[string]string) + if externalIDFile != "" { + mode = "file" + for accountID, value := range assignments[setupID] { + selected[accountID] = value + } + } else if len(cfg.AccountIDs) > 0 { + mode = "selected" + for _, rawAccountID := range cfg.AccountIDs { + accountID := strings.TrimSpace(rawAccountID) + if !awsAccountIDPattern.MatchString(accountID) { + return nil, fmt.Errorf("invalid AWS account ID %q; expected 12 digits", rawAccountID) + } + if _, exists := selected[accountID]; exists { + return nil, fmt.Errorf("duplicate --account-id %s", accountID) + } + selected[accountID] = externalID + } + } else { + for _, info := range infos { + accountID := assumeRoleAccountID(info) + selected[accountID] = externalID + } + } + + found := make(map[string]bool, len(selected)) + changes := make([]ExternalIDChange, 0, len(selected)) + changedCount := 0 + setCount := 0 + clearedCount := 0 for i := range infos { - infos[i].ExternalID = externalID + accountID := assumeRoleAccountID(infos[i]) + target, ok := selected[accountID] + if !ok { + continue + } + found[accountID] = true + previous := strings.TrimSpace(infos[i].ExternalID) + changed := previous != target + if changed { + changedCount++ + } + action := "set" + if target == "" { + action = "clear" + clearedCount++ + } else { + setCount++ + } + changes = append(changes, ExternalIDChange{ + AccountID: accountID, + AccountName: infos[i].AccountName, + Action: action, + PreviousConfigured: previous != "", + TargetConfigured: target != "", + Changed: changed, + }) + infos[i].ExternalID = target } + missing := make([]string, 0) + for accountID := range selected { + if !found[accountID] { + missing = append(missing, accountID) + } + } + if len(missing) > 0 { + sort.Strings(missing) + return nil, fmt.Errorf("AWS setup %s does not contain selected account(s): %s", setupID, strings.Join(missing, ", ")) + } + targetConfigured, targetConsistent := externalIDState(infos) payload := ExternalIDPatchPayload{ Type: "AWS", AssumeRoleInfos: infos, @@ -106,15 +222,23 @@ func ChangeExternalID(ctx context.Context, cfg ExternalIDConfig) (*ExternalIDSum NetworkID: networkID, SetupID: setupID, Apply: cfg.Apply, + Mode: mode, AccountCount: len(infos), + SelectedAccountCount: len(selected), + ChangedAccountCount: changedCount, + SetAccountCount: setCount, + ClearedAccountCount: clearedCount, + UnchangedAccountCount: len(selected) - changedCount, PreviousExternalIDConfigured: previousConfigured, PreviousExternalIDConsistent: previousConsistent, - TargetExternalIDConfigured: externalID != "", + TargetExternalIDConfigured: targetConfigured, + TargetExternalIDConsistent: targetConsistent, + Changes: changes, Output: output, PayloadSHA256: sha, Payload: payload, } - if !cfg.Apply { + if !cfg.Apply || changedCount == 0 { return summary, nil } if _, _, err := writeJSONPayload(auditPath(output), payloads); err != nil { @@ -127,6 +251,13 @@ func ChangeExternalID(ctx context.Context, cfg ExternalIDConfig) (*ExternalIDSum return summary, nil } +func assumeRoleAccountID(info api.AssumeRoleInfo) string { + if accountID := strings.TrimSpace(info.AccountID); accountID != "" { + return accountID + } + return accountIDFromRoleArn(info.RoleArn) +} + func findAWSSetup(accounts []api.CloudAccount, setupID string) (api.CloudAccount, error) { for _, account := range accounts { if account.Name != setupID { diff --git a/internal/app/external_id_file.go b/internal/app/external_id_file.go new file mode 100644 index 0000000..9bb7918 --- /dev/null +++ b/internal/app/external_id_file.go @@ -0,0 +1,113 @@ +package app + +import ( + "encoding/csv" + "fmt" + "io" + "os" + "strings" +) + +type externalIDAssignments map[string]map[string]string + +func loadExternalIDAssignments(path, defaultSetupID string) (externalIDAssignments, error) { + path = strings.TrimSpace(path) + if path == "" { + return nil, nil + } + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("open external ID file: %w", err) + } + defer file.Close() + + reader := csv.NewReader(file) + reader.FieldsPerRecord = -1 + header, err := reader.Read() + if err != nil { + return nil, fmt.Errorf("read external ID file header: %w", err) + } + for i := range header { + header[i] = strings.ToLower(strings.TrimSpace(header[i])) + } + withSetup := equalStrings(header, []string{"setup_id", "account_id", "action", "external_id"}) + withoutSetup := equalStrings(header, []string{"account_id", "action", "external_id"}) + if !withSetup && !withoutSetup { + return nil, fmt.Errorf("external ID file header must be setup_id,account_id,action,external_id or account_id,action,external_id") + } + defaultSetupID = strings.TrimSpace(defaultSetupID) + if withoutSetup && defaultSetupID == "" { + return nil, fmt.Errorf("external ID file without setup_id requires exactly one selected setup") + } + + assignments := make(externalIDAssignments) + for row := 2; ; row++ { + record, err := reader.Read() + if err == io.EOF { + break + } + if err != nil { + return nil, fmt.Errorf("read external ID file row %d: %w", row, err) + } + expectedFields := 3 + if withSetup { + expectedFields = 4 + } + if len(record) != expectedFields { + return nil, fmt.Errorf("external ID file row %d has %d fields; expected %d", row, len(record), expectedFields) + } + for i := range record { + record[i] = strings.TrimSpace(record[i]) + } + setupID := defaultSetupID + offset := 0 + if withSetup { + setupID = record[0] + offset = 1 + } + accountID := record[offset] + action := strings.ToLower(record[offset+1]) + externalID := record[offset+2] + if setupID == "" { + return nil, fmt.Errorf("external ID file row %d has an empty setup_id", row) + } + if !awsAccountIDPattern.MatchString(accountID) { + return nil, fmt.Errorf("external ID file row %d has invalid AWS account ID %q; expected 12 digits", row, accountID) + } + switch action { + case "set": + if externalID == "" { + return nil, fmt.Errorf("external ID file row %d uses action set but external_id is empty", row) + } + case "clear": + if externalID != "" { + return nil, fmt.Errorf("external ID file row %d uses action clear but external_id is not empty", row) + } + default: + return nil, fmt.Errorf("external ID file row %d has invalid action %q; expected set or clear", row, action) + } + if assignments[setupID] == nil { + assignments[setupID] = make(map[string]string) + } + if _, exists := assignments[setupID][accountID]; exists { + return nil, fmt.Errorf("external ID file contains duplicate setup/account entry %s/%s", setupID, accountID) + } + assignments[setupID][accountID] = externalID + } + if len(assignments) == 0 { + return nil, fmt.Errorf("external ID file contains no assignments") + } + return assignments, nil +} + +func equalStrings(left, right []string) bool { + if len(left) != len(right) { + return false + } + for i := range left { + if left[i] != right[i] { + return false + } + } + return true +} diff --git a/internal/app/external_id_test.go b/internal/app/external_id_test.go index 13720d9..e900f75 100644 --- a/internal/app/external_id_test.go +++ b/internal/app/external_id_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "os" "path/filepath" "testing" @@ -121,3 +122,108 @@ func TestChangeExternalIDRequiresOneAction(t *testing.T) { } } } + +func TestChangeExternalIDScopesSelectedAccountsAndPreservesOthers(t *testing.T) { + stored := api.CloudAccount{ + Type: "AWS", + Name: "setup-a", + AssumeRoleInfos: []api.AssumeRoleInfo{ + {AccountID: "111111111111", AccountName: "one", RoleArn: "arn:aws:iam::111111111111:role/ForwardRole", ExternalID: "first", Enabled: true}, + {AccountID: "222222222222", AccountName: "two", RoleArn: "arn:aws:iam::222222222222:role/ForwardRole", ExternalID: "second", Enabled: true}, + {AccountID: "333333333333", AccountName: "three", RoleArn: "arn:aws:iam::333333333333:role/ForwardRole", Enabled: true}, + }, + } + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/api/networks/network-1/cloudAccounts" { + w.WriteHeader(http.StatusNotFound) + return + } + _ = json.NewEncoder(w).Encode([]api.CloudAccount{stored}) + })) + defer server.Close() + + summary, err := ChangeExternalID(context.Background(), ExternalIDConfig{ + Host: server.URL, + Username: "alice", + Password: "secret", + NetworkID: "network-1", + SetupID: "setup-a", + AccountIDs: []string{"222222222222"}, + ExternalID: "test-value", + Output: filepath.Join(t.TempDir(), "scoped.json"), + APIPrefix: "/api", + Insecure: true, + }) + if err != nil { + t.Fatalf("ChangeExternalID() error = %v", err) + } + if summary.Mode != "selected" || summary.SelectedAccountCount != 1 || summary.ChangedAccountCount != 1 { + t.Fatalf("unexpected scoped summary: %#v", summary) + } + infos := summary.Payload.AssumeRoleInfos + if infos[0].ExternalID != "first" || infos[1].ExternalID != "test-value" || infos[2].ExternalID != "" { + t.Fatalf("scoped change did not preserve unselected accounts: %#v", infos) + } + if summary.TargetExternalIDConsistent { + t.Fatalf("mixed target should be reported as inconsistent: %#v", summary) + } +} + +func TestChangeExternalIDUsesCSVSetAndClearActions(t *testing.T) { + stored := api.CloudAccount{ + Type: "AWS", + Name: "setup-a", + AssumeRoleInfos: []api.AssumeRoleInfo{ + {AccountID: "111111111111", ExternalID: "old", Enabled: true}, + {AccountID: "222222222222", ExternalID: "remove-me", Enabled: true}, + }, + } + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode([]api.CloudAccount{stored}) + })) + defer server.Close() + dir := t.TempDir() + csvPath := filepath.Join(dir, "external-ids.csv") + if err := os.WriteFile(csvPath, []byte("account_id,action,external_id\n111111111111,set,new-value\n222222222222,clear,\n"), 0o600); err != nil { + t.Fatal(err) + } + summary, err := ChangeExternalID(context.Background(), ExternalIDConfig{ + Host: server.URL, + Username: "alice", + Password: "secret", + NetworkID: "network-1", + SetupID: "setup-a", + ExternalIDFile: csvPath, + Output: filepath.Join(dir, "payload.json"), + APIPrefix: "/api", + Insecure: true, + }) + if err != nil { + t.Fatalf("ChangeExternalID() error = %v", err) + } + if summary.Mode != "file" || summary.SetAccountCount != 1 || summary.ClearedAccountCount != 1 || summary.ChangedAccountCount != 2 { + t.Fatalf("unexpected CSV summary: %#v", summary) + } + if got := summary.Payload.AssumeRoleInfos; got[0].ExternalID != "new-value" || got[1].ExternalID != "" { + t.Fatalf("unexpected CSV payload: %#v", got) + } +} + +func TestLoadExternalIDAssignmentsRejectsUnsafeRows(t *testing.T) { + for name, contents := range map[string]string{ + "blank set": "account_id,action,external_id\n111111111111,set,\n", + "value on clear": "account_id,action,external_id\n111111111111,clear,value\n", + "duplicate": "account_id,action,external_id\n111111111111,set,one\n111111111111,set,two\n", + "invalid account": "account_id,action,external_id\n111,set,value\n", + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "external-ids.csv") + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + if _, err := loadExternalIDAssignments(path, "setup-a"); err == nil { + t.Fatal("expected CSV validation error") + } + }) + } +} diff --git a/internal/app/preflight.go b/internal/app/preflight.go index bd64559..9c741ca 100644 --- a/internal/app/preflight.go +++ b/internal/app/preflight.go @@ -122,7 +122,7 @@ func Preflight(ctx context.Context, cfg Config) (*PreflightSummary, error) { result.pass("nqe_setup_id_differentiator", fmt.Sprintf("NQE rows include setup IDs: %s", strings.Join(setupIDValues, ", "))) } - plan, err := buildPlan(items, cloudAccounts, cfg.QueryID, cfg.SetupIDs) + plan, err := buildPlanForConfig(cfg, items, cloudAccounts) if err != nil { result.fail("patch_plan", err.Error()) return result, nil diff --git a/internal/app/run.go b/internal/app/run.go index 41b29cb..b7d2433 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -66,6 +66,7 @@ type Config struct { AllowNoCandidates bool AllowNoOrgEvidence bool MaxSnapshotAge time.Duration + ExternalIDFile string Source string AuthoritativeInput bool } @@ -123,6 +124,7 @@ type SetupSummary struct { RoleName string `json:"role_name"` OrgID int `json:"org_id,omitempty"` ExternalIDConfigured bool `json:"external_id_configured"` + ExternalIDConsistent bool `json:"external_id_consistent"` ProxyServerID string `json:"proxy_server_id,omitempty"` RegionToProxyServerID map[string]string `json:"region_to_proxy_server_id,omitempty"` Regions []string `json:"regions,omitempty"` @@ -239,7 +241,7 @@ func runPlannedSync( items []map[string]any, cloudAccounts []api.CloudAccount, ) (*Summary, error) { - plan, err := buildPlan(items, cloudAccounts, cfg.QueryID, cfg.SetupIDs) + plan, err := buildPlanForConfig(cfg, items, cloudAccounts) if err != nil { return nil, err } @@ -473,6 +475,7 @@ func RunAWSOrganizations(ctx context.Context, cfg AWSOrganizationConfig, source SetupID: setupID, RoleName: roleName, ExternalIDConfigured: externalID != "", + ExternalIDConsistent: true, Regions: regionList, ConfiguredAccountCount: 0, NQEAccountRowCount: 0, @@ -820,6 +823,7 @@ func buildSummary( RoleName: setup.RoleName, OrgID: setup.OrgID, ExternalIDConfigured: setup.ExternalIDConfigured, + ExternalIDConsistent: setup.ExternalIDConsistent, ProxyServerID: setup.ProxyServerID, RegionToProxyServerID: nonEmptyStringMap(setup.Payload.RegionToProxyServerID), Regions: regions, @@ -887,6 +891,7 @@ type plannedSetup struct { RoleName string OrgID int ExternalIDConfigured bool + ExternalIDConsistent bool ProxyServerID string Payload api.PatchPayload AddedAccounts []accountRow @@ -970,14 +975,30 @@ func (p *patchPlan) HasGovCloudRemovalsWithoutOrganizationEvidence() bool { } type buildPlanOptions struct { - RoleNameBySetup map[string]string - ExternalIDBySetup map[string]string + RoleNameBySetup map[string]string + ExternalIDBySetup map[string]string + ExternalIDByAccount externalIDAssignments } func buildPlan(items []map[string]any, cloudAccounts []api.CloudAccount, queryID string, requestedSetupIDs []string) (*patchPlan, error) { return buildPlanWithOptions(items, cloudAccounts, queryID, requestedSetupIDs, buildPlanOptions{}) } +func buildPlanForConfig(cfg Config, items []map[string]any, cloudAccounts []api.CloudAccount) (*patchPlan, error) { + defaultSetupID := "" + setupIDs := cleanSetupIDs(cfg.SetupIDs) + if len(setupIDs) == 1 { + defaultSetupID = setupIDs[0] + } + assignments, err := loadExternalIDAssignments(cfg.ExternalIDFile, defaultSetupID) + if err != nil { + return nil, err + } + return buildPlanWithOptions(items, cloudAccounts, cfg.QueryID, cfg.SetupIDs, buildPlanOptions{ + ExternalIDByAccount: assignments, + }) +} + func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccount, _ string, requestedSetupIDs []string, opts buildPlanOptions) (*patchPlan, error) { cloudMetaMap := cloudAccountMetaMap(cloudAccounts, requestedSetupIDs) if len(cloudMetaMap) == 0 { @@ -990,6 +1011,11 @@ func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccou groupedAccounts = map[string][]accountRow{fallbackSetupID: fallbackAccounts} } } + for setupID := range opts.ExternalIDByAccount { + if _, ok := groupedAccounts[setupID]; !ok { + return nil, fmt.Errorf("external ID file contains setup %s, but that setup is not present in the discovered account inventory", setupID) + } + } if len(groupedAccounts) == 0 { if len(cloudMetaMap) > 1 && hasAccountRows(items) { return nil, fmt.Errorf("NQE response has AWS accounts but no setup ID data; pass --query-id only if overriding the platform query") @@ -1021,25 +1047,42 @@ func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccou plan.Skips = append(plan.Skips, SkipSummary{SetupID: setupID, Reason: "unable to determine role ARN name from assumeRoleInfos"}) continue } - externalID := extractExternalID(meta.AssumeRoleInfos) - if override, ok := opts.ExternalIDBySetup[setupID]; ok { - externalID = strings.TrimSpace(override) - } - orgID := parseOrgID(externalID) partition := extractRolePartition(meta.AssumeRoleInfos) nextAccounts := groupedAccounts[setupID] current := currentAccounts(meta.AssumeRoleInfos) + added, removed, unchanged := accountDiff(current, nextAccounts) + uniformExternalID, hasUniformOverride := opts.ExternalIDBySetup[setupID] + if hasUniformOverride && len(opts.ExternalIDByAccount[setupID]) > 0 { + return nil, fmt.Errorf("setup %s has both setup-wide and per-account External ID overrides", setupID) + } + infos, err := buildAssumeRoleInfosPreservingExternalIDs( + nextAccounts, + meta.AssumeRoleInfos, + roleName, + partition, + hasUniformOverride, + strings.TrimSpace(uniformExternalID), + opts.ExternalIDByAccount[setupID], + ) + if err != nil { + return nil, fmt.Errorf("setup %s: %w", setupID, err) + } + externalIDConfigured, externalIDConsistent := externalIDState(infos) + externalID := "" + if externalIDConsistent && len(infos) > 0 { + externalID = strings.TrimSpace(infos[0].ExternalID) + } + orgID := parseOrgID(externalID) payload := api.PatchPayload{ Type: "AWS", Name: setupID, Regions: regionMap(meta.Regions), RegionToProxyServerID: stringMap(meta.RegionToProxyServerID), - AssumeRoleInfos: buildAssumeRoleInfosForPartition(nextAccounts, roleName, externalID, partition), + AssumeRoleInfos: infos, } if strings.TrimSpace(meta.ProxyServerID) != "" { payload.ProxyServerID = meta.ProxyServerID } - added, removed, unchanged := accountDiff(current, nextAccounts) collectedCount := countCollectedAccounts(items, setupID) candidateCount := countUncollectedCandidates(items, setupID) orgUnitRowCount := countOrgUnitRows(items, setupID) @@ -1048,7 +1091,8 @@ func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccou SetupID: setupID, RoleName: roleName, OrgID: orgID, - ExternalIDConfigured: externalID != "", + ExternalIDConfigured: externalIDConfigured, + ExternalIDConsistent: externalIDConsistent, ProxyServerID: meta.ProxyServerID, Payload: payload, AddedAccounts: added, @@ -1377,16 +1421,6 @@ func validateCloudAccountPartition(account api.CloudAccount) error { return nil } -func extractExternalID(assumeRoleInfos []api.AssumeRoleInfo) string { - for _, info := range assumeRoleInfos { - extID := strings.TrimSpace(info.ExternalID) - if extID != "" { - return extID - } - } - return "" -} - func parseOrgID(externalID string) int { var orgID int if _, err := fmt.Sscanf(strings.TrimSpace(externalID), "Org:%d", &orgID); err == nil { @@ -1416,6 +1450,80 @@ func buildAssumeRoleInfosForPartition(accounts []accountRow, roleName, externalI return result } +func buildAssumeRoleInfosPreservingExternalIDs( + accounts []accountRow, + current []api.AssumeRoleInfo, + roleName string, + partition string, + hasUniformOverride bool, + uniformOverride string, + assignments map[string]string, +) ([]api.AssumeRoleInfo, error) { + currentIDs := make(map[string]string, len(current)) + for _, info := range current { + accountID := assumeRoleAccountID(info) + if accountID != "" { + if _, exists := currentIDs[accountID]; exists { + return nil, fmt.Errorf("current setup contains duplicate account %s", accountID) + } + currentIDs[accountID] = strings.TrimSpace(info.ExternalID) + } + } + _, currentConsistent := externalIDState(current) + currentDefault := "" + if currentConsistent && len(current) > 0 { + currentDefault = strings.TrimSpace(current[0].ExternalID) + } + + nextIDs := make(map[string]bool, len(accounts)) + for _, account := range accounts { + nextIDs[account.AccountID] = true + } + unknownAssignments := make([]string, 0) + for accountID := range assignments { + if !nextIDs[accountID] { + unknownAssignments = append(unknownAssignments, accountID) + } + } + if len(unknownAssignments) > 0 { + sort.Strings(unknownAssignments) + return nil, fmt.Errorf("external ID file contains account(s) not present in the discovered inventory: %s", strings.Join(unknownAssignments, ", ")) + } + + result := make([]api.AssumeRoleInfo, 0, len(accounts)) + missingAssignments := make([]string, 0) + for _, account := range accounts { + externalID := "" + if hasUniformOverride { + externalID = uniformOverride + } else if assigned, ok := assignments[account.AccountID]; ok { + externalID = assigned + } else if existing, ok := currentIDs[account.AccountID]; ok { + externalID = existing + } else if currentConsistent { + externalID = currentDefault + } else { + missingAssignments = append(missingAssignments, account.AccountID) + } + info := api.AssumeRoleInfo{ + AccountID: account.AccountID, + AccountName: account.AccountName, + RoleArn: roleARN(partition, account.AccountID, roleName), + ExternalID: externalID, + Enabled: true, + } + result = append(result, info) + } + if len(missingAssignments) > 0 { + sort.Strings(missingAssignments) + return nil, fmt.Errorf( + "existing accounts use mixed External IDs; provide --external-id-file assignments for each new account: %s", + strings.Join(missingAssignments, ", "), + ) + } + return result, nil +} + func roleARN(partition, accountID, roleName string) string { return fmt.Sprintf("arn:%s:iam::%s:role/%s", partition, accountID, roleName) } diff --git a/internal/app/run_test.go b/internal/app/run_test.go index a55ff87..4c78977 100644 --- a/internal/app/run_test.go +++ b/internal/app/run_test.go @@ -476,6 +476,90 @@ func TestBuildPlanPreservesNonOrgExternalID(t *testing.T) { } } +func TestBuildPlanPreservesMixedPerAccountExternalIDs(t *testing.T) { + items := []map[string]any{ + {"Setup ID": "setup-a", "Cloud Account ID": "111111111111", "Cloud Account Name": "one"}, + {"Setup ID": "setup-a", "Cloud Account ID": "222222222222", "Cloud Account Name": "two"}, + } + cloudAccounts := []api.CloudAccount{{ + Name: "setup-a", + AssumeRoleInfos: []api.AssumeRoleInfo{ + {AccountID: "111111111111", RoleArn: "arn:aws:iam::111111111111:role/ForwardRole", ExternalID: "one-id", Enabled: true}, + {AccountID: "222222222222", RoleArn: "arn:aws:iam::222222222222:role/ForwardRole", ExternalID: "two-id", Enabled: true}, + }, + }} + + plan, err := buildPlan(items, cloudAccounts, "", nil) + if err != nil { + t.Fatalf("buildPlan() error = %v", err) + } + infos := plan.Payloads["setup-a"].AssumeRoleInfos + if infos[0].ExternalID != "one-id" || infos[1].ExternalID != "two-id" { + t.Fatalf("mixed External IDs were not preserved: %#v", infos) + } + if !plan.Setups[0].ExternalIDConfigured || plan.Setups[0].ExternalIDConsistent || plan.Setups[0].OrgID != 0 { + t.Fatalf("unexpected mixed External ID summary: %#v", plan.Setups[0]) + } +} + +func TestBuildPlanRequiresAssignmentsForNewAccountsInMixedSetup(t *testing.T) { + items := []map[string]any{ + {"Setup ID": "setup-a", "Cloud Account ID": "111111111111", "Cloud Account Name": "one"}, + {"Setup ID": "setup-a", "Cloud Account ID": "222222222222", "Cloud Account Name": "two"}, + {"Setup ID": "setup-a", "Cloud Account ID": "333333333333", "Cloud Account Name": "three"}, + } + cloudAccounts := []api.CloudAccount{{ + Name: "setup-a", + AssumeRoleInfos: []api.AssumeRoleInfo{ + {AccountID: "111111111111", RoleArn: "arn:aws:iam::111111111111:role/ForwardRole", ExternalID: "one-id", Enabled: true}, + {AccountID: "222222222222", RoleArn: "arn:aws:iam::222222222222:role/ForwardRole", ExternalID: "two-id", Enabled: true}, + }, + }} + + if _, err := buildPlan(items, cloudAccounts, "", nil); err == nil || !strings.Contains(err.Error(), "provide --external-id-file assignments") { + t.Fatalf("expected fail-closed mixed-ID addition error, got %v", err) + } + plan, err := buildPlanWithOptions(items, cloudAccounts, "", nil, buildPlanOptions{ + ExternalIDByAccount: externalIDAssignments{ + "setup-a": {"333333333333": "three-id"}, + }, + }) + if err != nil { + t.Fatalf("buildPlanWithOptions() error = %v", err) + } + infos := plan.Payloads["setup-a"].AssumeRoleInfos + if infos[0].ExternalID != "one-id" || infos[1].ExternalID != "two-id" || infos[2].ExternalID != "three-id" { + t.Fatalf("unexpected assigned mixed-ID payload: %#v", infos) + } +} + +func TestBuildPlanForConfigLoadsPerAccountExternalIDFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "external-ids.csv") + contents := "setup_id,account_id,action,external_id\nsetup-a,333333333333,set,three-id\n" + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + items := []map[string]any{ + {"Setup ID": "setup-a", "Cloud Account ID": "111111111111", "Cloud Account Name": "one"}, + {"Setup ID": "setup-a", "Cloud Account ID": "222222222222", "Cloud Account Name": "two"}, + {"Setup ID": "setup-a", "Cloud Account ID": "333333333333", "Cloud Account Name": "three"}, + } + cloudAccounts := []api.CloudAccount{{ + Name: "setup-a", + AssumeRoleInfos: []api.AssumeRoleInfo{ + {AccountID: "111111111111", RoleArn: "arn:aws:iam::111111111111:role/ForwardRole", ExternalID: "one-id", Enabled: true}, + {AccountID: "222222222222", RoleArn: "arn:aws:iam::222222222222:role/ForwardRole", ExternalID: "two-id", Enabled: true}, + }, + }} + plan, err := buildPlanForConfig(Config{ExternalIDFile: path}, items, cloudAccounts) + if err != nil { + t.Fatalf("buildPlanForConfig() error = %v", err) + } + if got := plan.Payloads["setup-a"].AssumeRoleInfos[2].ExternalID; got != "three-id" { + t.Fatalf("new account External ID = %q, want three-id", got) + } +} + func TestBuildPlanAllowsMultipleSetupsWithDefaultQueryWhenRowsHaveSetupIDs(t *testing.T) { items := []map[string]any{ {"Setup ID": "setup-a", "Cloud Account ID": "111", "Cloud Account Name": "acct-a"},