From 2ae51c1831f79af864abf380cb20701df6457f9f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 19:59:47 +0200 Subject: [PATCH 1/5] refactor(cmd): split Azure subscription listing out of configure_azure.go Pure move of newAzureWizardCredential and listAzureSubscriptions into configure_azure_subs.go. No behaviour change. Part of #1834. Co-Authored-By: Claude Opus 5.5 --- cmd/configure_azure.go | 60 -------------------------------- cmd/configure_azure_subs.go | 69 +++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 60 deletions(-) create mode 100644 cmd/configure_azure_subs.go diff --git a/cmd/configure_azure.go b/cmd/configure_azure.go index 69f17990b..fb4910c6c 100644 --- a/cmd/configure_azure.go +++ b/cmd/configure_azure.go @@ -15,8 +15,6 @@ import ( "time" - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions" "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" @@ -337,64 +335,6 @@ func promptForAzureCredentialFields(reader *bufio.Reader, creds *AzureCredential return nil } -// newAzureWizardCredential builds the credential used by the interactive Azure -// setup wizard. It binds explicitly to the Azure CLI session (the "az login" -// the operator runs in Step 1) via AzureCLICredential rather than -// DefaultAzureCredential, whose chain prioritizes environment / workload / -// managed-identity credentials and could otherwise resolve to a different -// principal than the one the operator just signed in as. -func newAzureWizardCredential() (azcore.TokenCredential, error) { - cred, err := azidentity.NewAzureCLICredential(nil) - if err != nil { - return nil, fmt.Errorf("failed to build Azure CLI credential: %w\n"+ - "Ensure you are authenticated: run 'az login' (Step 1) before continuing", err) - } - return cred, nil -} - -// listAzureSubscriptions retrieves the operator's subscriptions via the ARM -// Subscriptions SDK and prints them in a table matching "az account list" -// output. It uses the Azure CLI credential so the listing matches the -// operator's active "az login" session. -func listAzureSubscriptions(ctx context.Context) error { - cred, err := newAzureWizardCredential() - if err != nil { - return err - } - - client, err := armsubscriptions.NewClient(cred, nil) - if err != nil { - return fmt.Errorf("failed to create subscriptions client: %w", err) - } - - fmt.Printf("%-40s %-38s %s\n", "Name", "SubscriptionId", "State") - fmt.Println(strings.Repeat("-", 95)) - - pager := client.NewListPager(nil) - for pager.More() { - page, pageErr := pager.NextPage(ctx) - if pageErr != nil { - return fmt.Errorf("failed to list subscriptions: %w", pageErr) - } - for _, sub := range page.Value { - name := "" - subID := "" - state := "" - if sub.DisplayName != nil { - name = *sub.DisplayName - } - if sub.SubscriptionID != nil { - subID = *sub.SubscriptionID - } - if sub.State != nil { - state = string(*sub.State) - } - fmt.Printf("%-40s %-38s %s\n", name, subID, state) - } - } - return nil -} - // runAzureSetupCommands guides the operator through the Azure setup wizard. // // Step 1 (az login): performed via the Azure CLI. "az login" launches an diff --git a/cmd/configure_azure_subs.go b/cmd/configure_azure_subs.go new file mode 100644 index 000000000..16fa7330e --- /dev/null +++ b/cmd/configure_azure_subs.go @@ -0,0 +1,69 @@ +package main + +import ( + "context" + "fmt" + "strings" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions" +) + +// newAzureWizardCredential builds the credential used by the interactive Azure +// setup wizard. It binds explicitly to the Azure CLI session (the "az login" +// the operator runs in Step 1) via AzureCLICredential rather than +// DefaultAzureCredential, whose chain prioritizes environment / workload / +// managed-identity credentials and could otherwise resolve to a different +// principal than the one the operator just signed in as. +func newAzureWizardCredential() (azcore.TokenCredential, error) { + cred, err := azidentity.NewAzureCLICredential(nil) + if err != nil { + return nil, fmt.Errorf("failed to build Azure CLI credential: %w\n"+ + "Ensure you are authenticated: run 'az login' (Step 1) before continuing", err) + } + return cred, nil +} + +// listAzureSubscriptions retrieves the operator's subscriptions via the ARM +// Subscriptions SDK and prints them in a table matching "az account list" +// output. It uses the Azure CLI credential so the listing matches the +// operator's active "az login" session. +func listAzureSubscriptions(ctx context.Context) error { + cred, err := newAzureWizardCredential() + if err != nil { + return err + } + + client, err := armsubscriptions.NewClient(cred, nil) + if err != nil { + return fmt.Errorf("failed to create subscriptions client: %w", err) + } + + fmt.Printf("%-40s %-38s %s\n", "Name", "SubscriptionId", "State") + fmt.Println(strings.Repeat("-", 95)) + + pager := client.NewListPager(nil) + for pager.More() { + page, pageErr := pager.NextPage(ctx) + if pageErr != nil { + return fmt.Errorf("failed to list subscriptions: %w", pageErr) + } + for _, sub := range page.Value { + name := "" + subID := "" + state := "" + if sub.DisplayName != nil { + name = *sub.DisplayName + } + if sub.SubscriptionID != nil { + subID = *sub.SubscriptionID + } + if sub.State != nil { + state = string(*sub.State) + } + fmt.Printf("%-40s %-38s %s\n", name, subID, state) + } + } + return nil +} From a4c56dc68ddb6cd93352d6837314674b00834e0c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 20:00:03 +0200 Subject: [PATCH 2/5] refactor(cmd): split Azure setup flow out of configure_azure.go Pure move of runAzureSetupCommands and the login, subscription and tenant steps into configure_azure_setup.go. No behaviour change. Part of #1834. Co-Authored-By: Claude Opus 5.5 --- cmd/configure_azure.go | 110 -------------------------------- cmd/configure_azure_setup.go | 120 +++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 110 deletions(-) create mode 100644 cmd/configure_azure_setup.go diff --git a/cmd/configure_azure.go b/cmd/configure_azure.go index fb4910c6c..e6a308f17 100644 --- a/cmd/configure_azure.go +++ b/cmd/configure_azure.go @@ -15,7 +15,6 @@ import ( "time" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions" "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/secretsmanager" @@ -335,115 +334,6 @@ func promptForAzureCredentialFields(reader *bufio.Reader, creds *AzureCredential return nil } -// runAzureSetupCommands guides the operator through the Azure setup wizard. -// -// Step 1 (az login): performed via the Azure CLI. "az login" launches an -// interactive browser-based OAuth flow that cannot be replicated through the -// SDK on behalf of a human operator who does not yet have a credential. This -// is the only CLI call retained in this wizard. -// -// Step 2 (list subscriptions): performed via the ARM Subscriptions SDK using -// the Azure CLI credential, which reuses the session that "az login" just -// established. Fails loud if the SDK cannot authenticate (no CLI fallback). -// -// Step 3 (create service principal): performed via the Microsoft Graph SDK -// (application + service principal + password credential) and armauthorization -// (resolve the "Reservations Administrator" role definition and assign it at -// subscription scope). This is the create-for-rbac equivalent and fails loud -// on any SDK error. -// -// It returns the created service principal's credentials (nil when Step 3 was -// skipped) so the caller can store them without echoing the secret. -func runAzureSetupCommands(ctx context.Context, reader *bufio.Reader) (*AzureCredentials, error) { - if err := azureStepLogin(reader); err != nil { - return nil, err - } - - subscriptionID, err := azureStepListSubscriptions(ctx, reader) - if err != nil { - return nil, err - } - - return azureStepCreateServicePrincipal(ctx, reader, subscriptionID) -} - -// azureStepLogin prompts to run "az login". -func azureStepLogin(reader *bufio.Reader) error { - fmt.Println("Step 1: Azure Login") - fmt.Println("-------------------") - fmt.Println("This will open a browser window for Azure authentication.") - fmt.Println() - return promptAndRunExplicitCommand(reader, "Azure Login", "az login", "az", "login") -} - -// listAzureSubscriptionsWithTimeout runs the SDK subscription listing under a -// bounded timeout so a hung ARM call cannot stall the wizard. -func listAzureSubscriptionsWithTimeout(ctx context.Context) error { - listCtx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - return listAzureSubscriptions(listCtx) -} - -// azureStepListSubscriptions optionally lists subscriptions via SDK and prompts -// the operator to enter their subscription ID. The listing is behind a -// [R]un/[S]kip prompt so an operator who already knows their subscription ID -// can proceed even if the SDK listing would fail; when RUN it fails loud (no -// CLI fallback), instructing the operator to run "az login" first. -func azureStepListSubscriptions(ctx context.Context, reader *bufio.Reader) (string, error) { - fmt.Println() - fmt.Println("Step 2: Get Subscription ID") - fmt.Println("---------------------------") - - run, err := promptRunOrSkipListing(reader, "the Azure subscription listing (via SDK)") - if err != nil { - return "", err - } - if run { - fmt.Println("Listing your Azure subscriptions via SDK (Azure CLI credential)...") - fmt.Println() - if err = listAzureSubscriptionsWithTimeout(ctx); err != nil { - return "", fmt.Errorf("failed to list Azure subscriptions via SDK: %w\n"+ - "Ensure you are authenticated: run 'az login' (Step 1) before continuing", err) - } - fmt.Println() - } - - fmt.Print("Enter your Subscription ID: ") - subscriptionID, err := readTrimmedLine(reader) - if err != nil { - return "", fmt.Errorf("failed to read subscription ID: %w", err) - } - - if subscriptionID == "" { - return "", fmt.Errorf("subscription ID is required") - } - if err := validateAzureUUID(subscriptionID, "Subscription ID"); err != nil { - return "", err - } - return subscriptionID, nil -} - -// resolveAzureTenantID looks up the tenant ID for the given subscription via -// the ARM Subscriptions SDK, using the Azure CLI ("az login") credential. -func resolveAzureTenantID(ctx context.Context, subscriptionID string) (string, error) { - cred, err := newAzureWizardCredential() - if err != nil { - return "", err - } - client, err := armsubscriptions.NewClient(cred, nil) - if err != nil { - return "", fmt.Errorf("failed to create subscriptions client: %w", err) - } - resp, err := client.Get(ctx, subscriptionID, nil) - if err != nil { - return "", fmt.Errorf("failed to get subscription %s: %w", subscriptionID, err) - } - if resp.TenantID == nil || *resp.TenantID == "" { - return "", fmt.Errorf("subscription %s returned no tenant ID", subscriptionID) - } - return *resp.TenantID, nil -} - // resolveAzureTenantIDFn and newAzureSPProvisionerFn are the cloud-facing // constructors used by azureStepCreateServicePrincipal. They are variables so // unit tests can substitute fakes and never reach Azure. diff --git a/cmd/configure_azure_setup.go b/cmd/configure_azure_setup.go new file mode 100644 index 000000000..9c25e8f51 --- /dev/null +++ b/cmd/configure_azure_setup.go @@ -0,0 +1,120 @@ +package main + +import ( + "bufio" + "context" + "fmt" + + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions" +) + +// runAzureSetupCommands guides the operator through the Azure setup wizard. +// +// Step 1 (az login): performed via the Azure CLI. "az login" launches an +// interactive browser-based OAuth flow that cannot be replicated through the +// SDK on behalf of a human operator who does not yet have a credential. This +// is the only CLI call retained in this wizard. +// +// Step 2 (list subscriptions): performed via the ARM Subscriptions SDK using +// the Azure CLI credential, which reuses the session that "az login" just +// established. Fails loud if the SDK cannot authenticate (no CLI fallback). +// +// Step 3 (create service principal): performed via the Microsoft Graph SDK +// (application + service principal + password credential) and armauthorization +// (resolve the "Reservations Administrator" role definition and assign it at +// subscription scope). This is the create-for-rbac equivalent and fails loud +// on any SDK error. +// +// It returns the created service principal's credentials (nil when Step 3 was +// skipped) so the caller can store them without echoing the secret. +func runAzureSetupCommands(ctx context.Context, reader *bufio.Reader) (*AzureCredentials, error) { + if err := azureStepLogin(reader); err != nil { + return nil, err + } + + subscriptionID, err := azureStepListSubscriptions(ctx, reader) + if err != nil { + return nil, err + } + + return azureStepCreateServicePrincipal(ctx, reader, subscriptionID) +} + +// azureStepLogin prompts to run "az login". +func azureStepLogin(reader *bufio.Reader) error { + fmt.Println("Step 1: Azure Login") + fmt.Println("-------------------") + fmt.Println("This will open a browser window for Azure authentication.") + fmt.Println() + return promptAndRunExplicitCommand(reader, "Azure Login", "az login", "az", "login") +} + +// listAzureSubscriptionsWithTimeout runs the SDK subscription listing under a +// bounded timeout so a hung ARM call cannot stall the wizard. +func listAzureSubscriptionsWithTimeout(ctx context.Context) error { + listCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + return listAzureSubscriptions(listCtx) +} + +// azureStepListSubscriptions optionally lists subscriptions via SDK and prompts +// the operator to enter their subscription ID. The listing is behind a +// [R]un/[S]kip prompt so an operator who already knows their subscription ID +// can proceed even if the SDK listing would fail; when RUN it fails loud (no +// CLI fallback), instructing the operator to run "az login" first. +func azureStepListSubscriptions(ctx context.Context, reader *bufio.Reader) (string, error) { + fmt.Println() + fmt.Println("Step 2: Get Subscription ID") + fmt.Println("---------------------------") + + run, err := promptRunOrSkipListing(reader, "the Azure subscription listing (via SDK)") + if err != nil { + return "", err + } + if run { + fmt.Println("Listing your Azure subscriptions via SDK (Azure CLI credential)...") + fmt.Println() + if err = listAzureSubscriptionsWithTimeout(ctx); err != nil { + return "", fmt.Errorf("failed to list Azure subscriptions via SDK: %w\n"+ + "Ensure you are authenticated: run 'az login' (Step 1) before continuing", err) + } + fmt.Println() + } + + fmt.Print("Enter your Subscription ID: ") + subscriptionID, err := readTrimmedLine(reader) + if err != nil { + return "", fmt.Errorf("failed to read subscription ID: %w", err) + } + + if subscriptionID == "" { + return "", fmt.Errorf("subscription ID is required") + } + if err := validateAzureUUID(subscriptionID, "Subscription ID"); err != nil { + return "", err + } + return subscriptionID, nil +} + +// resolveAzureTenantID looks up the tenant ID for the given subscription via +// the ARM Subscriptions SDK, using the Azure CLI ("az login") credential. +func resolveAzureTenantID(ctx context.Context, subscriptionID string) (string, error) { + cred, err := newAzureWizardCredential() + if err != nil { + return "", err + } + client, err := armsubscriptions.NewClient(cred, nil) + if err != nil { + return "", fmt.Errorf("failed to create subscriptions client: %w", err) + } + resp, err := client.Get(ctx, subscriptionID, nil) + if err != nil { + return "", fmt.Errorf("failed to get subscription %s: %w", subscriptionID, err) + } + if resp.TenantID == nil || *resp.TenantID == "" { + return "", fmt.Errorf("subscription %s returned no tenant ID", subscriptionID) + } + return *resp.TenantID, nil +} From b561dd6e121ca0512a88d82678c4bd09ae9f419f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 20:00:17 +0200 Subject: [PATCH 3/5] refactor(cmd): split Azure service-principal step out of configure_azure.go Pure move of azureStepCreateServicePrincipal and its result and recovery-hint helpers into configure_azure_sp_step.go. No behaviour change. Part of #1834. Co-Authored-By: Claude Opus 5.5 --- cmd/configure_azure.go | 114 ------------------------------ cmd/configure_azure_sp_step.go | 124 +++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 114 deletions(-) create mode 100644 cmd/configure_azure_sp_step.go diff --git a/cmd/configure_azure.go b/cmd/configure_azure.go index e6a308f17..1b0cf84b2 100644 --- a/cmd/configure_azure.go +++ b/cmd/configure_azure.go @@ -334,120 +334,6 @@ func promptForAzureCredentialFields(reader *bufio.Reader, creds *AzureCredential return nil } -// resolveAzureTenantIDFn and newAzureSPProvisionerFn are the cloud-facing -// constructors used by azureStepCreateServicePrincipal. They are variables so -// unit tests can substitute fakes and never reach Azure. -var ( - resolveAzureTenantIDFn = resolveAzureTenantID - newAzureSPProvisionerFn = func(subscriptionID string) (azureSPProvisioner, error) { - return newGraphSPProvisioner(subscriptionID) - } -) - -// azureStepCreateServicePrincipal creates the service principal via the -// Microsoft Graph + armauthorization SDKs (the create-for-rbac equivalent) and -// returns its credentials for direct storage (nil when the operator skips the -// step). The client secret is never printed. It fails loud: any SDK error is -// returned rather than silently falling back to the CLI. -func azureStepCreateServicePrincipal(ctx context.Context, reader *bufio.Reader, subscriptionID string) (*AzureCredentials, error) { - fmt.Println() - fmt.Println("Step 3: Create Service Principal") - fmt.Println("---------------------------------") - fmt.Println("This creates an Azure Service Principal with the") - fmt.Printf("%q role at subscription scope, via the Microsoft Graph SDK.\n", azureSPRoleName) - fmt.Println() - fmt.Printf("Create service principal %q with role %q at /subscriptions/%s?\n", azureSPName, azureSPRoleName, subscriptionID) - fmt.Printf("[r]un, [S]kip (default)? ") - - choice, err := readTrimmedLine(reader) - if err != nil { - return nil, fmt.Errorf("failed to read service-principal choice: %w", err) - } - // Creating an Entra app, password and role assignment is not harmless, so - // Enter skips, like the GCP wizard. - choice = strings.ToLower(choice) - if choice != "r" && choice != "run" { - fmt.Println("Skipping Create Service Principal") - fmt.Println() - fmt.Println("Provide the appId, client secret and tenant ID for an existing") - fmt.Println("service principal in the next step.") - fmt.Println() - return nil, nil - } - - // The step budget must exceed roleAssignRetryBudget (3 min) plus the - // pre-assignment overhead (tenant resolution + the application / password / - // service-principal / role-definition Graph calls) so the PrincipalNotFound - // retry loop in AssignRole gets its full propagation budget. A tighter - // budget would cut the retry short via ctx cancellation, then the rollback - // would delete the just-created application and reset the AAD replication - // clock on every re-run. See roleAssignRetryBudget in configure_azure_sp.go. - spCtx, cancel := context.WithTimeout(ctx, 6*time.Minute) - defer cancel() - - tenantID, err := resolveAzureTenantIDFn(spCtx, subscriptionID) - if err != nil { - return nil, err - } - - provisioner, err := newAzureSPProvisionerFn(subscriptionID) - if err != nil { - return nil, fmt.Errorf("failed to initialize Azure SDK clients: %w\n"+ - "Ensure you are authenticated: run 'az login' (Step 1) before continuing", err) - } - - fmt.Println() - fmt.Println(strings.Repeat("-", 60)) - result, err := createAzureServicePrincipal(spCtx, provisioner, subscriptionID, tenantID) - if err != nil { - return nil, err - } - fmt.Println(strings.Repeat("-", 60)) - - printAzureSPResult(result, subscriptionID, azureOpts.StackName) - return &AzureCredentials{ - TenantID: result.TenantID, - ClientID: result.AppID, - ClientSecret: result.ClientSecret, - SubscriptionID: subscriptionID, - }, nil -} - -// printAzureSPResult prints the non-secret identifiers of the created service -// principal. The client secret is deliberately NOT printed (not even -// partially): stdout is routinely captured by script/tee, CI logs and terminal -// scrollback, and the wizard hands the secret straight to the Secrets Manager -// store step, so the operator never needs to see or retype it. -func printAzureSPResult(result azureSPResult, subscriptionID, stackName string) { - fmt.Println() - fmt.Println("Service principal created:") - fmt.Println() - fmt.Printf(" appId (Client ID): %s\n", result.AppID) - fmt.Println(" password (Client Secret): [hidden]") - fmt.Printf(" tenant (Tenant ID): %s\n", result.TenantID) - fmt.Printf(" Subscription ID: %s\n", subscriptionID) - fmt.Println() - fmt.Printf("The client secret is not displayed. It will be stored directly in the\n") - fmt.Printf("AWS Secrets Manager secret %q (field client_secret).\n", stackName+"-AzureCredentials") - fmt.Println() -} - -// withAzureSPRecoveryHint annotates a failure that happens after the wizard -// created a service principal but before its secret was stored (AWS config -// and credentials are checked before creation, so this covers the remaining -// store-time failures such as ListSecrets/UpdateSecret errors). The secret was -// never displayed, so the operator is told how to mint a replacement instead. -func withAzureSPRecoveryHint(err error, created *AzureCredentials) error { - if created == nil { - return err - } - return fmt.Errorf("%w\nThe service principal (appId %s) was created but its client secret was NOT stored "+ - "and was never displayed. Fix the error above, then generate a new secret with "+ - "'az ad app credential reset --id %s' and re-run 'cudly configure-azure --skip-setup', "+ - "entering it at the hidden Client Secret prompt", - err, created.ClientID, created.ClientID) -} - // promptAndRunExplicitCommand shows a command and asks to run or skip. // Takes explicit program and args to avoid command injection via string splitting. func promptAndRunExplicitCommand(reader *bufio.Reader, name, displayCmd, program string, args ...string) error { diff --git a/cmd/configure_azure_sp_step.go b/cmd/configure_azure_sp_step.go new file mode 100644 index 000000000..b9b84989d --- /dev/null +++ b/cmd/configure_azure_sp_step.go @@ -0,0 +1,124 @@ +package main + +import ( + "bufio" + "context" + "fmt" + "strings" + + "time" +) + +// resolveAzureTenantIDFn and newAzureSPProvisionerFn are the cloud-facing +// constructors used by azureStepCreateServicePrincipal. They are variables so +// unit tests can substitute fakes and never reach Azure. +var ( + resolveAzureTenantIDFn = resolveAzureTenantID + newAzureSPProvisionerFn = func(subscriptionID string) (azureSPProvisioner, error) { + return newGraphSPProvisioner(subscriptionID) + } +) + +// azureStepCreateServicePrincipal creates the service principal via the +// Microsoft Graph + armauthorization SDKs (the create-for-rbac equivalent) and +// returns its credentials for direct storage (nil when the operator skips the +// step). The client secret is never printed. It fails loud: any SDK error is +// returned rather than silently falling back to the CLI. +func azureStepCreateServicePrincipal(ctx context.Context, reader *bufio.Reader, subscriptionID string) (*AzureCredentials, error) { + fmt.Println() + fmt.Println("Step 3: Create Service Principal") + fmt.Println("---------------------------------") + fmt.Println("This creates an Azure Service Principal with the") + fmt.Printf("%q role at subscription scope, via the Microsoft Graph SDK.\n", azureSPRoleName) + fmt.Println() + fmt.Printf("Create service principal %q with role %q at /subscriptions/%s?\n", azureSPName, azureSPRoleName, subscriptionID) + fmt.Printf("[r]un, [S]kip (default)? ") + + choice, err := readTrimmedLine(reader) + if err != nil { + return nil, fmt.Errorf("failed to read service-principal choice: %w", err) + } + // Creating an Entra app, password and role assignment is not harmless, so + // Enter skips, like the GCP wizard. + choice = strings.ToLower(choice) + if choice != "r" && choice != "run" { + fmt.Println("Skipping Create Service Principal") + fmt.Println() + fmt.Println("Provide the appId, client secret and tenant ID for an existing") + fmt.Println("service principal in the next step.") + fmt.Println() + return nil, nil + } + + // The step budget must exceed roleAssignRetryBudget (3 min) plus the + // pre-assignment overhead (tenant resolution + the application / password / + // service-principal / role-definition Graph calls) so the PrincipalNotFound + // retry loop in AssignRole gets its full propagation budget. A tighter + // budget would cut the retry short via ctx cancellation, then the rollback + // would delete the just-created application and reset the AAD replication + // clock on every re-run. See roleAssignRetryBudget in configure_azure_sp.go. + spCtx, cancel := context.WithTimeout(ctx, 6*time.Minute) + defer cancel() + + tenantID, err := resolveAzureTenantIDFn(spCtx, subscriptionID) + if err != nil { + return nil, err + } + + provisioner, err := newAzureSPProvisionerFn(subscriptionID) + if err != nil { + return nil, fmt.Errorf("failed to initialize Azure SDK clients: %w\n"+ + "Ensure you are authenticated: run 'az login' (Step 1) before continuing", err) + } + + fmt.Println() + fmt.Println(strings.Repeat("-", 60)) + result, err := createAzureServicePrincipal(spCtx, provisioner, subscriptionID, tenantID) + if err != nil { + return nil, err + } + fmt.Println(strings.Repeat("-", 60)) + + printAzureSPResult(result, subscriptionID, azureOpts.StackName) + return &AzureCredentials{ + TenantID: result.TenantID, + ClientID: result.AppID, + ClientSecret: result.ClientSecret, + SubscriptionID: subscriptionID, + }, nil +} + +// printAzureSPResult prints the non-secret identifiers of the created service +// principal. The client secret is deliberately NOT printed (not even +// partially): stdout is routinely captured by script/tee, CI logs and terminal +// scrollback, and the wizard hands the secret straight to the Secrets Manager +// store step, so the operator never needs to see or retype it. +func printAzureSPResult(result azureSPResult, subscriptionID, stackName string) { + fmt.Println() + fmt.Println("Service principal created:") + fmt.Println() + fmt.Printf(" appId (Client ID): %s\n", result.AppID) + fmt.Println(" password (Client Secret): [hidden]") + fmt.Printf(" tenant (Tenant ID): %s\n", result.TenantID) + fmt.Printf(" Subscription ID: %s\n", subscriptionID) + fmt.Println() + fmt.Printf("The client secret is not displayed. It will be stored directly in the\n") + fmt.Printf("AWS Secrets Manager secret %q (field client_secret).\n", stackName+"-AzureCredentials") + fmt.Println() +} + +// withAzureSPRecoveryHint annotates a failure that happens after the wizard +// created a service principal but before its secret was stored (AWS config +// and credentials are checked before creation, so this covers the remaining +// store-time failures such as ListSecrets/UpdateSecret errors). The secret was +// never displayed, so the operator is told how to mint a replacement instead. +func withAzureSPRecoveryHint(err error, created *AzureCredentials) error { + if created == nil { + return err + } + return fmt.Errorf("%w\nThe service principal (appId %s) was created but its client secret was NOT stored "+ + "and was never displayed. Fix the error above, then generate a new secret with "+ + "'az ad app credential reset --id %s' and re-run 'cudly configure-azure --skip-setup', "+ + "entering it at the hidden Client Secret prompt", + err, created.ClientID, created.ClientID) +} From eb6fb78ca05941ad12aebb994cd036dc981f64b3 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 20:00:32 +0200 Subject: [PATCH 4/5] refactor(cmd): split Azure command prompt helpers out of configure_azure.go Pure move of promptAndRunExplicitCommand and executeExplicitCommand into configure_azure_exec.go. No behaviour change. Part of #1834. Co-Authored-By: Claude Opus 5.5 --- cmd/configure_azure.go | 62 -------------------------------- cmd/configure_azure_exec.go | 70 +++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 62 deletions(-) create mode 100644 cmd/configure_azure_exec.go diff --git a/cmd/configure_azure.go b/cmd/configure_azure.go index 1b0cf84b2..cda526579 100644 --- a/cmd/configure_azure.go +++ b/cmd/configure_azure.go @@ -9,7 +9,6 @@ import ( "io" "log" "os" - "os/exec" "regexp" "strings" @@ -333,64 +332,3 @@ func promptForAzureCredentialFields(reader *bufio.Reader, creds *AzureCredential return nil } - -// promptAndRunExplicitCommand shows a command and asks to run or skip. -// Takes explicit program and args to avoid command injection via string splitting. -func promptAndRunExplicitCommand(reader *bufio.Reader, name, displayCmd, program string, args ...string) error { - fmt.Printf("Command: %s\n", displayCmd) - fmt.Println() - fmt.Printf("[R]un, [S]kip? ") - - choice, err := readTrimmedLine(reader) - if err != nil { - return fmt.Errorf("failed to read choice: %w", err) - } - choice = strings.ToLower(choice) - - switch choice { - case "r", "run", "": - return executeExplicitCommand(reader, displayCmd, program, args...) - case "s", "skip": - fmt.Printf("Skipping %s\n", name) - return nil - default: - fmt.Printf("Unknown option '%s', skipping\n", choice) - return nil - } -} - -// executeExplicitCommand runs a command with explicit program and arguments. -// It is used only for the interactive "az login" auth bootstrap (Step 1), -// which has no SDK equivalent that preserves the cached-credential UX. -// The caller's reader is threaded through to the retry prompt so all input -// is consumed from one consistent buffered stream (a fresh -// bufio.NewReader(os.Stdin) here would drop input already buffered by the -// caller's reader, breaking piped input after earlier prompts). -func executeExplicitCommand(reader *bufio.Reader, displayCmd, program string, args ...string) error { - fmt.Println() - fmt.Printf("Executing: %s\n", displayCmd) - fmt.Println(strings.Repeat("-", 60)) - - // #nosec G204 -- interactive operator auth (az login): program and args are hardcoded literals from the caller (runAzureSetupCommands passes "az","login"), no shell, not attacker-controlled - cmd := exec.Command(program, args...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin - - err := cmd.Run() - fmt.Println(strings.Repeat("-", 60)) - - if err != nil { - fmt.Printf("Command failed: %v\n", err) - fmt.Print("Continue anyway? [y/N]: ") - response, readErr := readTrimmedLine(reader) - if readErr != nil { - return fmt.Errorf("failed to read response: %w", readErr) - } - if !strings.EqualFold(response, "y") { - return fmt.Errorf("command failed: %w", err) - } - } - - return nil -} diff --git a/cmd/configure_azure_exec.go b/cmd/configure_azure_exec.go new file mode 100644 index 000000000..482b14b08 --- /dev/null +++ b/cmd/configure_azure_exec.go @@ -0,0 +1,70 @@ +package main + +import ( + "bufio" + "fmt" + "os" + "os/exec" + "strings" +) + +// promptAndRunExplicitCommand shows a command and asks to run or skip. +// Takes explicit program and args to avoid command injection via string splitting. +func promptAndRunExplicitCommand(reader *bufio.Reader, name, displayCmd, program string, args ...string) error { + fmt.Printf("Command: %s\n", displayCmd) + fmt.Println() + fmt.Printf("[R]un, [S]kip? ") + + choice, err := readTrimmedLine(reader) + if err != nil { + return fmt.Errorf("failed to read choice: %w", err) + } + choice = strings.ToLower(choice) + + switch choice { + case "r", "run", "": + return executeExplicitCommand(reader, displayCmd, program, args...) + case "s", "skip": + fmt.Printf("Skipping %s\n", name) + return nil + default: + fmt.Printf("Unknown option '%s', skipping\n", choice) + return nil + } +} + +// executeExplicitCommand runs a command with explicit program and arguments. +// It is used only for the interactive "az login" auth bootstrap (Step 1), +// which has no SDK equivalent that preserves the cached-credential UX. +// The caller's reader is threaded through to the retry prompt so all input +// is consumed from one consistent buffered stream (a fresh +// bufio.NewReader(os.Stdin) here would drop input already buffered by the +// caller's reader, breaking piped input after earlier prompts). +func executeExplicitCommand(reader *bufio.Reader, displayCmd, program string, args ...string) error { + fmt.Println() + fmt.Printf("Executing: %s\n", displayCmd) + fmt.Println(strings.Repeat("-", 60)) + + // #nosec G204 -- interactive operator auth (az login): program and args are hardcoded literals from the caller (runAzureSetupCommands passes "az","login"), no shell, not attacker-controlled + cmd := exec.Command(program, args...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Stdin = os.Stdin + + err := cmd.Run() + fmt.Println(strings.Repeat("-", 60)) + + if err != nil { + fmt.Printf("Command failed: %v\n", err) + fmt.Print("Continue anyway? [y/N]: ") + response, readErr := readTrimmedLine(reader) + if readErr != nil { + return fmt.Errorf("failed to read response: %w", readErr) + } + if !strings.EqualFold(response, "y") { + return fmt.Errorf("command failed: %w", err) + } + } + + return nil +} From 73f2d550ddad0f0c084508f863eeaa4edc8b9dc2 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 20:30:38 +0200 Subject: [PATCH 5/5] fix(lint): cover split configure files in the noctx exclusion The moved exec.Command calls live in configure_{gcp,azure}_{exec,setup}.go, so widen the per-file noctx exclusion to match. Part of #1834. Co-Authored-By: Claude Opus 5.5 --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 0819b9791..5a6e5bd64 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -113,7 +113,7 @@ linters: # Suppressed per-file rather than per-line so #nosec G204 can lead the comment. - linters: - noctx - path: (cmd/configure_gcp|cmd/configure_azure)\.go + path: cmd/configure_(gcp|azure)(_exec|_setup)?\.go # noctx: http.Get and net.Listen in test helpers appropriately use the # background context; threading a test context through every helper would # add noise without improving test correctness.