Skip to content

Commit

Permalink
fix(cloudformation): should not throw ErrChangeSetEmpty when CFN tmp …
Browse files Browse the repository at this point in the history 
…has bad reference (aws#2270)

fixes aws#2089, fixes aws#2257

<!-- Provide summary of changes -->

<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
  • Loading branch information
@iamhopaul123 @thrau
iamhopaul123 authored and thrau committed Dec 9, 2022
1 parent c4b8052 commit 64ac9b2
Show file tree
Hide file tree
Showing 2 changed files with 209 additions and 16 deletions.
8 changes: 5 additions & 3 deletions internal/pkg/aws/cloudformation/changeset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package cloudformation
import (
"context"
"fmt"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -201,17 +202,18 @@ func (cs *changeSet) createAndExecute(conf *stackConfig) error {
if descrErr != nil {
return fmt.Errorf("check if changeset is empty: %v: %w", err, descrErr)
}
// The change set was empty - so we clean it up.
// The change set was empty - so we clean it up. The status reason will be like
// "The submitted information didn't contain changes. Submit different information to create a change set."
// We try to clean up the change set because there's a limit on the number
// of failed change sets a customer can have on a particular stack.
// See https://cloudonaut.io/aws-cli-cloudformation-deploy-limit-exceeded/.
if len(descr.Changes) == 0 {
if len(descr.Changes) == 0 && strings.Contains(descr.StatusReason, "didn't contain changes") {
_ = cs.delete()
return &ErrChangeSetEmpty{
cs: cs,
}
}
return err
return fmt.Errorf("%w: %s", err, descr.StatusReason)
}
return cs.execute()
}
Expand Down
217 changes: 204 additions & 13 deletions internal/pkg/aws/cloudformation/cloudformation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ func TestCloudFormation_WaitForCreate(t *testing.T) {
}

func TestCloudFormation_Update(t *testing.T) {
const (
mockStackName = "id"
mockChangeSetName = "copilot-31323334-3536-4738-b930-313233333435"
)
testCases := map[string]struct {
createMock func(ctrl *gomock.Controller) client
wantedErr error
Expand All @@ -262,29 +266,216 @@ func TestCloudFormation_Update(t *testing.T) {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{
{
StackStatus: aws.String(cloudformation.StackStatusUpdateInProgress),
},
},
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateInProgress)}},
}, nil)
return m
},
wantedErr: &ErrStackUpdateInProgress{
Name: mockStack.Name,
},
},
"update a previously existing stack": {
"error if fail to create the changeset because of random issue": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{
{
StackStatus: aws.String(cloudformation.StackStatusCreateComplete),
},
},
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
addUpdateDeployCalls(m)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(nil, errors.New("some error"))
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(&cloudformation.DescribeChangeSetOutput{
Changes: []*cloudformation.Change{},
StatusReason: aws.String("some other reason"),
}, nil)
return m
},
wantedErr: fmt.Errorf("create change set copilot-31323334-3536-4738-b930-313233333435 for stack id: some error: some other reason"),
},
"error if fail to wait until the changeset creation complete": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(&cloudformation.CreateChangeSetOutput{
Id: aws.String(mockChangeSetName),
}, nil)
m.EXPECT().WaitUntilChangeSetCreateCompleteWithContext(gomock.Any(), &cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
}, gomock.Any()).Return(errors.New("some error"))
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(&cloudformation.DescribeChangeSetOutput{
StatusReason: aws.String("some reason"),
}, nil)
return m
},
wantedErr: fmt.Errorf("wait for creation of change set copilot-31323334-3536-4738-b930-313233333435 for stack id: some error: some reason"),
},
"error if fail to describe change set after creation failed": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(nil, errors.New("some error"))
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(&cloudformation.DescribeChangeSetOutput{
NextToken: aws.String("mockNext"),
}, nil)
m.EXPECT().DescribeChangeSet(&cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName),
NextToken: aws.String("mockNext")}).
Return(nil, errors.New("some error"))
return m
},
wantedErr: fmt.Errorf("check if changeset is empty: create change set copilot-31323334-3536-4738-b930-313233333435 for stack id: some error: describe change set copilot-31323334-3536-4738-b930-313233333435 for stack id: some error"),
},
"delete change set and throw ErrChangeSetEmpty if failed to create the change set because it is empty": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(nil, errors.New("some error"))
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(&cloudformation.DescribeChangeSetOutput{
Changes: []*cloudformation.Change{},
StatusReason: aws.String("The submitted information didn't contain changes. Submit different information to create a change set."),
}, nil)
m.EXPECT().DeleteChangeSet(&cloudformation.DeleteChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName),
}).Return(nil, nil)
return m
},
wantedErr: fmt.Errorf("change set with name copilot-31323334-3536-4738-b930-313233333435 for stack id has no changes"),
},
"error if creation succeed but failed to describe change set": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(&cloudformation.CreateChangeSetOutput{
Id: aws.String(mockChangeSetName),
}, nil)
m.EXPECT().WaitUntilChangeSetCreateCompleteWithContext(gomock.Any(), &cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
}, gomock.Any()).Return(nil)
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(nil, errors.New("some error"))
return m
},
wantedErr: fmt.Errorf("describe change set copilot-31323334-3536-4738-b930-313233333435 for stack id: some error"),
},
"ignore execute request if the change set does not contain any modifications": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(&cloudformation.CreateChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName),
ChangeSetType: aws.String("UPDATE"),
IncludeNestedStacks: aws.Bool(true),
Capabilities: aws.StringSlice([]string{
cloudformation.CapabilityCapabilityIam,
cloudformation.CapabilityCapabilityNamedIam,
cloudformation.CapabilityCapabilityAutoExpand,
}),
TemplateBody: aws.String("template"),
}).Return(&cloudformation.CreateChangeSetOutput{
Id: aws.String(mockChangeSetName),
}, nil)
m.EXPECT().WaitUntilChangeSetCreateCompleteWithContext(gomock.Any(), &cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
}, gomock.Any()).Return(nil)
m.EXPECT().DescribeChangeSet(&cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName)}).
Return(&cloudformation.DescribeChangeSetOutput{
ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
StatusReason: aws.String(noChangesReason),
}, nil)
return m
},
},
"error if change set is not executable": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(&cloudformation.CreateChangeSetOutput{
Id: aws.String(mockChangeSetName),
}, nil)
m.EXPECT().WaitUntilChangeSetCreateCompleteWithContext(gomock.Any(), &cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
}, gomock.Any()).Return(nil)
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(&cloudformation.DescribeChangeSetOutput{
ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
StatusReason: aws.String("some other reason"),
}, nil)
return m
},
wantedErr: fmt.Errorf("execute change set copilot-31323334-3536-4738-b930-313233333435 for stack id because status is UNAVAILABLE with reason some other reason"),
},
"error if fail to execute change set": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(gomock.Any()).Return(&cloudformation.CreateChangeSetOutput{
Id: aws.String(mockChangeSetName),
}, nil)
m.EXPECT().WaitUntilChangeSetCreateCompleteWithContext(gomock.Any(), &cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
}, gomock.Any()).Return(nil)
m.EXPECT().DescribeChangeSet(gomock.Any()).
Return(&cloudformation.DescribeChangeSetOutput{
ExecutionStatus: aws.String(cloudformation.ExecutionStatusAvailable),
}, nil)
m.EXPECT().ExecuteChangeSet(gomock.Any()).Return(nil, errors.New("some error"))
return m
},
wantedErr: fmt.Errorf("execute change set copilot-31323334-3536-4738-b930-313233333435 for stack id: some error"),
},
"success": {
createMock: func(ctrl *gomock.Controller) client {
m := mocks.NewMockclient(ctrl)
m.EXPECT().DescribeStacks(gomock.Any()).Return(&cloudformation.DescribeStacksOutput{
Stacks: []*cloudformation.Stack{{StackStatus: aws.String(cloudformation.StackStatusUpdateComplete)}},
}, nil)
m.EXPECT().CreateChangeSet(&cloudformation.CreateChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName),
ChangeSetType: aws.String("UPDATE"),
IncludeNestedStacks: aws.Bool(true),
Capabilities: aws.StringSlice([]string{
cloudformation.CapabilityCapabilityIam,
cloudformation.CapabilityCapabilityNamedIam,
cloudformation.CapabilityCapabilityAutoExpand,
}),
TemplateBody: aws.String("template"),
}).Return(&cloudformation.CreateChangeSetOutput{
Id: aws.String(mockChangeSetName),
}, nil)
m.EXPECT().WaitUntilChangeSetCreateCompleteWithContext(gomock.Any(), &cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
}, gomock.Any()).Return(nil)
m.EXPECT().DescribeChangeSet(&cloudformation.DescribeChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName)}).
Return(&cloudformation.DescribeChangeSetOutput{
ExecutionStatus: aws.String(cloudformation.ExecutionStatusAvailable),
}, nil)
m.EXPECT().ExecuteChangeSet(&cloudformation.ExecuteChangeSetInput{
ChangeSetName: aws.String(mockChangeSetName),
StackName: aws.String(mockStackName),
}).Return(&cloudformation.ExecuteChangeSetOutput{}, nil)
return m
},
},
Expand All @@ -311,7 +502,7 @@ func TestCloudFormation_Update(t *testing.T) {
require.EqualError(t, err, tc.wantedErr.Error())
} else {
require.NoError(t, err)
require.Equal(t, mockChangeSetID, id)
require.Equal(t, mockChangeSetName, id)
}
})
}
Expand Down

0 comments on commit 64ac9b2

Please sign in to comment.