From 71de1a425e947c2bb4d0272c80341967d3cf6751 Mon Sep 17 00:00:00 2001 From: captainpacket Date: Fri, 17 Jul 2026 07:55:32 -0500 Subject: [PATCH 1/3] Add reversible AWS external ID migration --- README.md | 19 ++++ cmd/awssync/main.go | 76 ++++++++++++++ docs/aws-account-sync-procedure.md | 82 ++++++++++++++- docs/quick-start.md | 23 +++++ internal/api/client.go | 3 +- internal/app/external_id.go | 160 +++++++++++++++++++++++++++++ internal/app/external_id_test.go | 123 ++++++++++++++++++++++ 7 files changed, 482 insertions(+), 4 deletions(-) create mode 100644 internal/app/external_id.go create mode 100644 internal/app/external_id_test.go diff --git a/README.md b/README.md index c3aef30..3376e80 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,25 @@ The Forward collection IAM role name must be the same in every AWS account that Both Forward IAM role and IAM user/access-key multi-account setups are supported. In IAM user/access-key mode, Forward still uses the configured access key to assume the per-account role ARNs in `assumeRoleInfos`; the PATCH updates those account entries and leaves stored credentials unchanged. +An existing setup can add, replace, or clear its per-account External ID without changing those stored credentials. Use the dedicated one-time migration command; its dry run reads the current setup directly and does not depend on NQE or a new snapshot: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --format human + +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --apply \ + --yes +``` + +The value is written to every existing `assumeRoleInfos` entry for that setup. Review and apply the Forward payload first, test a representative account, and then update the target-role trust policies to require the identical value. After the migration PATCH, normal syncs preserve the stored External ID without rerunning this command. Use `external-id --clear` for an intentional rollback to null. Stored IAM access keys and secrets are not included in or changed by the PATCH. + ## Procedure For an end-to-end flow diagram showing connection types and required permissions, see [AWS Account Sync End-to-End Flow](docs/architecture-flow.md). diff --git a/cmd/awssync/main.go b/cmd/awssync/main.go index c18682b..b78c8c6 100644 --- a/cmd/awssync/main.go +++ b/cmd/awssync/main.go @@ -129,6 +129,7 @@ func newRootCommand() *cobra.Command { bindRunFlags(v, cmd.Flags()) cmd.AddCommand( newPreflightCommand(v), + newExternalIDCommand(v), newApplyPlanCommand(v), newStatusCommand(v), newWaitCommand(v), @@ -139,6 +140,60 @@ func newRootCommand() *cobra.Command { return cmd } +func newExternalIDCommand(v *viper.Viper) *cobra.Command { + cmd := &cobra.Command{ + Use: "external-id", + Short: "Set or clear the External ID on an existing AWS setup", + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, _ []string) error { + password, err := resolvePassword(v, os.Stdin, os.Stderr) + if err != nil { + return err + } + networkID, err := resolveNetworkIDForCLI(cmd.Context(), v, password, flagString(cmd, v, "network-id"), os.Stdin, os.Stderr) + if err != nil { + return err + } + setupID, _ := cmd.Flags().GetString("setup-id") + value, _ := cmd.Flags().GetString("value") + clearValue, _ := cmd.Flags().GetBool("clear") + apply, _ := cmd.Flags().GetBool("apply") + yes, _ := cmd.Flags().GetBool("yes") + if err := confirmApply(apply, yes, os.Stdin, os.Stderr); err != nil { + return err + } + 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, + }) + if err != nil { + return err + } + return emitResult(cmd, v, summary) + }, + } + bindNetworkFlag(v, cmd.Flags()) + 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().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") + return cmd +} + func bindCommonFlags(v *viper.Viper, flags *pflag.FlagSet) { flags.String("host", "", "Forward base URL, for example https://fwd.app") flags.String("username", "", "Forward username") @@ -920,11 +975,32 @@ func emitResult(cmd *cobra.Command, v *viper.Viper, value any) error { return emitPreflightHuman(summary) case *app.Summary: return emitSummaryHuman(summary) + case *app.ExternalIDSummary: + return emitExternalIDHuman(summary) default: return emitJSON(value) } } +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, " prior ID: configured=%t consistent=%t\n", summary.PreviousExternalIDConfigured, summary.PreviousExternalIDConsistent) + 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) + fmt.Fprintf(os.Stdout, " sha256: %s\n", summary.PayloadSHA256) + return nil +} + func emitPreflightHuman(summary *app.PreflightSummary) error { fmt.Fprintf(os.Stdout, "Preflight report\n") fmt.Fprintf(os.Stdout, " host: %s\n", summary.Host) diff --git a/docs/aws-account-sync-procedure.md b/docs/aws-account-sync-procedure.md index c1f81e2..7ceafcd 100644 --- a/docs/aws-account-sync-procedure.md +++ b/docs/aws-account-sync-procedure.md @@ -30,7 +30,7 @@ Important separation: - **Member account**: Any AWS account that belongs to the Organization. - **IAM role**: An AWS identity with permissions. In multi-account AWS setups, Forward assumes this role in each collected account. - **IAM user/access key**: A stored AWS credential that Forward can use to assume the per-account roles in a multi-account setup. -- **External ID**: An optional safety value used in the IAM trust policy when Forward assumes a role. IAM user/access-key setups commonly do not use a Forward external ID. +- **External ID**: An optional safety value used in the IAM trust policy when Forward assumes a role. An IAM user/access-key setup can use either no External ID or a customer-defined value, as long as Forward and the target role trust policy use the same value. - **Forward AWS setup**: The cloud account setup configured in Forward for AWS collection. ## Required AWS Model @@ -335,7 +335,7 @@ Review the JSON summary printed by the command: - `candidate_check`: whether uncollected candidate accounts were visible in the snapshot. If none are visible, verify the management or delegated discovery account before applying removals. - `organization_discovery_signal`: whether an Organization-level signal was visible for the setup (`visible_candidates`, `visible_ou_ids`, `visible_candidates_and_ou_ids`, or `no_org_signal`). - `role_name`: IAM role name that will be used in each generated role ARN. -- `external_id_configured`: whether an external ID from the existing setup will be preserved. This can be `false` for IAM user/access-key setups. +- `external_id_configured`: whether the normal sync payload preserves an External ID from the existing setup. - `payload_sha256`: fingerprint of the payload written to disk. - `manual_output`: optional path of setup-keyed manual payload for UI drag-and-drop. - `manual_payload_sha256`: fingerprint of the manual payload written to disk. @@ -348,7 +348,7 @@ Then review `aws_sync_payload.json`. Confirm: - Account IDs are expected 12-digit AWS account IDs. - Account names look correct. - Role ARNs use the intended role name. -- External ID is present only when the existing setup uses one. +- External ID matches the existing setup. Use the separate `external-id` command below when intentionally adding, replacing, or clearing it. - Regions and proxy settings match the existing Forward setup. - The PATCH payload does not include access keys or secrets; those stored credentials remain unchanged in Forward. - Removed accounts are expected. If removals are not expected, stop and inspect the Forward snapshot and NQE query before applying. @@ -358,6 +358,82 @@ If `--manual-output` is used, also confirm that manual payload file by opening i - Setup keys match `selected_setup_ids`. - Each setup value is an array of account records with generated role ARNs and external IDs (if configured). +## Add a Customer-Defined External ID to an Existing Setup + +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. + +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. + +First run a dry plan: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --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. + +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: + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "TrustForwardCollectorUser", + "Effect": "Allow", + "Principal": { + "AWS": "arn:aws:iam:::user/forward-collector" + }, + "Action": "sts:AssumeRole", + "Condition": { + "StringEquals": { + "sts:ExternalId": "customer-defined-value" + } + } + } + ] +} +``` + +Apply the reviewed Forward payload before making the condition mandatory in AWS. AWS can receive an External ID on `AssumeRole` even while the existing trust statement does not yet require one, which makes it possible to stage the change without intentionally breaking collection: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --apply \ + --yes +``` + +Run a Forward snapshot and verify one representative account still collects. Then roll out the matching trust-policy condition with the existing StackSet, Terraform module, or account-vending automation. Test the representative account again before enforcing it everywhere. After this one-time PATCH stores the new value, later normal syncs read and preserve it without rerunning `external-id`. + +An `sts:AssumeRole` failure after the trust-policy rollout usually means the trust policy principal or `sts:ExternalId` value does not exactly match the Forward setup payload. + +To roll back intentionally, first remove the mandatory External ID condition from the affected role trust policies, then dry-run and apply the clear operation: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --clear \ + --output aws_external_id_clear_payload.json + +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --clear \ + --output aws_external_id_clear_payload.json \ + --apply \ + --yes +``` + +The clear payload omits `externalId` from every `assumeRoleInfos` entry, which stores it as null in Forward. Test a representative account again after the rollback. + ## Run Preflight Checks `preflight` performs read-only checks and prints a JSON readiness report. diff --git a/docs/quick-start.md b/docs/quick-start.md index fc8dd2a..6027d43 100644 --- a/docs/quick-start.md +++ b/docs/quick-start.md @@ -71,6 +71,29 @@ If you need a manual fallback format for UI drag-and-drop, also review `aws_sync Stop if removed accounts are unexpected. +## Add an External ID to an Existing IAM User Setup + +This is a one-time change separate from the AWS Organizations setup checklist. To add a customer-defined External ID while keeping the existing IAM user/access-key credentials, use the dedicated command. It reads the existing setup directly, so it does not need NQE account discovery or a new snapshot: + +```bash +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --format human + +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --apply \ + --yes +``` + +Confirm the dry run reports the expected prior and target state and verify every generated account entry has the expected `externalId`. Apply the reviewed command and test one account. Then configure the identical `sts:ExternalId` condition in each target collection role trust policy, preferably through the existing StackSet or account-vending automation, and test again before broad rollout. + +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 roll back intentionally, replace `--value VALUE` with `--clear`, review the dry run, and apply it. + ## Discover Before Onboarding Prefer the Forward Terraform provider for native IaC onboarding. Use this CLI path for a new Forward AWS setup when you need manual review artifacts or cannot use the provider. It reads AWS Organizations directly and never patches an existing Forward setup. diff --git a/internal/api/client.go b/internal/api/client.go index 9e0b755..9a95cc8 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -82,6 +82,7 @@ type AssumeRoleInfo struct { AccountName string `json:"accountName,omitempty"` RoleArn string `json:"roleArn,omitempty"` ExternalID string `json:"externalId,omitempty"` + ErrorMsg string `json:"errorMsg,omitempty"` Enabled bool `json:"enabled"` } @@ -340,7 +341,7 @@ func (c *Client) CloudAccounts(ctx context.Context, networkID string) ([]CloudAc return accounts, nil } -func (c *Client) PatchCloudAccount(ctx context.Context, networkID, setupID string, payload PatchPayload) error { +func (c *Client) PatchCloudAccount(ctx context.Context, networkID, setupID string, payload any) error { if strings.TrimSpace(networkID) == "" { return fmt.Errorf("network ID is required") } diff --git a/internal/app/external_id.go b/internal/app/external_id.go new file mode 100644 index 0000000..f3b27b6 --- /dev/null +++ b/internal/app/external_id.go @@ -0,0 +1,160 @@ +package app + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/forwardnetworks/aws-sync/internal/api" +) + +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 +} + +type ExternalIDSummary struct { + Host string `json:"host"` + NetworkID string `json:"network_id"` + SetupID string `json:"setup_id"` + Apply bool `json:"apply"` + Patched bool `json:"patched"` + AccountCount int `json:"account_count"` + PreviousExternalIDConfigured bool `json:"previous_external_id_configured"` + PreviousExternalIDConsistent bool `json:"previous_external_id_consistent"` + TargetExternalIDConfigured bool `json:"target_external_id_configured"` + Output string `json:"output"` + PayloadSHA256 string `json:"payload_sha256"` + Payload ExternalIDPatchPayload `json:"payload"` +} + +type ExternalIDPatchPayload struct { + Type string `json:"type"` + AssumeRoleInfos []api.AssumeRoleInfo `json:"assumeRoleInfos"` +} + +func ChangeExternalID(ctx context.Context, cfg ExternalIDConfig) (*ExternalIDSummary, error) { + setupID := strings.TrimSpace(cfg.SetupID) + externalID := strings.TrimSpace(cfg.ExternalID) + if setupID == "" { + return nil, fmt.Errorf("setup ID is required") + } + if cfg.Clear == (externalID != "") { + return nil, fmt.Errorf("specify exactly one of an external ID value or clear") + } + + client, err := api.NewClient( + cfg.Host, + cfg.APIPrefix, + cfg.Username, + cfg.Password, + cfg.Insecure, + cfg.Timeout, + ) + if err != nil { + return nil, err + } + networkID, err := ResolveNetworkID(ctx, client, cfg.NetworkID) + if err != nil { + return nil, err + } + accounts, err := client.CloudAccounts(ctx, networkID) + if err != nil { + return nil, err + } + account, err := findAWSSetup(accounts, setupID) + if err != nil { + return nil, err + } + if len(account.AssumeRoleInfos) == 0 { + return nil, fmt.Errorf("AWS setup %s has no assumeRoleInfos to update", setupID) + } + + previousConfigured, previousConsistent := externalIDState(account.AssumeRoleInfos) + infos := make([]api.AssumeRoleInfo, len(account.AssumeRoleInfos)) + copy(infos, account.AssumeRoleInfos) + for i := range infos { + infos[i].ExternalID = externalID + } + payload := ExternalIDPatchPayload{ + Type: "AWS", + AssumeRoleInfos: infos, + } + output := strings.TrimSpace(cfg.Output) + if output == "" { + output = "aws_external_id_payload.json" + } + payloads := map[string]ExternalIDPatchPayload{setupID: payload} + output, sha, err := writeJSONPayload(output, payloads) + if err != nil { + return nil, err + } + + summary := &ExternalIDSummary{ + Host: cfg.Host, + NetworkID: networkID, + SetupID: setupID, + Apply: cfg.Apply, + AccountCount: len(infos), + PreviousExternalIDConfigured: previousConfigured, + PreviousExternalIDConsistent: previousConsistent, + TargetExternalIDConfigured: externalID != "", + Output: output, + PayloadSHA256: sha, + Payload: payload, + } + if !cfg.Apply { + return summary, nil + } + if _, _, err := writeJSONPayload(auditPath(output), payloads); err != nil { + return nil, err + } + if err := client.PatchCloudAccount(ctx, networkID, setupID, payload); err != nil { + return nil, fmt.Errorf("patch setup %s: %w", setupID, err) + } + summary.Patched = true + return summary, nil +} + +func findAWSSetup(accounts []api.CloudAccount, setupID string) (api.CloudAccount, error) { + for _, account := range accounts { + if account.Name != setupID { + continue + } + if account.Type != "" && !strings.EqualFold(account.Type, "AWS") { + return api.CloudAccount{}, fmt.Errorf("cloud setup %s is not an AWS setup", setupID) + } + return account, nil + } + return api.CloudAccount{}, fmt.Errorf("AWS setup %s was not found", setupID) +} + +func externalIDState(infos []api.AssumeRoleInfo) (configured bool, consistent bool) { + if len(infos) == 0 { + return false, true + } + first := strings.TrimSpace(infos[0].ExternalID) + configured = first != "" + consistent = true + for _, info := range infos[1:] { + value := strings.TrimSpace(info.ExternalID) + if value != first { + consistent = false + } + if value != "" { + configured = true + } + } + return configured, consistent +} diff --git a/internal/app/external_id_test.go b/internal/app/external_id_test.go new file mode 100644 index 0000000..13720d9 --- /dev/null +++ b/internal/app/external_id_test.go @@ -0,0 +1,123 @@ +package app + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/forwardnetworks/aws-sync/internal/api" +) + +func TestChangeExternalIDSetsAndClearsWithoutNQE(t *testing.T) { + stored := api.CloudAccount{ + Type: "AWS", + Name: "setup-a", + Regions: map[string]api.RegionMeta{ + "us-east-1": {TestInstant: 123}, + }, + AssumeRoleInfos: []api.AssumeRoleInfo{{ + AccountID: "111", + AccountName: "acct-a", + RoleArn: "arn:aws:iam::111:role/ForwardRole", + Enabled: true, + }, { + AccountID: "222", + AccountName: "failed-account", + ErrorMsg: "role is not configured", + Enabled: false, + }}, + } + patchCount := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user, pass, ok := r.BasicAuth() + if !ok || user != "alice" || pass != "secret" { + w.WriteHeader(http.StatusUnauthorized) + return + } + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/networks/network-1/cloudAccounts": + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode([]api.CloudAccount{stored}) + case r.Method == http.MethodPatch && r.URL.Path == "/api/networks/network-1/cloudAccounts/setup-a": + data, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("read patch: %v", err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + t.Fatalf("decode patch fields: %v", err) + } + if len(fields) != 2 || fields["type"] == nil || fields["assumeRoleInfos"] == nil { + t.Fatalf("external ID PATCH changed unrelated fields: %s", string(data)) + } + var payload api.PatchPayload + if err := json.Unmarshal(data, &payload); err != nil { + t.Fatalf("decode patch: %v", err) + } + stored.AssumeRoleInfos = payload.AssumeRoleInfos + patchCount++ + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + dir := t.TempDir() + base := ExternalIDConfig{ + Host: server.URL, + Username: "alice", + Password: "secret", + NetworkID: "network-1", + SetupID: "setup-a", + APIPrefix: "/api", + Insecure: true, + Apply: true, + } + setConfig := base + setConfig.ExternalID = "customer-value" + setConfig.Output = filepath.Join(dir, "set.json") + setSummary, err := ChangeExternalID(context.Background(), setConfig) + if err != nil { + t.Fatalf("set ChangeExternalID() error = %v", err) + } + if !setSummary.Patched || setSummary.PreviousExternalIDConfigured || !setSummary.TargetExternalIDConfigured { + t.Fatalf("unexpected set summary: %#v", setSummary) + } + if patchCount != 1 || stored.AssumeRoleInfos[0].ExternalID != "customer-value" || stored.AssumeRoleInfos[1].ExternalID != "customer-value" { + t.Fatalf("expected set PATCH: count=%d stored=%#v", patchCount, stored.AssumeRoleInfos) + } + if stored.AssumeRoleInfos[1].ErrorMsg != "role is not configured" { + t.Fatalf("expected account error to be preserved: %#v", stored.AssumeRoleInfos[1]) + } + + clearConfig := base + clearConfig.Clear = true + clearConfig.Output = filepath.Join(dir, "clear.json") + clearSummary, err := ChangeExternalID(context.Background(), clearConfig) + if err != nil { + t.Fatalf("clear ChangeExternalID() error = %v", err) + } + if !clearSummary.Patched || !clearSummary.PreviousExternalIDConfigured || clearSummary.TargetExternalIDConfigured { + t.Fatalf("unexpected clear summary: %#v", clearSummary) + } + if patchCount != 2 || stored.AssumeRoleInfos[0].ExternalID != "" || stored.AssumeRoleInfos[1].ExternalID != "" { + t.Fatalf("expected clear PATCH: count=%d stored=%#v", patchCount, stored.AssumeRoleInfos) + } +} + +func TestChangeExternalIDRequiresOneAction(t *testing.T) { + for _, cfg := range []ExternalIDConfig{ + {SetupID: "setup-a"}, + {SetupID: "setup-a", ExternalID: "value", Clear: true}, + } { + if _, err := ChangeExternalID(context.Background(), cfg); err == nil { + t.Fatalf("expected action validation error for %#v", cfg) + } + } +} From 04fa105ce94d86fce9c35ea92ee8f54366666717 Mon Sep 17 00:00:00 2001 From: captainpacket Date: Tue, 21 Jul 2026 17:25:33 -0500 Subject: [PATCH 2/3] Add safe AWS GovCloud account workflows --- .github/RELEASE_NOTES_TEMPLATE.md | 6 + README.md | 2 + cmd/awssync/main.go | 197 ++++++++++++++++++++++-- docs/govcloud-workflow.md | 159 +++++++++++++++++++ docs/quick-start.md | 2 + internal/app/account_manifest.go | 102 +++++++++++++ internal/app/account_manifest_test.go | 169 +++++++++++++++++++++ internal/app/apply_plan.go | 55 ++++++- internal/app/apply_plan_test.go | 47 +++++- internal/app/preflight.go | 17 +++ internal/app/run.go | 211 +++++++++++++++++++++++--- internal/app/run_test.go | 91 +++++++++++ internal/awsorg/discover.go | 31 +++- internal/awsorg/discover_test.go | 3 +- 14 files changed, 1049 insertions(+), 43 deletions(-) create mode 100644 docs/govcloud-workflow.md create mode 100644 internal/app/account_manifest.go create mode 100644 internal/app/account_manifest_test.go diff --git a/.github/RELEASE_NOTES_TEMPLATE.md b/.github/RELEASE_NOTES_TEMPLATE.md index c94962e..13d6639 100644 --- a/.github/RELEASE_NOTES_TEMPLATE.md +++ b/.github/RELEASE_NOTES_TEMPLATE.md @@ -2,6 +2,12 @@ ### Highlights +- Added AWS GovCloud workflows for both regular Forward Organizations/NQE discovery and reviewed standalone-account manifests. +- Added `onboard-accounts` and `sync-accounts` for environments where AWS Organizations is unavailable by policy. +- Preserved `arn:aws-us-gov` IAM role partitions and rejected mixed or region-mismatched role ARNs. +- Blocked GovCloud removals without positive Organizations evidence; authoritative manifest removals require explicit review and `--allow-removals`. +- Added collector instance-profile onboarding payloads for self-managed GovCloud collectors. + - Positioned the Forward Terraform provider as the native IaC workflow for new AWS Organizations onboarding. - Kept `awssync` focused on existing Forward setup synchronization from NQE data plus manual/break-glass onboarding artifacts. - Added `discover-org` for initial AWS Organizations onboarding before Forward has collected the org. diff --git a/README.md b/README.md index 3376e80..378fcd3 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,8 @@ For a short quick start, see [AWS Account Sync Quick Start](docs/quick-start.md) For the full procedure, including AWS Organizations prerequisites, management-account or delegated-account discovery checks, IAM role checks, dry-run review, apply, and post-apply validation, see [AWS Account Sync Procedure](docs/aws-account-sync-procedure.md). +For GovCloud Organizations and standalone-account workflows, including collector instance-profile credentials, GovCloud ARN validation, a reviewed account-manifest fallback, and stricter removal gates, see [AWS GovCloud Account Workflow](docs/govcloud-workflow.md). + ## Build ```bash diff --git a/cmd/awssync/main.go b/cmd/awssync/main.go index b78c8c6..734f7bc 100644 --- a/cmd/awssync/main.go +++ b/cmd/awssync/main.go @@ -134,6 +134,8 @@ func newRootCommand() *cobra.Command { newStatusCommand(v), newWaitCommand(v), newDiscoverOrgCommand(v), + newOnboardAccountsCommand(v), + newSyncAccountsCommand(v), newServeWebhookCommand(v), newConfigureWebhookCommand(v), ) @@ -359,14 +361,15 @@ func newApplyPlanCommand(v *viper.Viper) *cobra.Command { return err } summary, err := app.ApplyPlan(cmd.Context(), app.ApplyPlanConfig{ - Host: v.GetString("host"), - Username: v.GetString("username"), - Password: password, - NetworkID: networkID, - PlanPath: flagString(cmd, v, "plan"), - APIPrefix: v.GetString("api-prefix"), - Insecure: v.GetBool("insecure"), - Timeout: v.GetDuration("timeout"), + Host: v.GetString("host"), + Username: v.GetString("username"), + Password: password, + NetworkID: networkID, + PlanPath: flagString(cmd, v, "plan"), + APIPrefix: v.GetString("api-prefix"), + Insecure: v.GetBool("insecure"), + Timeout: v.GetDuration("timeout"), + AllowRemovals: flagBool(cmd, v, "allow-removals"), }) if err != nil { return err @@ -377,8 +380,10 @@ func newApplyPlanCommand(v *viper.Viper) *cobra.Command { bindNetworkFlag(v, cmd.Flags()) cmd.Flags().String("plan", "aws_sync_payload.json", "reviewed payload file to apply") cmd.Flags().Bool("yes", false, "confirm applying the reviewed payload file") + cmd.Flags().Bool("allow-removals", false, "allow reviewed commercial-partition account removals; GovCloud removals must use their source workflow") mustBind(v, cmd.Flags(), "plan") mustBind(v, cmd.Flags(), "yes") + mustBind(v, cmd.Flags(), "allow-removals") return cmd } @@ -469,6 +474,7 @@ func newDiscoverOrgCommand(v *viper.Viper) *cobra.Command { OrganizationID: discovered.OrganizationID, ManagementAccountID: discovered.ManagementAccountID, SkippedAccountCount: discovered.SkippedAccountCount, + Partition: discovered.Partition, } source.Accounts = make([]app.AWSOrganizationAccount, 0, len(discovered.Accounts)) for _, account := range discovered.Accounts { @@ -512,7 +518,7 @@ func newDiscoverOrgCommand(v *viper.Viper) *cobra.Command { cmd.Flags().String("role-name", "", "AWS IAM role name that Forward will assume in each discovered account") cmd.Flags().String("external-id", "", "optional external ID; fetched from Forward when host credentials are supplied") cmd.Flags().StringSlice("collect-region", nil, "AWS region for Forward to collect; repeatable or comma-separated") - cmd.Flags().String("credential-mode", app.CredentialModeForwardRole, "Forward collection credential mode: forward-role or static-keys") + cmd.Flags().String("credential-mode", app.CredentialModeForwardRole, "Forward collection credential mode: forward-role, static-keys, or instance-profile") cmd.Flags().String("collector-access-key-id", "", "collector AWS access key ID for static-keys mode") cmd.Flags().String("collector-secret-access-key", "", "collector AWS secret access key for static-keys mode; prefer AWSSYNC_COLLECTOR_SECRET_ACCESS_KEY") cmd.Flags().Bool("post", false, "POST the create payload to Forward after writing JSON files") @@ -539,6 +545,150 @@ func newDiscoverOrgCommand(v *viper.Viper) *cobra.Command { return cmd } +func newOnboardAccountsCommand(v *viper.Viper) *cobra.Command { + cmd := &cobra.Command{ + Use: "onboard-accounts", + SilenceUsage: true, + SilenceErrors: true, + Short: "Write Forward onboarding payloads from a reviewed AWS account manifest", + RunE: func(cmd *cobra.Command, _ []string) error { + setupIDs := cleanSetupIDs(flagStringSlice(cmd, v, "setup-id")) + if len(setupIDs) != 1 { + return fmt.Errorf("onboard-accounts requires exactly one --setup-id for the new Forward AWS setup") + } + accounts, err := app.LoadAWSAccountManifest(flagString(cmd, v, "accounts-file")) + if err != nil { + return err + } + password := "" + networkID := flagString(cmd, v, "network-id") + if strings.TrimSpace(v.GetString("host")) != "" { + password, err = resolvePassword(v, os.Stdin, os.Stderr) + if err != nil { + return err + } + networkID, err = resolveNetworkIDForCLI(cmd.Context(), v, password, networkID, os.Stdin, os.Stderr) + if err != nil { + return err + } + } + + credentialMode := flagString(cmd, v, "credential-mode") + collectorSecret, err := resolveCollectorSecret(v, credentialMode, flagBool(cmd, v, "post"), os.Stdin, os.Stderr) + if err != nil { + return err + } + if err := confirmPost(flagBool(cmd, v, "post"), flagBool(cmd, v, "yes"), setupIDs[0], os.Stdin, os.Stderr); err != nil { + return err + } + summary, err := app.RunAWSAccountManifest(cmd.Context(), app.AWSOrganizationConfig{ + Host: v.GetString("host"), + Username: v.GetString("username"), + Password: password, + NetworkID: networkID, + SetupIDs: setupIDs, + Output: flagString(cmd, v, "output"), + ManualOutput: flagString(cmd, v, "manual-output"), + RoleName: flagString(cmd, v, "role-name"), + ExternalID: flagString(cmd, v, "external-id"), + Regions: flagStringSlice(cmd, v, "collect-region"), + CredentialMode: credentialMode, + CollectorAccessKeyID: flagString(cmd, v, "collector-access-key-id"), + CollectorSecretAccessKey: collectorSecret, + Post: flagBool(cmd, v, "post"), + APIPrefix: v.GetString("api-prefix"), + Insecure: v.GetBool("insecure"), + Timeout: v.GetDuration("timeout"), + IncludeManual: true, + Partition: flagString(cmd, v, "partition"), + }, accounts) + if err != nil { + return err + } + return emitResult(cmd, v, summary) + }, + } + bindNetworkFlag(v, cmd.Flags()) + cmd.Flags().String("accounts-file", "", "reviewed JSON array of AWS accounts with id and optional name") + cmd.Flags().StringSlice("setup-id", nil, "new Forward AWS setup ID/name for generated onboarding payloads") + cmd.Flags().String("role-name", "", "AWS IAM role name that Forward will assume in each account") + cmd.Flags().String("external-id", "", "optional external ID; fetched from Forward when host credentials are supplied") + cmd.Flags().StringSlice("collect-region", nil, "AWS region for Forward to collect; repeatable or comma-separated") + cmd.Flags().String("partition", "aws", "AWS ARN partition: aws, aws-us-gov, or aws-cn") + cmd.Flags().String("credential-mode", app.CredentialModeForwardRole, "Forward collection credential mode: forward-role, static-keys, or instance-profile") + cmd.Flags().String("collector-access-key-id", "", "collector AWS access key ID for static-keys mode") + cmd.Flags().String("collector-secret-access-key", "", "collector AWS secret access key for static-keys mode; prefer AWSSYNC_COLLECTOR_SECRET_ACCESS_KEY") + cmd.Flags().Bool("post", false, "POST the create payload to Forward after writing JSON files") + cmd.Flags().Bool("yes", false, "skip create confirmation prompt") + cmd.Flags().String("output", "", "Forward create-setup POST JSON path (defaults to aws_create_payload_.json)") + cmd.Flags().String("manual-output", "", "manual drag-and-drop JSON path (defaults to fwd_accounts_data_.json)") + for _, name := range []string{"accounts-file", "setup-id", "role-name", "external-id", "collect-region", "partition", "credential-mode", "collector-access-key-id", "collector-secret-access-key", "post", "yes", "output", "manual-output"} { + mustBind(v, cmd.Flags(), name) + } + return cmd +} + +func newSyncAccountsCommand(v *viper.Viper) *cobra.Command { + cmd := &cobra.Command{ + Use: "sync-accounts", + SilenceUsage: true, + SilenceErrors: true, + Short: "Plan or apply an existing AWS setup from an authoritative account manifest", + RunE: func(cmd *cobra.Command, _ []string) error { + setupIDs := cleanSetupIDs(flagStringSlice(cmd, v, "setup-id")) + if len(setupIDs) != 1 { + return fmt.Errorf("sync-accounts requires exactly one --setup-id") + } + accounts, err := app.LoadAWSAccountManifest(flagString(cmd, v, "accounts-file")) + if err != nil { + return err + } + password, err := resolvePassword(v, os.Stdin, os.Stderr) + if err != nil { + return err + } + networkID, err := resolveNetworkIDForCLI(cmd.Context(), v, password, flagString(cmd, v, "network-id"), os.Stdin, os.Stderr) + if err != nil { + return err + } + apply := flagBool(cmd, v, "apply") + if err := confirmApply(apply, flagBool(cmd, v, "yes"), os.Stdin, os.Stderr); err != nil { + return err + } + summary, err := app.SyncAWSAccountManifest(cmd.Context(), app.Config{ + Host: v.GetString("host"), + Username: v.GetString("username"), + Password: password, + NetworkID: networkID, + SetupIDs: setupIDs, + Output: flagString(cmd, v, "output"), + ManualOutput: flagString(cmd, v, "manual-output"), + APIPrefix: v.GetString("api-prefix"), + Insecure: v.GetBool("insecure"), + Timeout: v.GetDuration("timeout"), + Apply: apply, + AllowRemovals: flagBool(cmd, v, "allow-removals"), + }, accounts) + if err != nil { + return err + } + return emitResult(cmd, v, summary) + }, + } + bindNetworkFlag(v, cmd.Flags()) + cmd.Flags().String("accounts-file", "", "authoritative reviewed JSON array of AWS accounts with id and optional name") + cmd.Flags().StringSlice("setup-id", nil, "existing Forward AWS setup ID") + cmd.Flags().String("output", "", "output JSON path for the generated PATCH payload") + cmd.Flags().String("manual-output", "", "optional UI-friendly account JSON output path") + cmd.Flags().Bool("apply", false, "PATCH the generated setup payload into Forward") + cmd.Flags().Bool("yes", false, "skip apply confirmation prompt") + cmd.Flags().Bool("allow-removals", false, "allow reviewed manifest entries to remove accounts from the setup") + for _, name := range []string{"accounts-file", "setup-id", "output", "manual-output", "apply", "yes", "allow-removals"} { + mustBind(v, cmd.Flags(), name) + } + return cmd +} + func newServeWebhookCommand(v *viper.Viper) *cobra.Command { cmd := &cobra.Command{ Use: "serve-webhook", @@ -1026,6 +1176,12 @@ func emitPreflightHuman(summary *app.PreflightSummary) error { len(setup.AddedAccounts), len(setup.RemovedAccounts), ) + if len(setup.AddedAccounts) > 0 { + fmt.Fprintf(os.Stdout, " added: %s\n", accountSummaryIDs(setup.AddedAccounts)) + } + if len(setup.RemovedAccounts) > 0 { + fmt.Fprintf(os.Stdout, " removed: %s\n", accountSummaryIDs(setup.RemovedAccounts)) + } fmt.Fprintf(os.Stdout, " %s\n", setup.OrganizationDiscoveryMessage) } if summary.Ready { @@ -1038,7 +1194,7 @@ func emitPreflightHuman(summary *app.PreflightSummary) error { } func emitSummaryHuman(summary *app.Summary) error { - if summary.Source == "aws_organizations" { + if summary.Source == "aws_organizations" || (summary.Source == "account_manifest" && summary.CreatePayload != nil) { return emitAWSOrganizationsHuman(summary) } fmt.Fprintf(os.Stdout, "Sync report\n") @@ -1077,6 +1233,12 @@ func emitSummaryHuman(summary *app.Summary) error { len(setup.RemovedAccounts), setup.UnchangedAccountCount, ) + if len(setup.AddedAccounts) > 0 { + fmt.Fprintf(os.Stdout, " added: %s\n", accountSummaryIDs(setup.AddedAccounts)) + } + if len(setup.RemovedAccounts) > 0 { + fmt.Fprintf(os.Stdout, " removed: %s\n", accountSummaryIDs(setup.RemovedAccounts)) + } fmt.Fprintf(os.Stdout, " %s\n", setup.OrganizationDiscoveryMessage) } fmt.Fprintln(os.Stdout, "\nSummary:") @@ -1084,8 +1246,21 @@ func emitSummaryHuman(summary *app.Summary) error { return nil } +func accountSummaryIDs(accounts []app.AccountSummary) string { + ids := make([]string, 0, len(accounts)) + for _, account := range accounts { + ids = append(ids, account.AccountID) + } + sort.Strings(ids) + return strings.Join(ids, ", ") +} + func emitAWSOrganizationsHuman(summary *app.Summary) error { - fmt.Fprintf(os.Stdout, "AWS Organizations onboarding report\n") + title := "AWS Organizations onboarding report" + if summary.Source == "account_manifest" { + title = "AWS account-manifest onboarding report" + } + fmt.Fprintln(os.Stdout, title) if summary.Host != "" { fmt.Fprintf(os.Stdout, " host: %s\n", summary.Host) } diff --git a/docs/govcloud-workflow.md b/docs/govcloud-workflow.md new file mode 100644 index 0000000..7877962 --- /dev/null +++ b/docs/govcloud-workflow.md @@ -0,0 +1,159 @@ +# AWS GovCloud Account Workflow + +Use this workflow for AWS GovCloud (US) accounts, including customers that cannot use AWS Organizations. + +AWS Organizations is available in GovCloud, but a GovCloud organization is independent from a commercial AWS organization. Its Organizations control plane is in `us-gov-west-1`. Forward's regular AWS collection pipeline can query Organizations using the GovCloud setup credentials and primary region. + +There are two supported inventory paths: + +1. **GovCloud Organizations + Forward NQE** is preferred when the customer can grant read access to the GovCloud organization. +2. **Reviewed account manifest** is the fallback for standalone accounts or customers that cannot grant Organizations access. + +Do not treat a successfully collected GovCloud region as proof that Organizations discovery succeeded. Resource collection and organization inventory are separate checks. + +## Credential Model + +For a self-managed collector running in GovCloud, prefer its EC2 instance profile. Give that instance-profile role permission to assume the consistent collection role in every target GovCloud account. + +The generated target role ARNs must use the GovCloud partition: + +```text +arn:aws-us-gov:iam::111111111111:role/ForwardReadOnlyAccess +``` + +Do not use `arn:aws:iam` for GovCloud accounts. If a customer-defined External ID is required, configure the identical value in Forward's per-account entries and in every target role trust policy. + +## Path A: Regular Forward Organizations Discovery + +Use this path when the configured GovCloud account is an Organizations management account or has the required delegated/read access. + +The regular Forward collector uses these APIs: + +- `organizations:DescribeAccount` +- `organizations:ListAccounts` +- `organizations:ListRoots` +- `organizations:ListOrganizationalUnitsForParent` + +Configure at least `us-gov-west-1` in the Forward AWS setup. Run a Forward connectivity test and a new snapshot, then run read-only preflight: + +```bash +./bin/awssync preflight \ + --network-id NETWORK_ID \ + --setup-id GOVCLOUD_SETUP \ + --max-snapshot-age 24h \ + --format human +``` + +Preflight must confirm all of the following before any removal: + +- the setup's role ARNs consistently use `arn:aws-us-gov`; +- the configured collection regions are GovCloud regions; +- the current snapshot returns AWS accounts for the selected setup; +- Forward NQE exposes positive Organizations evidence, such as uncollected candidate accounts or Organizational Unit IDs; +- every proposed removed account ID has been reviewed. + +An account directly under the organization root may have no OU ID. A missing OU ID alone does not prove failure, but a removal plan with neither candidate accounts nor OU evidence is unsafe. GovCloud removals from the NQE path are blocked in that state and cannot be forced with the generic no-evidence flags. + +If preflight is ready and the plan has no removals, generate the payload normally. If it proposes removals, review the exact IDs printed by the human report and the JSON payload before applying. + +## Path B: Manual Account Manifest + +Use this path when the customer has standalone GovCloud accounts, Organizations is unavailable by policy, or Forward cannot see the GovCloud organization. + +Create a reviewed JSON file containing the complete authoritative account inventory: + +```json +[ + { + "id": "111111111111", + "name": "security" + }, + { + "id": "222222222222", + "name": "production" + } +] +``` + +Account IDs must contain exactly 12 digits and must be unique. Keep real customer manifests in the customer's approved secret/configuration system; do not commit them to this repository. + +### Create a New Forward Setup + +Generate review artifacts without changing Forward: + +```bash +./bin/awssync onboard-accounts \ + --accounts-file govcloud-accounts.json \ + --partition aws-us-gov \ + --credential-mode instance-profile \ + --setup-id GOVCLOUD_SETUP \ + --role-name ForwardReadOnlyAccess \ + --collect-region us-gov-west-1 \ + --external-id CUSTOMER_DEFINED_VALUE \ + --output govcloud-create.json \ + --manual-output govcloud-fwd-accounts.json \ + --format human +``` + +Verify that every `roleArn` begins with `arn:aws-us-gov:iam::`. To create the setup after review, supply the Forward connection settings and add `--post --yes`. Without `--post`, this command only writes files. + +### Update an Existing Forward Setup + +First generate a dry plan from the complete manifest: + +```bash +./bin/awssync sync-accounts \ + --network-id NETWORK_ID \ + --setup-id GOVCLOUD_SETUP \ + --accounts-file govcloud-accounts.json \ + --output govcloud-sync-plan.json \ + --format human +``` + +The report prints the exact added and removed account IDs. A dry run never patches Forward. + +If there are no removals, apply the reviewed plan with: + +```bash +./bin/awssync sync-accounts \ + --network-id NETWORK_ID \ + --setup-id GOVCLOUD_SETUP \ + --accounts-file govcloud-accounts.json \ + --output govcloud-sync-plan.json \ + --apply \ + --yes +``` + +If removals are intentional, the apply is blocked unless the operator also supplies `--allow-removals`: + +```bash +./bin/awssync sync-accounts \ + --network-id NETWORK_ID \ + --setup-id GOVCLOUD_SETUP \ + --accounts-file govcloud-accounts.json \ + --output govcloud-sync-plan.json \ + --apply \ + --allow-removals \ + --yes +``` + +After any update, run a Forward connectivity test for representative accounts, run a new snapshot, and inspect per-account collection errors. + +Do not use `apply-plan` to bypass these source checks. `apply-plan` reloads the current Forward setup before patching and refuses GovCloud account removals; rerun the NQE or manifest workflow that produced the inventory instead. + +## When This Is a Forward Product Issue + +Escalate as a possible Forward collection enhancement or defect only when all of these are true: + +1. Forward successfully collects GovCloud resources with the configured GovCloud credential path. +2. The caller is in the expected GovCloud organization and can run the four Organizations APIs above directly. +3. The Forward setup includes `us-gov-west-1` and uses GovCloud role ARNs. +4. A fresh Forward snapshot still reports no organization member accounts or OU inventory. +5. Collector logs show that the Organizations request used the GovCloud context, or show a reproducible failure from that request. + +Capture the setup ID, snapshot ID, collector version, Organizations error, configured regions, and redacted role ARN partition. Do not include secrets or full customer manifests in the issue. + +AWS references: + +- [AWS Organizations in AWS GovCloud (US)](https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-organizations.html) +- [Region support for AWS Organizations](https://docs.aws.amazon.com/organizations/latest/userguide/region-support.html) diff --git a/docs/quick-start.md b/docs/quick-start.md index 6027d43..b1012f2 100644 --- a/docs/quick-start.md +++ b/docs/quick-start.md @@ -4,6 +4,8 @@ Use `awssync` to update an existing Forward AWS setup when AWS Organization acco For new AWS Organizations onboarding, prefer the Forward Terraform provider as the native IaC workflow. It supports Forward assume-role, static-key, and collector instance-profile credential models. Use `awssync discover-org` only when Forward has not onboarded that AWS Organization yet and you need manual JSON files, a break-glass create payload, or a static-key workflow that should stay outside Terraform state. +For AWS GovCloud, use the dedicated [AWS GovCloud Account Workflow](govcloud-workflow.md). It covers both the regular Forward Organizations/NQE path and an authoritative account-manifest path for customers without Organizations access. + ## Before You Start - Forward must collect the AWS management account or a delegated account that can list AWS Organizations accounts. diff --git a/internal/app/account_manifest.go b/internal/app/account_manifest.go new file mode 100644 index 0000000..e2e8cf8 --- /dev/null +++ b/internal/app/account_manifest.go @@ -0,0 +1,102 @@ +package app + +import ( + "context" + "encoding/json" + "fmt" + "os" + "regexp" + "strings" + + "github.com/forwardnetworks/aws-sync/internal/api" +) + +var awsAccountIDPattern = regexp.MustCompile(`^[0-9]{12}$`) + +type AWSAccountManifestEntry struct { + ID string `json:"id"` + Name string `json:"name,omitempty"` +} + +func LoadAWSAccountManifest(path string) ([]AWSOrganizationAccount, error) { + path = strings.TrimSpace(path) + if path == "" { + return nil, fmt.Errorf("--accounts-file is required") + } + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("open accounts file: %w", err) + } + defer file.Close() + + decoder := json.NewDecoder(file) + decoder.DisallowUnknownFields() + var entries []AWSAccountManifestEntry + if err := decoder.Decode(&entries); err != nil { + return nil, fmt.Errorf("decode accounts file: %w", err) + } + if len(entries) == 0 { + return nil, fmt.Errorf("accounts file contains no accounts") + } + + seen := make(map[string]bool, len(entries)) + accounts := make([]AWSOrganizationAccount, 0, len(entries)) + for index, entry := range entries { + accountID := strings.TrimSpace(entry.ID) + if !awsAccountIDPattern.MatchString(accountID) { + return nil, fmt.Errorf("accounts file entry %d has invalid AWS account ID %q; expected 12 digits", index+1, entry.ID) + } + if seen[accountID] { + return nil, fmt.Errorf("accounts file contains duplicate AWS account ID %s", accountID) + } + seen[accountID] = true + accountName := strings.TrimSpace(entry.Name) + if accountName == "" { + accountName = accountID + } + accounts = append(accounts, AWSOrganizationAccount{ID: accountID, Name: accountName}) + } + return accounts, nil +} + +func RunAWSAccountManifest(ctx context.Context, cfg AWSOrganizationConfig, accounts []AWSOrganizationAccount) (*Summary, error) { + return RunAWSOrganizations(ctx, cfg, AWSOrganizationSource{ + Accounts: accounts, + Partition: cfg.Partition, + Source: "account_manifest", + DiscoveryMessage: "Account inventory came from the explicitly reviewed manifest; AWS Organizations was not queried", + }) +} + +func SyncAWSAccountManifest(ctx context.Context, cfg Config, accounts []AWSOrganizationAccount) (*Summary, error) { + setupIDs := cleanSetupIDs(cfg.SetupIDs) + if len(setupIDs) != 1 { + return nil, fmt.Errorf("account-manifest sync requires exactly one --setup-id") + } + client, err := api.NewClient(cfg.Host, cfg.APIPrefix, cfg.Username, cfg.Password, cfg.Insecure, cfg.Timeout) + if err != nil { + return nil, err + } + networkID, err := ResolveNetworkID(ctx, client, cfg.NetworkID) + if err != nil { + return nil, err + } + cfg.NetworkID = networkID + cfg.Source = "account_manifest" + cfg.AuthoritativeInput = true + + cloudAccounts, err := client.CloudAccounts(ctx, networkID) + if err != nil { + return nil, err + } + items := make([]map[string]any, 0, len(accounts)) + for _, account := range accounts { + items = append(items, map[string]any{ + "Cloud Setup ID": setupIDs[0], + "Cloud Account ID": account.ID, + "Cloud Account Name": account.Name, + "Collected?": false, + }) + } + return runPlannedSync(ctx, cfg, client, items, cloudAccounts) +} diff --git a/internal/app/account_manifest_test.go b/internal/app/account_manifest_test.go new file mode 100644 index 0000000..6edfc39 --- /dev/null +++ b/internal/app/account_manifest_test.go @@ -0,0 +1,169 @@ +package app + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/forwardnetworks/aws-sync/internal/api" +) + +func TestLoadAWSAccountManifestValidatesAndNormalizes(t *testing.T) { + path := filepath.Join(t.TempDir(), "accounts.json") + if err := os.WriteFile(path, []byte(`[ + {"id":"111111111111","name":"security"}, + {"id":"222222222222"} +]`), 0o600); err != nil { + t.Fatal(err) + } + + accounts, err := LoadAWSAccountManifest(path) + if err != nil { + t.Fatalf("LoadAWSAccountManifest() error = %v", err) + } + if len(accounts) != 2 || accounts[0].Name != "security" || accounts[1].Name != "222222222222" { + t.Fatalf("unexpected accounts: %#v", accounts) + } +} + +func TestLoadAWSAccountManifestRejectsInvalidAndDuplicateIDs(t *testing.T) { + for name, contents := range map[string]string{ + "invalid": `[{"id":"123"}]`, + "duplicate": `[{"id":"111111111111"},{"id":"111111111111"}]`, + "unknown": `[{"id":"111111111111","email":"private@example.gov"}]`, + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "accounts.json") + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + if _, err := LoadAWSAccountManifest(path); err == nil { + t.Fatal("expected manifest validation error") + } + }) + } +} + +func TestRunAWSAccountManifestBuildsGovCloudRoleARNs(t *testing.T) { + output := filepath.Join(t.TempDir(), "payload.json") + summary, err := RunAWSAccountManifest(context.Background(), AWSOrganizationConfig{ + SetupIDs: []string{"gov-prod"}, + RoleName: "ForwardReadOnlyAccess", + ExternalID: "customer-value", + Regions: []string{"us-gov-west-1"}, + Partition: "aws-us-gov", + CredentialMode: CredentialModeInstanceProfile, + Output: output, + IncludeManual: true, + }, []AWSOrganizationAccount{{ID: "111111111111", Name: "security"}}) + if err != nil { + t.Fatalf("RunAWSAccountManifest() error = %v", err) + } + if summary.Source != "account_manifest" || summary.PostedSetupCount != 0 { + t.Fatalf("unexpected summary: %#v", summary) + } + if !strings.Contains(summary.PlannedSetups[0].OrganizationDiscoveryMessage, "Organizations was not queried") { + t.Fatalf("unexpected discovery message: %#v", summary.PlannedSetups[0]) + } + + var payload api.CreateAWSPayload + data, err := os.ReadFile(output) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(data, &payload); err != nil { + t.Fatal(err) + } + want := "arn:aws-us-gov:iam::111111111111:role/ForwardReadOnlyAccess" + if got := payload.AssumeRoleInfos[0].RoleArn; got != want { + t.Fatalf("role ARN = %q, want %q", got, want) + } + if payload.UseForwardAccountToAssumeRole == nil || *payload.UseForwardAccountToAssumeRole { + t.Fatalf("expected collector instance-profile mode: %#v", payload) + } + if payload.Username != "" || payload.Password != "" { + t.Fatalf("instance-profile payload must not contain static credentials: %#v", payload) + } +} + +func TestRunAWSAccountManifestRejectsPartitionRegionMismatch(t *testing.T) { + _, err := RunAWSAccountManifest(context.Background(), AWSOrganizationConfig{ + SetupIDs: []string{"gov-prod"}, + RoleName: "ForwardReadOnlyAccess", + Regions: []string{"us-east-1"}, + Partition: "aws-us-gov", + Output: filepath.Join(t.TempDir(), "payload.json"), + }, []AWSOrganizationAccount{{ID: "111111111111"}}) + if err == nil || !strings.Contains(err.Error(), "does not belong to partition") { + t.Fatalf("expected partition mismatch, got %v", err) + } +} + +func TestSyncAWSAccountManifestDryRunReportsRemovalAndApplyRequiresApproval(t *testing.T) { + patchCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/networks/network-1/cloudAccounts": + _, _ = w.Write([]byte(`[{"type":"AWS","name":"gov-prod","regions":{"us-gov-west-1":{"testInstant":1}},"assumeRoleInfos":[ + {"accountId":"111111111111","accountName":"keep","roleArn":"arn:aws-us-gov:iam::111111111111:role/ForwardRole","enabled":true}, + {"accountId":"222222222222","accountName":"remove","roleArn":"arn:aws-us-gov:iam::222222222222:role/ForwardRole","enabled":true} + ]}]`)) + case r.Method == http.MethodPatch && r.URL.Path == "/api/networks/network-1/cloudAccounts/gov-prod": + patchCount++ + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + accounts := []AWSOrganizationAccount{{ID: "111111111111", Name: "keep"}} + base := Config{ + Host: server.URL, + Username: "user", + Password: "pass", + NetworkID: "network-1", + SetupIDs: []string{"gov-prod"}, + APIPrefix: "/api", + Output: filepath.Join(t.TempDir(), "plan.json"), + Apply: false, + } + summary, err := SyncAWSAccountManifest(context.Background(), base, accounts) + if err != nil { + t.Fatalf("dry run error = %v", err) + } + if summary.Source != "account_manifest" || len(summary.PlannedSetups[0].RemovedAccounts) != 1 { + t.Fatalf("expected one visible manifest removal: %#v", summary) + } + if got := summary.PlannedSetups[0].RemovedAccounts[0].AccountID; got != "222222222222" { + t.Fatalf("removed account = %q", got) + } + if patchCount != 0 { + t.Fatalf("dry run unexpectedly patched %d setup(s)", patchCount) + } + + blocked := base + blocked.Apply = true + blocked.Output = filepath.Join(t.TempDir(), "blocked.json") + if _, err := SyncAWSAccountManifest(context.Background(), blocked, accounts); err == nil || !strings.Contains(err.Error(), "--allow-removals") { + t.Fatalf("expected removal approval error, got %v", err) + } + if patchCount != 0 { + t.Fatalf("blocked apply unexpectedly patched %d setup(s)", patchCount) + } + + approved := blocked + approved.AllowRemovals = true + approved.Output = filepath.Join(t.TempDir(), "approved.json") + if _, err := SyncAWSAccountManifest(context.Background(), approved, accounts); err != nil { + t.Fatalf("approved apply error = %v", err) + } + if patchCount != 1 { + t.Fatalf("approved apply patch count = %d, want 1", patchCount) + } +} diff --git a/internal/app/apply_plan.go b/internal/app/apply_plan.go index fb99f07..87db309 100644 --- a/internal/app/apply_plan.go +++ b/internal/app/apply_plan.go @@ -14,14 +14,15 @@ import ( ) type ApplyPlanConfig struct { - Host string - Username string - Password string - NetworkID string - PlanPath string - APIPrefix string - Insecure bool - Timeout time.Duration + Host string + Username string + Password string + NetworkID string + PlanPath string + APIPrefix string + Insecure bool + Timeout time.Duration + AllowRemovals bool } type ApplyPlanSummary struct { @@ -69,6 +70,44 @@ func ApplyPlan(ctx context.Context, cfg ApplyPlanConfig) (*ApplyPlanSummary, err if len(setupIDs) == 0 { return nil, fmt.Errorf("plan contains no setup payloads") } + cloudAccounts, err := client.CloudAccounts(ctx, cfg.NetworkID) + if err != nil { + return nil, fmt.Errorf("load current cloud setups before apply: %w", err) + } + currentByName := make(map[string]api.CloudAccount, len(cloudAccounts)) + for _, account := range cloudAccounts { + currentByName[strings.TrimSpace(account.Name)] = account + } + for _, setupID := range setupIDs { + current, ok := currentByName[setupID] + if !ok { + return nil, fmt.Errorf("plan setup %s does not exist in Forward", setupID) + } + if err := validateCloudAccountPartition(current); err != nil { + return nil, fmt.Errorf("setup %s: %w", setupID, err) + } + payload := payloads[setupID] + planned := api.CloudAccount{Name: setupID, AssumeRoleInfos: payload.AssumeRoleInfos} + if len(payload.Regions) > 0 { + planned.Regions = make(map[string]api.RegionMeta, len(payload.Regions)) + for region, instant := range payload.Regions { + planned.Regions[region] = api.RegionMeta{TestInstant: instant} + } + } + if err := validateCloudAccountPartition(planned); err != nil { + return nil, fmt.Errorf("plan setup %s: %w", setupID, err) + } + _, removed, _ := accountDiff(currentAccounts(current.AssumeRoleInfos), currentAccounts(payload.AssumeRoleInfos)) + if len(removed) == 0 { + continue + } + if extractRolePartition(current.AssumeRoleInfos) == "aws-us-gov" { + return nil, fmt.Errorf("apply-plan cannot remove GovCloud accounts; rerun preflight/NQE with positive Organizations evidence or use sync-accounts with the authoritative manifest") + } + if !cfg.AllowRemovals { + return nil, fmt.Errorf("plan removes %d account(s) from setup %s; apply-plan requires --allow-removals", len(removed), setupID) + } + } sort.Strings(setupIDs) for _, setupID := range setupIDs { if err := client.PatchCloudAccount(ctx, cfg.NetworkID, setupID, payloads[setupID]); err != nil { diff --git a/internal/app/apply_plan_test.go b/internal/app/apply_plan_test.go index 1484b43..059739c 100644 --- a/internal/app/apply_plan_test.go +++ b/internal/app/apply_plan_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" ) @@ -17,8 +18,12 @@ func TestApplyPlanPatchesReviewedPayload(t *testing.T) { w.WriteHeader(http.StatusUnauthorized) return } + if r.Method == http.MethodGet && r.URL.Path == "/api/networks/network-1/cloudAccounts" { + _, _ = w.Write([]byte(`[{"type":"AWS","name":"setup-a","assumeRoleInfos":[]}]`)) + return + } if r.Method != http.MethodPatch { - w.WriteHeader(http.StatusNotFound) + http.NotFound(w, r) return } patched = append(patched, r.URL.Path) @@ -57,3 +62,43 @@ func TestApplyPlanPatchesReviewedPayload(t *testing.T) { t.Fatalf("expected payload sha: %+v", summary) } } + +func TestApplyPlanCannotBypassGovCloudRemovalSafety(t *testing.T) { + patched := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/networks/network-1/cloudAccounts": + _, _ = w.Write([]byte(`[{"type":"AWS","name":"gov-prod","regions":{"us-gov-west-1":{"testInstant":1}},"assumeRoleInfos":[ + {"accountId":"111111111111","roleArn":"arn:aws-us-gov:iam::111111111111:role/ForwardRole","enabled":true}, + {"accountId":"222222222222","roleArn":"arn:aws-us-gov:iam::222222222222:role/ForwardRole","enabled":true} + ]}]`)) + case r.Method == http.MethodPatch: + patched = true + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + planPath := filepath.Join(t.TempDir(), "payload.json") + if err := os.WriteFile(planPath, []byte(`{"gov-prod":{"type":"AWS","name":"gov-prod","regions":{"us-gov-west-1":1},"assumeRoleInfos":[ + {"accountId":"111111111111","roleArn":"arn:aws-us-gov:iam::111111111111:role/ForwardRole","enabled":true} + ]}}`), 0o600); err != nil { + t.Fatal(err) + } + _, err := ApplyPlan(context.Background(), ApplyPlanConfig{ + Host: server.URL, + Username: "user", + Password: "pass", + NetworkID: "network-1", + PlanPath: planPath, + APIPrefix: "/api", + AllowRemovals: true, + }) + if err == nil || !strings.Contains(err.Error(), "cannot remove GovCloud accounts") { + t.Fatalf("expected GovCloud apply-plan block, got %v", err) + } + if patched { + t.Fatal("unsafe GovCloud apply-plan reached PATCH") + } +} diff --git a/internal/app/preflight.go b/internal/app/preflight.go index 161a052..d37219e 100644 --- a/internal/app/preflight.go +++ b/internal/app/preflight.go @@ -99,6 +99,18 @@ func Preflight(ctx context.Context, cfg Config) (*PreflightSummary, error) { } setupIDValues := nqeSetupIDValues(items) awsSetups := cloudAccountMetaMap(cloudAccounts, cfg.SetupIDs) + partitionIssues := make([]string, 0) + for setupID, setup := range awsSetups { + if err := validateCloudAccountPartition(setup); err != nil { + partitionIssues = append(partitionIssues, fmt.Sprintf("%s: %s", setupID, err)) + } + } + sort.Strings(partitionIssues) + if len(partitionIssues) > 0 { + result.fail("aws_partition_consistency", strings.Join(partitionIssues, "; ")) + return result, nil + } + result.pass("aws_partition_consistency", "IAM role ARN partitions match the configured AWS regions") if len(setupIDValues) == 0 && len(awsSetups) > 1 { result.fail("nqe_setup_id_differentiator", "NQE rows did not include Cloud Setup ID; multiple AWS setups cannot be separated. Use the default inline query or a saved query that selects cloudAccount.cloudSetupId as Cloud Setup ID.") } else if len(setupIDValues) == 0 { @@ -130,6 +142,11 @@ func Preflight(ctx context.Context, cfg Config) (*PreflightSummary, error) { missingOrgEvidenceSetups := plansWithoutOrganizationEvidence(plan) if len(missingOrgEvidenceSetups) == 0 { result.pass("aws_organizations_evidence", "Forward NQE exposes candidate rows or Organizational Unit IDs for every selected setup") + } else if plan.HasGovCloudRemovalsWithoutOrganizationEvidence() { + result.fail( + "aws_organizations_evidence", + fmt.Sprintf("GovCloud removals are blocked without positive Organizations evidence for: %s; use sync-accounts with an authoritative reviewed manifest if Organizations is unavailable", strings.Join(missingOrgEvidenceSetups, ", ")), + ) } else if plan.HasNoOrganizationEvidenceForRemovals() { result.fail( "aws_organizations_evidence", diff --git a/internal/app/run.go b/internal/app/run.go index ede0576..88d8226 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -38,9 +38,10 @@ select { };` const ( - CredentialModeForwardRole = "forward-role" - CredentialModeStaticKeys = "static-keys" - collectorSecretPlaceholder = "REPLACE_WITH_COLLECTOR_SECRET_ACCESS_KEY" + CredentialModeForwardRole = "forward-role" + CredentialModeStaticKeys = "static-keys" + CredentialModeInstanceProfile = "instance-profile" + collectorSecretPlaceholder = "REPLACE_WITH_COLLECTOR_SECRET_ACCESS_KEY" ) type Config struct { @@ -63,6 +64,8 @@ type Config struct { AllowNoCandidates bool AllowNoOrgEvidence bool MaxSnapshotAge time.Duration + Source string + AuthoritativeInput bool } type Summary struct { @@ -161,6 +164,9 @@ type AWSOrganizationSource struct { ManagementAccountID string Accounts []AWSOrganizationAccount SkippedAccountCount int + Partition string + Source string + DiscoveryMessage string } type ManualAccountData struct { @@ -190,6 +196,7 @@ type AWSOrganizationConfig struct { Insecure bool Timeout time.Duration IncludeManual bool + Partition string } func Run(ctx context.Context, cfg Config) (*Summary, error) { @@ -217,6 +224,16 @@ func Run(ctx context.Context, cfg Config) (*Summary, error) { if err != nil { return nil, err } + return runPlannedSync(ctx, cfg, client, items, cloudAccounts) +} + +func runPlannedSync( + ctx context.Context, + cfg Config, + client *api.Client, + items []map[string]any, + cloudAccounts []api.CloudAccount, +) (*Summary, error) { plan, err := buildPlan(items, cloudAccounts, cfg.QueryID, cfg.SetupIDs) if err != nil { return nil, err @@ -263,11 +280,15 @@ func Run(ctx context.Context, cfg Config) (*Summary, error) { summary.RemovalBlocked = true return summary, fmt.Errorf("planned account removals require --allow-removals") } - if cfg.Apply && plan.HasCandidateRemovalRisk() && !cfg.AllowNoCandidates { + if cfg.Apply && !cfg.AuthoritativeInput && plan.HasCandidateRemovalRisk() && !cfg.AllowNoCandidates { summary.RemovalBlocked = true return summary, fmt.Errorf("planned removals with no uncollected candidate accounts visible require --allow-no-candidates") } - if cfg.Apply && plan.HasNoOrganizationEvidenceForRemovals() && !cfg.AllowNoOrgEvidence { + if cfg.Apply && !cfg.AuthoritativeInput && plan.HasGovCloudRemovalsWithoutOrganizationEvidence() { + summary.RemovalBlocked = true + return summary, fmt.Errorf("GovCloud account removals require positive AWS Organizations evidence; use sync-accounts with an authoritative reviewed manifest when Organizations is unavailable") + } + if cfg.Apply && !cfg.AuthoritativeInput && plan.HasNoOrganizationEvidenceForRemovals() && !cfg.AllowNoOrgEvidence { missingSetups := strings.Join(plan.setupsWithoutOrganizationEvidenceForRemovals(), ", ") summary.RemovalBlocked = true return summary, fmt.Errorf("planned removals with no AWS Organizations evidence in NQE for setup(s): %s require --allow-no-org-evidence", missingSetups) @@ -313,9 +334,28 @@ func RunAWSOrganizations(ctx context.Context, cfg AWSOrganizationConfig, source if err != nil { return nil, err } + sourceName := strings.TrimSpace(source.Source) + if sourceName == "" { + sourceName = "aws_organizations" + } + discoveryMessage := strings.TrimSpace(source.DiscoveryMessage) + if discoveryMessage == "" { + discoveryMessage = "AWS Organizations DescribeOrganization, ListAccounts, and ListParents succeeded; account data came directly from AWS Organizations" + } + partitionValue := strings.TrimSpace(source.Partition) + if partitionValue == "" { + partitionValue = cfg.Partition + } + partition, err := normalizeAWSPartition(partitionValue) + if err != nil { + return nil, err + } + if err := validateRegionsForPartition(regions, partition); err != nil { + return nil, err + } accounts := awsOrganizationAccountRows(source.Accounts) if len(accounts) == 0 { - return nil, fmt.Errorf("AWS Organizations discovery returned no active accounts") + return nil, fmt.Errorf("%s returned no accounts", sourceName) } networkID := strings.TrimSpace(cfg.NetworkID) @@ -348,8 +388,8 @@ func RunAWSOrganizations(ctx context.Context, cfg AWSOrganizationConfig, source } } - manualAccountData := buildManualAccountData(accounts, roleName, externalID) - createPayload, createPayloadReady, err := buildCreateAWSPayload(setupID, accounts, roleName, externalID, regions, credentialMode, cfg) + manualAccountData := buildManualAccountData(accounts, roleName, externalID, partition) + createPayload, createPayloadReady, err := buildCreateAWSPayload(setupID, accounts, roleName, externalID, regions, credentialMode, partition, cfg) if err != nil { return nil, err } @@ -397,7 +437,7 @@ func RunAWSOrganizations(ctx context.Context, cfg AWSOrganizationConfig, source summary := &Summary{ Host: cfg.Host, NetworkID: networkID, - Source: "aws_organizations", + Source: sourceName, AWSOrganizationID: source.OrganizationID, AWSManagementID: source.ManagementAccountID, AWSAccountCount: len(source.Accounts), @@ -428,15 +468,15 @@ func RunAWSOrganizations(ctx context.Context, cfg AWSOrganizationConfig, source NQECollectedRowCount: 0, NQECandidateRowCount: 0, NQEOrgUnitRowCount: countAccountsWithOrgUnit(source.Accounts), - OrganizationDiscoverySignal: "aws_organizations_api", - OrganizationDiscoveryMessage: "AWS Organizations DescribeOrganization, ListAccounts, and ListParents succeeded; account data came directly from AWS Organizations", + OrganizationDiscoverySignal: sourceName, + OrganizationDiscoveryMessage: discoveryMessage, PlannedPayloadAccountCount: len(createPayload.AssumeRoleInfos), AddedAccounts: accountSummaries, UnchangedAccountCount: 0, Patched: false, }}, } - summary.Source = "aws_organizations" + summary.Source = sourceName summary.AWSOrganizationID = source.OrganizationID summary.AWSManagementID = source.ManagementAccountID summary.AWSAccountCount = len(source.Accounts) @@ -450,13 +490,54 @@ func normalizeCredentialMode(mode string) (string, error) { return CredentialModeForwardRole, nil } switch mode { - case CredentialModeForwardRole, CredentialModeStaticKeys: + case CredentialModeForwardRole, CredentialModeStaticKeys, CredentialModeInstanceProfile: return mode, nil default: - return "", fmt.Errorf("invalid credential mode %q; expected %q or %q", mode, CredentialModeForwardRole, CredentialModeStaticKeys) + return "", fmt.Errorf( + "invalid credential mode %q; expected %q, %q, or %q", + mode, + CredentialModeForwardRole, + CredentialModeStaticKeys, + CredentialModeInstanceProfile, + ) + } +} + +func normalizeAWSPartition(partition string) (string, error) { + partition = strings.TrimSpace(strings.ToLower(partition)) + if partition == "" { + return "aws", nil + } + switch partition { + case "aws", "aws-us-gov", "aws-cn": + return partition, nil + default: + return "", fmt.Errorf("invalid AWS partition %q; expected aws, aws-us-gov, or aws-cn", partition) } } +func validateRegionsForPartition(regions []string, partition string) error { + for _, region := range regions { + region = strings.TrimSpace(strings.ToLower(region)) + if region == "" { + continue + } + valid := false + switch partition { + case "aws-us-gov": + valid = strings.HasPrefix(region, "us-gov-") + case "aws-cn": + valid = strings.HasPrefix(region, "cn-") + case "aws": + valid = !strings.HasPrefix(region, "us-gov-") && !strings.HasPrefix(region, "cn-") + } + if !valid { + return fmt.Errorf("AWS region %q does not belong to partition %q", region, partition) + } + } + return nil +} + func cloudAccountNameExists(cloudAccounts []api.CloudAccount, setupID string) bool { setupID = strings.TrimSpace(setupID) for _, account := range cloudAccounts { @@ -487,10 +568,10 @@ func awsOrganizationAccountRows(accounts []AWSOrganizationAccount) []accountRow return rows } -func buildManualAccountData(accounts []accountRow, roleName, externalID string) []ManualAccountData { +func buildManualAccountData(accounts []accountRow, roleName, externalID, partition string) []ManualAccountData { result := make([]ManualAccountData, 0, len(accounts)) for _, account := range accounts { - roleArn := fmt.Sprintf("arn:aws:iam::%s:role/%s", account.AccountID, roleName) + roleArn := roleARN(partition, account.AccountID, roleName) entry := ManualAccountData{ ID: account.AccountID, Name: account.AccountName, @@ -511,6 +592,7 @@ func buildCreateAWSPayload( externalID string, regions []string, credentialMode string, + partition string, cfg AWSOrganizationConfig, ) (api.CreateAWSPayload, bool, error) { payload := api.CreateAWSPayload{ @@ -518,7 +600,7 @@ func buildCreateAWSPayload( Name: setupID, Collect: true, Regions: selectedRegionMap(regions), - AssumeRoleInfos: buildAssumeRoleInfos(accounts, roleName, externalID), + AssumeRoleInfos: buildAssumeRoleInfosForPartition(accounts, roleName, externalID, partition), } ready := true switch credentialMode { @@ -537,6 +619,9 @@ func buildCreateAWSPayload( payload.Password = collectorSecretPlaceholder ready = false } + case CredentialModeInstanceProfile: + useForwardAccount := false + payload.UseForwardAccountToAssumeRole = &useForwardAccount default: return payload, false, fmt.Errorf("invalid credential mode %q", credentialMode) } @@ -713,6 +798,12 @@ func buildSummary( regions = append(regions, region) } sort.Strings(regions) + discoverySignal := organizationDiscoveryStatus(setup.DiscoveredCandidateCount, setup.DiscoveredOrgUnitRowCount) + discoveryMessage := organizationDiscoveryMessage(setup.DiscoveredCandidateCount, setup.DiscoveredOrgUnitRowCount) + if cfg.AuthoritativeInput { + discoverySignal = "account_manifest" + discoveryMessage = "Account inventory came from the explicitly reviewed manifest; AWS Organizations was not queried" + } setupSummaries = append(setupSummaries, SetupSummary{ SetupID: setup.SetupID, RoleName: setup.RoleName, @@ -726,8 +817,8 @@ func buildSummary( NQECollectedRowCount: setup.DiscoveredCollectedCount, NQECandidateRowCount: setup.DiscoveredCandidateCount, NQEOrgUnitRowCount: setup.DiscoveredOrgUnitRowCount, - OrganizationDiscoverySignal: organizationDiscoveryStatus(setup.DiscoveredCandidateCount, setup.DiscoveredOrgUnitRowCount), - OrganizationDiscoveryMessage: organizationDiscoveryMessage(setup.DiscoveredCandidateCount, setup.DiscoveredOrgUnitRowCount), + OrganizationDiscoverySignal: discoverySignal, + OrganizationDiscoveryMessage: discoveryMessage, PlannedPayloadAccountCount: len(setup.Payload.AssumeRoleInfos), AddedAccounts: accountSummaries(setup.AddedAccounts), RemovedAccounts: accountSummaries(setup.RemovedAccounts), @@ -739,6 +830,7 @@ func buildSummary( return &Summary{ Host: cfg.Host, NetworkID: cfg.NetworkID, + Source: strings.TrimSpace(cfg.Source), SnapshotID: cfg.SnapshotID, QueryID: strings.TrimSpace(cfg.QueryID), QueryOverride: strings.TrimSpace(cfg.QueryID) != "", @@ -851,6 +943,21 @@ func (p *patchPlan) HasRemovals() bool { return false } +func (p *patchPlan) HasGovCloudRemovalsWithoutOrganizationEvidence() bool { + for _, setup := range p.Setups { + if len(setup.RemovedAccounts) == 0 || organizationDiscoveryVisible( + setup.DiscoveredCandidateCount, + setup.DiscoveredOrgUnitRowCount, + ) { + continue + } + if extractRolePartition(setup.Payload.AssumeRoleInfos) == "aws-us-gov" { + return true + } + } + return false +} + type buildPlanOptions struct { RoleNameBySetup map[string]string ExternalIDBySetup map[string]string @@ -892,6 +999,9 @@ func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccou plan.Skips = append(plan.Skips, SkipSummary{SetupID: setupID, Reason: "setup metadata not found in Forward"}) continue } + if err := validateCloudAccountPartition(meta); err != nil { + return nil, fmt.Errorf("setup %s: %w", setupID, err) + } roleName := extractRoleName(meta.AssumeRoleInfos) if override := strings.TrimSpace(opts.RoleNameBySetup[setupID]); override != "" { roleName = override @@ -905,6 +1015,7 @@ func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccou externalID = strings.TrimSpace(override) } orgID := parseOrgID(externalID) + partition := extractRolePartition(meta.AssumeRoleInfos) nextAccounts := groupedAccounts[setupID] current := currentAccounts(meta.AssumeRoleInfos) payload := api.PatchPayload{ @@ -912,7 +1023,7 @@ func buildPlanWithOptions(items []map[string]any, cloudAccounts []api.CloudAccou Name: setupID, Regions: regionMap(meta.Regions), RegionToProxyServerID: stringMap(meta.RegionToProxyServerID), - AssumeRoleInfos: buildAssumeRoleInfos(nextAccounts, roleName, externalID), + AssumeRoleInfos: buildAssumeRoleInfosForPartition(nextAccounts, roleName, externalID, partition), } if strings.TrimSpace(meta.ProxyServerID) != "" { payload.ProxyServerID = meta.ProxyServerID @@ -1205,6 +1316,56 @@ func extractRoleName(assumeRoleInfos []api.AssumeRoleInfo) string { return "" } +func extractRolePartition(assumeRoleInfos []api.AssumeRoleInfo) string { + for _, info := range assumeRoleInfos { + parts := strings.Split(strings.TrimSpace(info.RoleArn), ":") + if len(parts) >= 6 && parts[0] == "arn" && parts[2] == "iam" { + if partition, err := normalizeAWSPartition(parts[1]); err == nil { + return partition + } + } + } + return "aws" +} + +func validateCloudAccountPartition(account api.CloudAccount) error { + rolePartitions := make(map[string]bool) + for _, info := range account.AssumeRoleInfos { + parts := strings.Split(strings.TrimSpace(info.RoleArn), ":") + if len(parts) < 6 || parts[0] != "arn" || parts[2] != "iam" { + continue + } + partition, err := normalizeAWSPartition(parts[1]) + if err != nil { + return err + } + rolePartitions[partition] = true + } + if len(rolePartitions) > 1 { + partitions := make([]string, 0, len(rolePartitions)) + for partition := range rolePartitions { + partitions = append(partitions, partition) + } + sort.Strings(partitions) + return fmt.Errorf("mixed IAM role ARN partitions are unsafe: %s", strings.Join(partitions, ", ")) + } + if len(rolePartitions) == 0 || len(account.Regions) == 0 { + return nil + } + var rolePartition string + for partition := range rolePartitions { + rolePartition = partition + } + regions := make([]string, 0, len(account.Regions)) + for region := range account.Regions { + regions = append(regions, region) + } + if err := validateRegionsForPartition(regions, rolePartition); err != nil { + return fmt.Errorf("role ARN partition and configured regions disagree: %w", err) + } + return nil +} + func extractExternalID(assumeRoleInfos []api.AssumeRoleInfo) string { for _, info := range assumeRoleInfos { extID := strings.TrimSpace(info.ExternalID) @@ -1224,12 +1385,16 @@ func parseOrgID(externalID string) int { } func buildAssumeRoleInfos(accounts []accountRow, roleName, externalID string) []api.AssumeRoleInfo { + return buildAssumeRoleInfosForPartition(accounts, roleName, externalID, "aws") +} + +func buildAssumeRoleInfosForPartition(accounts []accountRow, roleName, externalID, partition string) []api.AssumeRoleInfo { result := make([]api.AssumeRoleInfo, 0, len(accounts)) for _, account := range accounts { info := api.AssumeRoleInfo{ AccountID: account.AccountID, AccountName: account.AccountName, - RoleArn: fmt.Sprintf("arn:aws:iam::%s:role/%s", account.AccountID, roleName), + RoleArn: roleARN(partition, account.AccountID, roleName), Enabled: true, } if externalID != "" { @@ -1240,6 +1405,10 @@ func buildAssumeRoleInfos(accounts []accountRow, roleName, externalID string) [] return result } +func roleARN(partition, accountID, roleName string) string { + return fmt.Sprintf("arn:%s:iam::%s:role/%s", partition, accountID, roleName) +} + func currentAccounts(infos []api.AssumeRoleInfo) []accountRow { accounts := make([]accountRow, 0, len(infos)) for _, info := range infos { diff --git a/internal/app/run_test.go b/internal/app/run_test.go index c627e41..5498a4e 100644 --- a/internal/app/run_test.go +++ b/internal/app/run_test.go @@ -337,6 +337,97 @@ func TestBuildPlanSupportsRoleARNsWithoutExternalID(t *testing.T) { } } +func TestBuildPlanPreservesGovCloudRolePartition(t *testing.T) { + items := []map[string]any{ + {"Cloud Setup ID": "setup-gov", "Cloud Account ID": "111111111111", "Cloud Account Name": "kept"}, + {"Cloud Setup ID": "setup-gov", "Cloud Account ID": "222222222222", "Cloud Account Name": "added"}, + } + cloudAccounts := []api.CloudAccount{{ + Name: "setup-gov", + AssumeRoleInfos: []api.AssumeRoleInfo{{ + AccountID: "111111111111", + RoleArn: "arn:aws-us-gov:iam::111111111111:role/ForwardRole", + Enabled: true, + }}, + }} + + plan, err := buildPlan(items, cloudAccounts, "", nil) + if err != nil { + t.Fatalf("buildPlan() error = %v", err) + } + want := "arn:aws-us-gov:iam::222222222222:role/ForwardRole" + if got := plan.Payloads["setup-gov"].AssumeRoleInfos[1].RoleArn; got != want { + t.Fatalf("new account role ARN = %q, want %q", got, want) + } +} + +func TestBuildPlanRejectsMixedOrMismatchedAWSPartitions(t *testing.T) { + items := []map[string]any{{"Cloud Setup ID": "setup-gov", "Cloud Account ID": "111111111111"}} + for name, setup := range map[string]api.CloudAccount{ + "mixed roles": { + Name: "setup-gov", + AssumeRoleInfos: []api.AssumeRoleInfo{ + {RoleArn: "arn:aws-us-gov:iam::111111111111:role/ForwardRole"}, + {RoleArn: "arn:aws:iam::222222222222:role/ForwardRole"}, + }, + }, + "region mismatch": { + Name: "setup-gov", + Regions: map[string]api.RegionMeta{"us-gov-west-1": {TestInstant: 1}}, + AssumeRoleInfos: []api.AssumeRoleInfo{{RoleArn: "arn:aws:iam::111111111111:role/ForwardRole"}}, + }, + } { + t.Run(name, func(t *testing.T) { + if _, err := buildPlan(items, []api.CloudAccount{setup}, "", nil); err == nil { + t.Fatal("expected unsafe partition plan to fail") + } + }) + } +} + +func TestRunBlocksGovCloudRemovalWithoutOrgEvidenceEvenWithBreakGlassFlags(t *testing.T) { + patched := false + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/api/nqe": + _, _ = w.Write([]byte(`{"items":[{"Cloud Setup ID":"gov-prod","Cloud Account ID":"111111111111","Cloud Account Name":"keep","Collected?":true}]}`)) + case r.Method == http.MethodGet && r.URL.Path == "/api/networks/network-1/cloudAccounts": + _, _ = w.Write([]byte(`[{"type":"AWS","name":"gov-prod","regions":{"us-gov-west-1":{"testInstant":1}},"assumeRoleInfos":[ + {"accountId":"111111111111","roleArn":"arn:aws-us-gov:iam::111111111111:role/ForwardRole","enabled":true}, + {"accountId":"222222222222","roleArn":"arn:aws-us-gov:iam::222222222222:role/ForwardRole","enabled":true} + ]}]`)) + case r.Method == http.MethodPatch: + patched = true + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + _, err := Run(context.Background(), Config{ + Host: server.URL, + Username: "user", + Password: "pass", + NetworkID: "network-1", + SnapshotID: "snapshot-1", + SetupIDs: []string{"gov-prod"}, + Output: filepath.Join(t.TempDir(), "plan.json"), + APIPrefix: "/api", + Insecure: true, + Apply: true, + AllowRemovals: true, + AllowNoCandidates: true, + AllowNoOrgEvidence: true, + }) + if err == nil || !strings.Contains(err.Error(), "GovCloud account removals require positive AWS Organizations evidence") { + t.Fatalf("expected GovCloud Organizations safety block, got %v", err) + } + if patched { + t.Fatal("unsafe GovCloud removal reached PATCH") + } +} + func TestBuildPlanFiltersRequestedSetupIDs(t *testing.T) { items := []map[string]any{ {"Cloud Setup ID": "setup-a", "Cloud Account ID": "111", "Cloud Account Name": "acct-a"}, diff --git a/internal/awsorg/discover.go b/internal/awsorg/discover.go index d540406..e4e384e 100644 --- a/internal/awsorg/discover.go +++ b/internal/awsorg/discover.go @@ -22,6 +22,7 @@ type Config struct { type Result struct { OrganizationID string `json:"organization_id,omitempty"` ManagementAccountID string `json:"management_account_id,omitempty"` + Partition string `json:"partition,omitempty"` Accounts []Account `json:"accounts"` SkippedAccountCount int `json:"skipped_account_count,omitempty"` } @@ -49,7 +50,14 @@ func Discover(ctx context.Context, cfg Config) (*Result, error) { return nil, fmt.Errorf("load AWS credentials/config: %w", err) } client := organizations.NewFromConfig(awsCfg) - return DiscoverWithClient(ctx, client, cfg.IncludeSuspended) + result, err := DiscoverWithClient(ctx, client, cfg.IncludeSuspended) + if err != nil { + return nil, err + } + if result.Partition == "" { + result.Partition = partitionFromRegion(region) + } + return result, nil } type Client interface { @@ -67,6 +75,7 @@ func DiscoverWithClient(ctx context.Context, client Client, includeSuspended boo if orgOutput.Organization != nil { result.OrganizationID = aws.ToString(orgOutput.Organization.Id) result.ManagementAccountID = aws.ToString(orgOutput.Organization.MasterAccountId) + result.Partition = partitionFromARN(aws.ToString(orgOutput.Organization.Arn)) } if result.OrganizationID == "" { return nil, fmt.Errorf("describe AWS organization: response did not include organization id") @@ -98,6 +107,26 @@ func DiscoverWithClient(ctx context.Context, client Client, includeSuspended boo return result, nil } +func partitionFromARN(arn string) string { + parts := strings.Split(strings.TrimSpace(arn), ":") + if len(parts) >= 2 && parts[0] == "arn" { + return parts[1] + } + return "" +} + +func partitionFromRegion(region string) string { + region = strings.TrimSpace(strings.ToLower(region)) + switch { + case strings.HasPrefix(region, "us-gov-"): + return "aws-us-gov" + case strings.HasPrefix(region, "cn-"): + return "aws-cn" + default: + return "aws" + } +} + func accountFromAWS(account orgtypes.Account) Account { return Account{ ID: aws.ToString(account.Id), diff --git a/internal/awsorg/discover_test.go b/internal/awsorg/discover_test.go index 4d74db5..8f9123b 100644 --- a/internal/awsorg/discover_test.go +++ b/internal/awsorg/discover_test.go @@ -26,6 +26,7 @@ func (f fakeOrganizationsClient) DescribeOrganization(context.Context, *organiza return &organizations.DescribeOrganizationOutput{ Organization: &orgtypes.Organization{ Id: aws.String("o-example"), + Arn: aws.String("arn:aws-us-gov:organizations::111111111111:organization/o-example"), MasterAccountId: aws.String("111111111111"), }, }, nil @@ -62,7 +63,7 @@ func TestDiscoverWithClientChecksOrganizationAndParents(t *testing.T) { if err != nil { t.Fatalf("DiscoverWithClient() error = %v", err) } - if result.OrganizationID != "o-example" || result.ManagementAccountID != "111111111111" { + if result.OrganizationID != "o-example" || result.ManagementAccountID != "111111111111" || result.Partition != "aws-us-gov" { t.Fatalf("unexpected organization metadata: %#v", result) } if len(result.Accounts) != 2 || result.SkippedAccountCount != 1 { From 3f310c6f494ccc2fd9ed2db22fca97325cab9e73 Mon Sep 17 00:00:00 2001 From: captainpacket Date: Tue, 21 Jul 2026 17:32:43 -0500 Subject: [PATCH 3/3] Document GovCloud release workflows --- .github/RELEASE_NOTES_TEMPLATE.md | 61 +++++++++------- docs/architecture-flow.md | 117 +++++++++++++++++++++++++++--- 2 files changed, 142 insertions(+), 36 deletions(-) diff --git a/.github/RELEASE_NOTES_TEMPLATE.md b/.github/RELEASE_NOTES_TEMPLATE.md index 13d6639..6929e91 100644 --- a/.github/RELEASE_NOTES_TEMPLATE.md +++ b/.github/RELEASE_NOTES_TEMPLATE.md @@ -2,23 +2,14 @@ ### Highlights +- Added reversible one-time External ID migration for existing AWS setups with `external-id --value` and `external-id --clear`. - Added AWS GovCloud workflows for both regular Forward Organizations/NQE discovery and reviewed standalone-account manifests. - Added `onboard-accounts` and `sync-accounts` for environments where AWS Organizations is unavailable by policy. - Preserved `arn:aws-us-gov` IAM role partitions and rejected mixed or region-mismatched role ARNs. - Blocked GovCloud removals without positive Organizations evidence; authoritative manifest removals require explicit review and `--allow-removals`. - Added collector instance-profile onboarding payloads for self-managed GovCloud collectors. - -- Positioned the Forward Terraform provider as the native IaC workflow for new AWS Organizations onboarding. -- Kept `awssync` focused on existing Forward setup synchronization from NQE data plus manual/break-glass onboarding artifacts. -- Added `discover-org` for initial AWS Organizations onboarding before Forward has collected the org. -- Writes both onboarding artifacts: - - `fwd_accounts_data_.json` for Forward UI drag-and-drop import. - - `aws_create_payload_.json` for `POST /api/networks/{networkId}/cloudAccounts`. -- Added AWS Organizations access checks using `DescribeOrganization`, `ListAccounts`, and `ListParents`. -- Added optional `discover-org --post --yes` to create a new Forward AWS setup from automation. -- Kept onboarding separate from existing setup sync: `discover-org` does not PATCH existing setups. -- Added static-key onboarding support with explicit collector credential flags and placeholder protection when the secret is not supplied. -- Updated docs and Mermaid architecture diagrams for NQE sync, direct Organizations onboarding, and webhook operation. +- Hardened `apply-plan` so saved payloads cannot bypass current-state or GovCloud removal validation. +- Added a dedicated GovCloud operator guide with product-enhancement escalation criteria. ### Download and verify @@ -37,19 +28,35 @@ Assets include platform binaries, tarballs, checksums, and release attestations: ### Quick usage ```bash -# Dry run + payload review -./awssync --network-id --output aws_sync_payload.json --manual-output aws_sync_manual_payload.json - -# Apply safely in automation -./awssync --network-id --apply --yes --output aws_sync_payload.json - -# Apply an exact reviewed payload file -./awssync apply-plan --plan aws_sync_payload.json --yes - -# Discover a not-yet-onboarded AWS Organization -AWS_PROFILE=org-readonly ./awssync discover-org \ - --setup-id AWS-PROD \ - --role-name ForwardRole \ - --collect-region us-east-1 \ - --external-id Org:12345 +# Add a customer-defined External ID to an existing setup +./awssync external-id \ + --network-id \ + --setup-id \ + --value \ + --output aws_external_id_payload.json + +# Generate a GovCloud onboarding payload from a reviewed manifest +./awssync onboard-accounts \ + --accounts-file govcloud-accounts.json \ + --partition aws-us-gov \ + --credential-mode instance-profile \ + --setup-id \ + --role-name ForwardReadOnlyAccess \ + --collect-region us-gov-west-1 + +# Dry-run an existing setup against an authoritative manifest +./awssync sync-accounts \ + --network-id \ + --setup-id \ + --accounts-file govcloud-accounts.json \ + --format human + +# Verify the regular Organizations/NQE path before applying +./awssync preflight \ + --network-id \ + --setup-id \ + --max-snapshot-age 24h \ + --format human ``` + +See `docs/govcloud-workflow.md` for the complete GovCloud Organizations and standalone-account procedures. diff --git a/docs/architecture-flow.md b/docs/architecture-flow.md index fdfb8c6..8e00db2 100644 --- a/docs/architecture-flow.md +++ b/docs/architecture-flow.md @@ -9,6 +9,44 @@ GitHub renders the Mermaid diagrams below automatically. --- +## GovCloud Inventory Decision + +GovCloud resource collection and Organizations inventory are separate capabilities. Use the regular Forward snapshot/NQE path when Forward has positive GovCloud Organizations evidence. Use a complete, reviewed manifest when Organizations is unavailable or cannot be delegated. + +```mermaid +flowchart TD + start["GovCloud AWS setup\narn:aws-us-gov roles"] + snapshot["Run connectivity test\nand fresh Forward snapshot"] + org_check{"Forward NQE shows positive\nOrganizations evidence?"} + nqe["Regular preflight + NQE plan"] + manifest["Authoritative account manifest\nonboard-accounts or sync-accounts"] + removals{"Plan contains removals?"} + review["Review exact account IDs"] + approve["Explicit --allow-removals\nall removal paths"] + apply["PATCH Forward setup"] + block["BLOCK\nno empty/unproven inventory apply"] + + start --> snapshot --> org_check + org_check -- "yes" --> nqe --> removals + org_check -- "no / unavailable" --> manifest --> removals + removals -- "no" --> apply + removals -- "yes, NQE evidence present" --> review + removals -- "yes, authoritative manifest" --> review + review --> approve --> apply + removals -- "yes, NQE evidence absent" --> block + + classDef neutral fill:#F1EFE8,stroke:#5F5E5A,color:#2C2C2A; + classDef safe fill:#E1F5EE,stroke:#0F6E56,color:#04342C; + classDef warn fill:#FAEEDA,stroke:#854F0B,color:#412402; + classDef blocked fill:#FCEBEB,stroke:#A32D2D,color:#501313; + + class start,snapshot,org_check,nqe,manifest,removals,review neutral; + class approve,apply safe; + class block blocked; +``` + +--- + ## Mode 1 — Existing Setup Sync Operator or scheduler invokes `awssync` directly to update one or more existing Forward AWS setups from Forward NQE data. @@ -26,7 +64,7 @@ flowchart TB plan["plan / dry-run\nPOST /nqe + GET /cloudAccounts"] disk["payload.json\nwritten to disk before any change"] apply["--apply\nPATCH /cloudAccounts/{setupId}"] - apply_plan["apply-plan\napply pre-reviewed file from disk"] + apply_plan["apply-plan\nreload current state + validate\nGovCloud removals refused"] end subgraph fwd["Forward platform (HTTPS · Basic Auth)"] @@ -111,7 +149,51 @@ flowchart TB --- -## Mode 3 — Native IaC Onboarding With Terraform +## Mode 3 — Manual Account-Manifest Workflow + +This mode does not call AWS Organizations. The manifest is the authoritative inventory and must contain every account that should remain in the setup. `onboard-accounts` creates review artifacts for a new setup; `sync-accounts` plans or updates an existing setup. + +```mermaid +flowchart TB + manifest["Reviewed accounts.json\nunique 12-digit IDs"] + validate["Validate manifest\nand AWS partition"] + + subgraph create_path["New setup"] + onboard["onboard-accounts\ndry run by default"] + create_files["create payload + UI import JSON\narn:aws-us-gov for GovCloud"] + post["--post --yes\nPOST /cloudAccounts"] + end + + subgraph update_path["Existing setup"] + sync["sync-accounts\nGET current setup"] + diff["Print exact add/remove IDs\nwrite payload before change"] + removal{"Any removals?"} + patch["--apply --yes\nPATCH /cloudAccounts/{setupId}"] + approved["--allow-removals\nexplicit approval"] + end + + manifest --> validate + validate --> onboard --> create_files --> post + validate --> sync --> diff --> removal + removal -- "no" --> patch + removal -- "yes" --> approved --> patch + + classDef neutral fill:#F1EFE8,stroke:#5F5E5A,color:#2C2C2A; + classDef fwdnode fill:#E6F1FB,stroke:#185FA5,color:#042C53; + classDef artifact fill:#FAEEDA,stroke:#854F0B,color:#412402; + classDef safe fill:#E1F5EE,stroke:#0F6E56,color:#04342C; + + class manifest,validate,onboard,sync,diff,removal neutral; + class create_files artifact; + class post,patch fwdnode; + class approved safe; +``` + +For GovCloud, use `--partition aws-us-gov` when onboarding. Existing-setup sync derives and preserves the partition from the current role ARNs. Mixed partitions or a mismatch between role ARNs and configured regions fail before a payload can be applied. + +--- + +## Mode 4 — Native IaC Onboarding With Terraform For new AWS Organizations onboarding, the preferred automation path is the Forward Terraform provider. Terraform can prepare AWS-side roles, read AWS Organizations, fetch Forward's external ID when needed, and create or update the Forward AWS cloud setup in one plan/apply workflow. The provider supports Forward assume-role, static-key, and collector instance-profile credential models. @@ -183,7 +265,7 @@ flowchart TB --- -## Mode 4 — Webhook Daemon +## Mode 5 — Webhook Daemon `awssync serve-webhook` runs as a long-lived HTTP server. Forward calls it on each `SNAPSHOT_READY` event. Traffic direction is **inbound to awssync**. @@ -226,7 +308,7 @@ flowchart TB ## AWS Credential Modes -For existing setup sync and webhook sync, `awssync` does not connect to AWS. The following two modes describe how **Forward** connects to AWS. Both end in `sts:AssumeRole` per member account. +For existing setup sync, manifest sync, and webhook sync, `awssync` does not connect to AWS. The following three modes describe how **Forward** connects to AWS. Multi-account modes end in `sts:AssumeRole` per member account. For `discover-org`, `awssync` also uses AWS credentials locally to read AWS Organizations. Those discovery credentials are not written to Forward. Static-key Forward collection requires separate collector key material if the create payload will be posted. @@ -248,11 +330,19 @@ flowchart LR fwd_key --> member_key end + subgraph mode_profile["Collector instance-profile mode"] + collector["Self-managed Forward collector\nEC2 instance profile"] + org_profile["AWS Organizations\nmanagement / delegated acct\noptional inventory"] + member_profile["Member / standalone accounts\nForwardRole\nsts:AssumeRole"] + collector --> org_profile + collector --> member_profile + end + classDef fwdnode fill:#E6F1FB,stroke:#185FA5,color:#042C53; classDef awsnode fill:#E1F5EE,stroke:#0F6E56,color:#04342C; - class fwd_role,fwd_key fwdnode; - class org_role,member_role,org_key,member_key awsnode; + class fwd_role,fwd_key,collector fwdnode; + class org_role,member_role,org_key,member_key,org_profile,member_profile awsnode; ``` --- @@ -268,7 +358,7 @@ flowchart LR | `GET /cloudAccounts` | Read setup metadata | read cloud accounts | | `PATCH /cloudAccounts/{id}` | Write account list | write cloud accounts | | `GET /cloudAccounts/aws/assumeRole/externalId` | Fetch Forward-generated AWS external ID for onboarding | read cloud account setup metadata | -| `POST /cloudAccounts` | Create a new AWS setup from `discover-org --post` | write cloud accounts | +| `POST /cloudAccounts` | Create a new AWS setup from `discover-org --post` or `onboard-accounts --post` | write cloud accounts | | `GET /snapshots/latestProcessed` | Check snapshot age | read snapshots | | `POST /webhooks` | Register webhook | manage webhooks | @@ -280,11 +370,14 @@ flowchart LR | `organizations:ListAccounts` | Build the account list for the Forward setup | | `organizations:ListParents` | Record parent/root or OU evidence per account | -### Forward → AWS (both credential modes) +### Forward → AWS (all credential modes) | Where | Permission | Purpose | | --- | --- | --- | +| Org management / delegated account | `organizations:DescribeAccount` | resolve account metadata | | Org management / delegated account | `organizations:ListAccounts` | discover account inventory | +| Org management / delegated account | `organizations:ListRoots` | discover root organization units | +| Org management / delegated account | `organizations:ListOrganizationalUnitsForParent` | discover child organization units | | Each member account | `ForwardRole` IAM role exists | collection target | | Each member account | Trust policy allows Forward to assume role | `sts:AssumeRole` | | Each member account | Read permissions on network resources | collection | @@ -304,13 +397,19 @@ flowchart LR - Existing setup sync and webhook sync do not connect to AWS; they use Forward NQE data. - `discover-org` connects to AWS Organizations only for initial onboarding. It does not write the discovery credentials to Forward. +- `onboard-accounts` and `sync-accounts` do not connect to AWS; they use a locally supplied, explicitly reviewed account manifest. - The payload JSON is **always written to disk before any PATCH** — changes can be reviewed before or instead of applying. - `discover-org` writes both onboarding JSON files before any optional `POST /cloudAccounts`. +- `onboard-accounts` writes both onboarding JSON files before any optional `POST /cloudAccounts`. - Static-key collector secrets are only included in the create payload when explicitly supplied. Without the secret, the file contains a placeholder and is marked not POST-ready. - Removals require explicit `--allow-removals` flag; `awssync` will not silently remove accounts from a Forward setup. +- GovCloud NQE removals additionally require positive Organizations evidence. Generic no-evidence flags cannot override this gate. +- Manifest removals require an authoritative complete manifest plus `--allow-removals`. +- `apply-plan` reloads current state and refuses GovCloud removals, so a saved payload cannot bypass the source workflow's safety checks. - Webhook receiver is protected by HTTP Basic Auth with a shared secret independent of Forward user credentials. For the full operational procedure see -[AWS Account Sync Procedure](aws-account-sync-procedure.md). +[AWS Account Sync Procedure](aws-account-sync-procedure.md) and +[AWS GovCloud Account Workflow](govcloud-workflow.md).