Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions internal/aws/cloudformation/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,19 @@ func (c *Client) CreateStack(ctx context.Context, params *CreateStackParams) (*S
input.Parameters = cfParams
}

_, err := c.cfn.CreateStack(ctx, input)
resp, err := c.cfn.CreateStack(ctx, input)
if err != nil {
return nil, wrapError(err)
}

// If no-wait is set, return immediately without waiting for completion
if params.NoWait {
return &StackOutput{
StackID: aws.ToString(resp.StackId),
Outputs: nil, // no outputs available yet
}, nil
}

// Wait for stack creation to complete
waiter := cloudformation.NewStackCreateCompleteWaiter(c.cfn)
err = waiter.Wait(ctx, &cloudformation.DescribeStacksInput{
Expand Down Expand Up @@ -84,11 +92,19 @@ func (c *Client) UpdateStack(ctx context.Context, params *UpdateStackParams) (*S
input.Parameters = cfParams
}

_, err := c.cfn.UpdateStack(ctx, input)
resp, err := c.cfn.UpdateStack(ctx, input)
if err != nil {
return nil, wrapError(err)
}

// If no-wait is set, return immediately without waiting for completion
if params.NoWait {
return &StackOutput{
StackID: aws.ToString(resp.StackId),
Outputs: nil, // no outputs available yet
}, nil
}

// Wait for stack update to complete
waiter := cloudformation.NewStackUpdateCompleteWaiter(c.cfn)
err = waiter.Wait(ctx, &cloudformation.DescribeStacksInput{
Expand All @@ -103,14 +119,19 @@ func (c *Client) UpdateStack(ctx context.Context, params *UpdateStackParams) (*S
}

// DeleteStack deletes a CloudFormation stack
func (c *Client) DeleteStack(ctx context.Context, stackName string, waitTimeout time.Duration) error {
func (c *Client) DeleteStack(ctx context.Context, stackName string, waitTimeout time.Duration, noWait bool) error {
_, err := c.cfn.DeleteStack(ctx, &cloudformation.DeleteStackInput{
StackName: aws.String(stackName),
})
if err != nil {
return wrapError(err)
}

// If no-wait is set, return immediately without waiting for deletion
if noWait {
return nil
}

// Wait for stack deletion to complete
waiter := cloudformation.NewStackDeleteCompleteWaiter(c.cfn)
err = waiter.Wait(ctx, &cloudformation.DescribeStacksInput{
Expand Down
2 changes: 2 additions & 0 deletions internal/aws/cloudformation/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type CreateStackParams struct {
Capabilities []types.Capability
Tags []types.Tag
WaitTimeout time.Duration
NoWait bool // if true, skip waiting for stack creation to complete
}

// UpdateStackParams contains parameters for updating a CloudFormation stack
Expand All @@ -24,6 +25,7 @@ type UpdateStackParams struct {
Parameters map[string]string
Capabilities []types.Capability
WaitTimeout time.Duration
NoWait bool // if true, skip waiting for stack update to complete
}

// StackOutput contains the outputs from a CloudFormation stack
Expand Down
18 changes: 13 additions & 5 deletions internal/commands/bootstrap/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type createOptions struct {
functionName string
region string
stackName string
noWait bool
}

func newCreateCommand() *cobra.Command {
Expand All @@ -49,6 +50,7 @@ Example:
cmd.Flags().StringVar(&opts.functionName, "function-name", defaultFunctionName, "Name of the Lambda function")
cmd.Flags().StringVar(&opts.region, "region", "", "AWS region (required)")
cmd.Flags().StringVar(&opts.stackName, "stack-name", defaultStackName, "Name of the CloudFormation stack")
cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack creation to complete")

_ = cmd.MarkFlagRequired("image-uri")
_ = cmd.MarkFlagRequired("region")
Expand Down Expand Up @@ -102,6 +104,7 @@ func runCreate(ctx context.Context, opts *createOptions) error {
},
},
WaitTimeout: defaultTimeout,
NoWait: opts.noWait,
}

fmt.Println("📋 Creating CloudFormation stack...")
Expand All @@ -112,11 +115,16 @@ func runCreate(ctx context.Context, opts *createOptions) error {
return fmt.Errorf("failed to create stack: %w", err)
}

fmt.Println("✅ Stack created successfully!")
fmt.Println()
fmt.Println("Outputs:")
for key, value := range output.Outputs {
fmt.Printf(" %s: %s\n", key, value)
if opts.noWait {
fmt.Println("✅ Stack creation submitted!")
fmt.Printf(" Stack ID: %s\n", output.StackID)
} else {
fmt.Println("✅ Stack created successfully!")
fmt.Println()
fmt.Println("Outputs:")
for key, value := range output.Outputs {
fmt.Printf(" %s: %s\n", key, value)
}
}

return nil
Expand Down
10 changes: 8 additions & 2 deletions internal/commands/bootstrap/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
type deleteOptions struct {
region string
stackName string
noWait bool
}

func newDeleteCommand() *cobra.Command {
Expand All @@ -34,6 +35,7 @@ Example:

cmd.Flags().StringVar(&opts.region, "region", "", "AWS region (required)")
cmd.Flags().StringVar(&opts.stackName, "stack-name", defaultStackName, "Name of the CloudFormation stack")
cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack deletion to complete")

_ = cmd.MarkFlagRequired("region")

Expand All @@ -57,12 +59,16 @@ func runDelete(ctx context.Context, opts *deleteOptions) error {
fmt.Println("📋 Deleting CloudFormation stack...")

// Delete stack
err = cfnClient.DeleteStack(ctx, opts.stackName, 10*time.Minute)
err = cfnClient.DeleteStack(ctx, opts.stackName, 10*time.Minute, opts.noWait)
if err != nil {
return fmt.Errorf("failed to delete stack: %w", err)
}

fmt.Println("✅ Stack deleted successfully!")
if opts.noWait {
fmt.Println("✅ Stack deletion submitted!")
} else {
fmt.Println("✅ Stack deleted successfully!")
}

return nil
}
52 changes: 35 additions & 17 deletions internal/commands/clusteriam/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type createOptions struct {
clusterName string
oidcIssuerURL string
region string
noWait bool
}

func newCreateCommand() *cobra.Command {
Expand Down Expand Up @@ -56,6 +57,7 @@ Examples:

cmd.Flags().StringVar(&opts.oidcIssuerURL, "oidc-issuer-url", "", "OIDC issuer URL (optional — also creates OIDC provider if provided)")
cmd.Flags().StringVar(&opts.region, "region", "", "AWS region (required)")
cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack creation to complete")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: default contradicts PR objectives and breaks reliability guarantee.

The default value true makes the command return immediately without waiting for stack creation, contradicting the PR description: "maintaining blocking behavior by default to preserve reliability." This inverts the intended behavior and will break existing workflows that depend on blocking completion.

The primary motivation is to reduce CI pipeline time, but that should be opt-in (--no-wait) rather than the default. The commit message acknowledges the default is true, which conflicts with the stated design intent in the PR objectives.

🔧 Proposed fix
-	cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack creation to complete")
+	cmd.Flags().BoolVar(&opts.noWait, "no-wait", false, "Return immediately without waiting for stack creation to complete")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack creation to complete")
cmd.Flags().BoolVar(&opts.noWait, "no-wait", false, "Return immediately without waiting for stack creation to complete")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/commands/clusteriam/create.go` at line 60, The BoolVar flag
definition for noWait in the create command has a default value of true, which
contradicts the PR objective to maintain blocking behavior by default. Change
the default value from true to false in the BoolVar call so that the command
blocks and waits for stack creation to complete by default, allowing users to
opt-in to non-blocking behavior by explicitly passing the --no-wait flag.


_ = cmd.MarkFlagRequired("region")

Expand Down Expand Up @@ -99,26 +101,34 @@ func runCreate(ctx context.Context, opts *createOptions) error {
iamReq := &clusteriam.CreateIAMRequest{
ClusterName: opts.clusterName,
OIDCIssuerDomain: oidcIssuerDomain,
NoWait: opts.noWait,
AWSConfig: cfg,
}

fmt.Printf("Creating or updating CloudFormation stack: rosa-%s-iam\n", opts.clusterName)
fmt.Println(" This may take several minutes...")
if !opts.noWait {
fmt.Println(" This may take several minutes...")
}
fmt.Println()

resp, err := clusteriam.CreateIAM(ctx, iamReq)
if err != nil {
return err
}

fmt.Println("Cluster IAM roles created successfully!")
fmt.Printf(" Stack ID: %s\n", resp.StackID)
fmt.Println()
if opts.noWait {
fmt.Println("Stack creation submitted!")
fmt.Printf(" Stack ID: %s\n", resp.StackID)
} else {
fmt.Println("Cluster IAM roles created successfully!")
fmt.Printf(" Stack ID: %s\n", resp.StackID)
fmt.Println()

if len(resp.Outputs) > 0 {
fmt.Println("Created Resources:")
for key, value := range resp.Outputs {
fmt.Printf(" %s: %s\n", key, value)
if len(resp.Outputs) > 0 {
fmt.Println("Created Resources:")
for key, value := range resp.Outputs {
fmt.Printf(" %s: %s\n", key, value)
}
}
}

Expand All @@ -131,26 +141,34 @@ func runCreate(ctx context.Context, opts *createOptions) error {
oidcReq := &clusteroidc.CreateOIDCRequest{
ClusterName: opts.clusterName,
OIDCIssuerURL: opts.oidcIssuerURL,
NoWait: opts.noWait,
AWSConfig: cfg,
}

fmt.Printf("Creating CloudFormation stack: rosa-%s-oidc\n", opts.clusterName)
fmt.Println(" This may take a few minutes...")
if !opts.noWait {
fmt.Println(" This may take a few minutes...")
}
fmt.Println()

oidcResp, err := clusteroidc.CreateOIDC(ctx, oidcReq)
if err != nil {
return fmt.Errorf("IAM roles created but OIDC provider failed: %w", err)
}

fmt.Println("Cluster OIDC provider created successfully!")
fmt.Printf(" Stack ID: %s\n", oidcResp.StackID)
fmt.Println()

if len(oidcResp.Outputs) > 0 {
fmt.Println("OIDC Resources:")
for key, value := range oidcResp.Outputs {
fmt.Printf(" %s: %s\n", key, value)
if opts.noWait {
fmt.Println("OIDC provider stack creation submitted!")
fmt.Printf(" Stack ID: %s\n", oidcResp.StackID)
} else {
fmt.Println("Cluster OIDC provider created successfully!")
fmt.Printf(" Stack ID: %s\n", oidcResp.StackID)
fmt.Println()

if len(oidcResp.Outputs) > 0 {
fmt.Println("OIDC Resources:")
for key, value := range oidcResp.Outputs {
fmt.Printf(" %s: %s\n", key, value)
}
}
}
}
Expand Down
13 changes: 11 additions & 2 deletions internal/commands/clusteriam/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
type deleteOptions struct {
clusterName string
region string
noWait bool
}

func newDeleteCommand() *cobra.Command {
Expand All @@ -34,6 +35,7 @@ Example:
}

cmd.Flags().StringVar(&opts.region, "region", "", "AWS region (required)")
cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack deletion to complete")

_ = cmd.MarkFlagRequired("region")

Expand All @@ -54,11 +56,14 @@ func runDelete(ctx context.Context, opts *deleteOptions) error {
// Create service request
req := &clusteriam.DeleteIAMRequest{
ClusterName: opts.clusterName,
NoWait: opts.noWait,
AWSConfig: cfg,
}

fmt.Printf("☁️ Deleting CloudFormation stack: rosa-%s-iam\n", opts.clusterName)
fmt.Println(" This may take several minutes...")
if !opts.noWait {
fmt.Println(" This may take several minutes...")
}
fmt.Println()

// Call service layer
Expand All @@ -67,7 +72,11 @@ func runDelete(ctx context.Context, opts *deleteOptions) error {
return err
}

fmt.Println("✅ Cluster IAM resources deleted successfully!")
if opts.noWait {
fmt.Println("✅ Stack deletion submitted!")
} else {
fmt.Println("✅ Cluster IAM resources deleted successfully!")
}

return nil
}
28 changes: 19 additions & 9 deletions internal/commands/clusteroidc/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type createOptions struct {
oidcIssuerURL string
oidcThumbprint string
region string
noWait bool
}

func newCreateCommand() *cobra.Command {
Expand Down Expand Up @@ -46,6 +47,7 @@ Example:
cmd.Flags().StringVar(&opts.oidcIssuerURL, "oidc-issuer-url", "", "OIDC issuer URL from the cluster (required)")
cmd.Flags().StringVar(&opts.oidcThumbprint, "oidc-thumbprint", "", "TLS thumbprint (optional, fetched automatically if omitted)")
cmd.Flags().StringVar(&opts.region, "region", "", "AWS region (required)")
cmd.Flags().BoolVar(&opts.noWait, "no-wait", true, "Return immediately without waiting for stack creation to complete")

_ = cmd.MarkFlagRequired("oidc-issuer-url")
_ = cmd.MarkFlagRequired("region")
Expand Down Expand Up @@ -73,26 +75,34 @@ func runCreate(ctx context.Context, opts *createOptions) error {
ClusterName: opts.clusterName,
OIDCIssuerURL: opts.oidcIssuerURL,
OIDCThumbprint: opts.oidcThumbprint,
NoWait: opts.noWait,
AWSConfig: cfg,
}

fmt.Printf("Creating CloudFormation stack: rosa-%s-oidc\n", opts.clusterName)
fmt.Println(" This may take a few minutes...")
if !opts.noWait {
fmt.Println(" This may take a few minutes...")
}
fmt.Println()

resp, err := clusteroidc.CreateOIDC(ctx, req)
if err != nil {
return err
}

fmt.Println("Cluster OIDC provider created successfully!")
fmt.Printf(" Stack ID: %s\n", resp.StackID)
fmt.Println()

if len(resp.Outputs) > 0 {
fmt.Println("Created Resources:")
for key, value := range resp.Outputs {
fmt.Printf(" %s: %s\n", key, value)
if opts.noWait {
fmt.Println("Stack creation submitted!")
fmt.Printf(" Stack ID: %s\n", resp.StackID)
} else {
fmt.Println("Cluster OIDC provider created successfully!")
fmt.Printf(" Stack ID: %s\n", resp.StackID)
fmt.Println()

if len(resp.Outputs) > 0 {
fmt.Println("Created Resources:")
for key, value := range resp.Outputs {
fmt.Printf(" %s: %s\n", key, value)
}
}
}

Expand Down
Loading