Skip to content

Commit 22f6b76

Browse files
authored
fix(app): AWS::Partition should be rendered correctly in app stack (#2567)
<!-- Provide summary of changes --> 1. fixes #2554 2. put one of the environment in domain e2e test in another account to make sure multi-account scenario is expected 3. remove the previous app template version since we never rollback <!-- Issue number, if available. E.g. "Fixes #31", "Addresses #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.
1 parent fdf6a3c commit 22f6b76

File tree

14 files changed

+51
-654
lines changed

14 files changed

+51
-654
lines changed

e2e/app-with-domain/app_with_domain_suite_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ import (
1515

1616
var cli *client.CLI
1717
var appName string
18+
var prodEnvironmentProfile string
1819

1920
func TestAppWithDomain(t *testing.T) {
2021
RegisterFailHandler(Fail)
2122
RunSpecs(t, "Multiple Svc Suite (one workspace)")
2223
}
2324

2425
var _ = BeforeSuite(func() {
26+
prodEnvironmentProfile = "e2eprodenv"
2527
copilotCLI, err := client.NewCLI()
2628
cli = copilotCLI
2729
Expect(err).NotTo(HaveOccurred())

e2e/app-with-domain/app_with_domain_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ var _ = Describe("App With Domain", func() {
6363
_, envInitErr = cli.EnvInit(&client.EnvInitRequest{
6464
AppName: appName,
6565
EnvName: "prod",
66-
Profile: "default",
66+
Profile: prodEnvironmentProfile,
6767
Prod: false,
6868
})
6969
Expect(envInitErr).NotTo(HaveOccurred())

internal/pkg/cli/app_show_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func TestShowAppOpts_Execute(t *testing.T) {
241241
wantedContent: `About
242242
243243
Name my-app
244-
Version v0.0.0 (latest available: v1.0.1)
244+
Version v0.0.0 (latest available: v1.0.2)
245245
URI example.com
246246
247247
Environments
@@ -301,7 +301,7 @@ Pipelines
301301
wantedContent: `About
302302
303303
Name my-app
304-
Version v1.0.1
304+
Version v1.0.2
305305
URI example.com
306306
307307
Environments

internal/pkg/deploy/app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const (
2020
// LegacyAppTemplateVersion is the version associated with the application template before we started versioning.
2121
LegacyAppTemplateVersion = "v0.0.0"
2222
// LatestAppTemplateVersion is the latest version number available for application templates.
23-
LatestAppTemplateVersion = "v1.0.1"
23+
LatestAppTemplateVersion = "v1.0.2"
2424
// AliasLeastAppTemplateVersion is the least version number available for HTTPS alias.
2525
AliasLeastAppTemplateVersion = "v1.0.0"
2626
)

internal/pkg/deploy/cloudformation/app.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ func (cf CloudFormation) DeployApp(in *deploy.CreateAppInput) error {
5656

5757
func (cf CloudFormation) UpgradeApplication(in *deploy.CreateAppInput) error {
5858
appConfig := stack.NewAppStackConfig(in)
59+
appStack, err := cf.cfnClient.Describe(appConfig.StackName())
60+
if err != nil {
61+
return fmt.Errorf("get existing application infrastructure stack: %w", err)
62+
}
63+
in.DNSDelegationAccounts = stack.DNSDelegatedAccountsForStack(appStack.SDK())
64+
appConfig = stack.NewAppStackConfig(in)
5965
s, err := toStack(appConfig)
6066
if err != nil {
6167
return err

internal/pkg/deploy/cloudformation/app_test.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,21 @@ func TestCloudFormation_UpgradeApplication(t *testing.T) {
123123

124124
wantedErr error
125125
}{
126+
"error if fail to get existing application infrastructure stack": {
127+
mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation {
128+
m := mocks.NewMockcfnClient(ctrl)
129+
m.EXPECT().Describe("phonetool-infrastructure-roles").Return(nil, errors.New("some error"))
130+
131+
return &CloudFormation{
132+
cfnClient: m,
133+
}
134+
},
135+
wantedErr: fmt.Errorf("get existing application infrastructure stack: some error"),
136+
},
126137
"error if fail to describe app stack": {
127138
mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation {
128139
m := mocks.NewMockcfnClient(ctrl)
140+
m.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil)
129141
m.EXPECT().Describe("phonetool-infrastructure-roles").Return(nil, errors.New("some error"))
130142

131143
return &CloudFormation{
@@ -137,7 +149,7 @@ func TestCloudFormation_UpgradeApplication(t *testing.T) {
137149
"error if fail to update app stack": {
138150
mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation {
139151
m := mocks.NewMockcfnClient(ctrl)
140-
m.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil)
152+
m.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil).Times(2)
141153
m.EXPECT().UpdateAndWait(gomock.Any()).Return(errors.New("some error"))
142154
return &CloudFormation{
143155
cfnClient: m,
@@ -148,7 +160,7 @@ func TestCloudFormation_UpgradeApplication(t *testing.T) {
148160
"error if fail to wait until stack set last operation complete": {
149161
mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation {
150162
mockCFNClient := mocks.NewMockcfnClient(ctrl)
151-
mockCFNClient.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil)
163+
mockCFNClient.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil).Times(2)
152164
mockCFNClient.EXPECT().UpdateAndWait(gomock.Any()).Return(nil)
153165

154166
mockAppStackSet := mocks.NewMockstackSetClient(ctrl)
@@ -165,7 +177,7 @@ func TestCloudFormation_UpgradeApplication(t *testing.T) {
165177
"success": {
166178
mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation {
167179
mockCFNClient := mocks.NewMockcfnClient(ctrl)
168-
mockCFNClient.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil)
180+
mockCFNClient.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil).Times(2)
169181
mockCFNClient.EXPECT().UpdateAndWait(gomock.Any()).Return(nil)
170182

171183
mockAppStackSet := mocks.NewMockstackSetClient(ctrl)
@@ -186,7 +198,7 @@ func TestCloudFormation_UpgradeApplication(t *testing.T) {
186198
mockCFNClient := mocks.NewMockcfnClient(ctrl)
187199
mockCFNClient.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{
188200
StackStatus: aws.String(awscfn.StackStatusCreateInProgress),
189-
}, nil)
201+
}, nil).Times(2)
190202
mockCFNClient.EXPECT().WaitForUpdate(gomock.Any(), "phonetool-infrastructure-roles").Return(nil)
191203
mockCFNClient.EXPECT().Describe("phonetool-infrastructure-roles").Return(&cloudformation.StackDescription{}, nil)
192204
mockCFNClient.EXPECT().UpdateAndWait(gomock.Any()).Return(nil)

internal/pkg/deploy/cloudformation/stack/app.go

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ type AppRegionalResources struct {
4848
}
4949

5050
const (
51-
fmtAppTemplatePath = "app/versions/%s/app.yml"
52-
fmtAppResourcesTemplatePath = "app/versions/%s/cf.yml"
51+
appTemplatePath = "app/app.yml"
52+
appResourcesTemplatePath = "app/cf.yml"
5353
appAdminRoleParamName = "AdminRoleName"
5454
appExecutionRoleParamName = "ExecutionRoleName"
5555
appDNSDelegationRoleParamName = "DNSDelegationRoleName"
@@ -89,10 +89,13 @@ func NewAppStackConfig(in *deploy.CreateAppInput) *AppStackConfig {
8989

9090
// Template returns the environment CloudFormation template.
9191
func (c *AppStackConfig) Template() (string, error) {
92-
// content, err := c.parser.Read(appTemplatePath(c.Version))
93-
content, err := c.parser.Parse(appTemplatePath(c.Version), struct {
94-
TemplateVersion string
95-
}{c.Version})
92+
content, err := c.parser.Parse(appTemplatePath, struct {
93+
TemplateVersion string
94+
AppDNSDelegatedAccounts []string
95+
}{
96+
c.Version,
97+
c.dnsDelegationAccounts(),
98+
})
9699
if err != nil {
97100
return "", err
98101
}
@@ -105,7 +108,7 @@ func (c *AppStackConfig) ResourceTemplate(config *AppResourcesConfig) (string, e
105108
sort.Strings(config.Accounts)
106109
sort.Strings(config.Services)
107110

108-
content, err := c.parser.Parse(appResourcesTemplatePath(c.Version), struct {
111+
content, err := c.parser.Parse(appResourcesTemplatePath, struct {
109112
*AppResourcesConfig
110113
ServiceTagKey string
111114
TemplateVersion string
@@ -120,20 +123,6 @@ func (c *AppStackConfig) ResourceTemplate(config *AppResourcesConfig) (string, e
120123
return content.String(), err
121124
}
122125

123-
func appTemplatePath(version string) string {
124-
if version == "" {
125-
return fmt.Sprintf(fmtAppTemplatePath, "v0.0.0")
126-
}
127-
return fmt.Sprintf(fmtAppTemplatePath, version)
128-
}
129-
130-
func appResourcesTemplatePath(version string) string {
131-
if version == "" {
132-
return fmt.Sprintf(fmtAppResourcesTemplatePath, "v0.0.0")
133-
}
134-
return fmt.Sprintf(fmtAppResourcesTemplatePath, version)
135-
}
136-
137126
// Parameters returns a list of parameters which accompany the app CloudFormation template.
138127
func (c *AppStackConfig) Parameters() ([]*cloudformation.Parameter, error) {
139128
return []*cloudformation.Parameter{

internal/pkg/deploy/cloudformation/stack/app_test.go

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,25 +43,12 @@ func TestAppTemplate(t *testing.T) {
4343
inVersion: "v1.0.0",
4444
mockDependencies: func(ctrl *gomock.Controller, c *AppStackConfig) {
4545
m := mocks.NewMockReadParser(ctrl)
46-
m.EXPECT().Parse(fmt.Sprintf(fmtAppTemplatePath, "v1.0.0"), struct {
47-
TemplateVersion string
46+
m.EXPECT().Parse(appTemplatePath, struct {
47+
TemplateVersion string
48+
AppDNSDelegatedAccounts []string
4849
}{
4950
"v1.0.0",
50-
}, gomock.Any()).Return(&template.Content{
51-
Buffer: bytes.NewBufferString("template"),
52-
}, nil)
53-
c.parser = m
54-
},
55-
56-
wantedTemplate: "template",
57-
},
58-
"success for legacy template": {
59-
mockDependencies: func(ctrl *gomock.Controller, c *AppStackConfig) {
60-
m := mocks.NewMockReadParser(ctrl)
61-
m.EXPECT().Parse(fmt.Sprintf(fmtAppTemplatePath, "v0.0.0"), struct {
62-
TemplateVersion string
63-
}{
64-
"",
51+
[]string{"123456"},
6552
}, gomock.Any()).Return(&template.Content{
6653
Buffer: bytes.NewBufferString("template"),
6754
}, nil)
@@ -79,7 +66,8 @@ func TestAppTemplate(t *testing.T) {
7966
defer ctrl.Finish()
8067
appStack := &AppStackConfig{
8168
CreateAppInput: &deploy.CreateAppInput{
82-
Version: tc.inVersion,
69+
Version: tc.inVersion,
70+
AccountID: "123456",
8371
},
8472
}
8573
tc.mockDependencies(ctrl, appStack)
@@ -159,7 +147,7 @@ func TestAppResourceTemplate(t *testing.T) {
159147
},
160148
mockDependencies: func(ctrl *gomock.Controller, c *AppStackConfig) {
161149
m := mocks.NewMockReadParser(ctrl)
162-
m.EXPECT().Parse(fmt.Sprintf(fmtAppResourcesTemplatePath, "v0.0.0"), struct {
150+
m.EXPECT().Parse(appResourcesTemplatePath, struct {
163151
*AppResourcesConfig
164152
ServiceTagKey string
165153
TemplateVersion string

templates/app/versions/v1.0.1/app.yml renamed to templates/app/app.yml

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
AWSTemplateFormatVersion: 2010-09-09
44
Description: Configure the AWSCloudFormationStackSetAdministrationRole to enable use of AWS CloudFormation StackSets.
55
Metadata:
6-
TemplateVersion: 'v1.0.1'
6+
TemplateVersion: 'v1.0.2'
77
Parameters:
88
AdminRoleName:
99
Type: String
@@ -118,13 +118,10 @@ Resources:
118118
- sts:AssumeRole
119119
- Effect: Allow
120120
Principal:
121-
AWS: !Split
122-
- ','
123-
- !Sub
124-
- 'arn:${AWS::Partition}:iam::${inner}:root'
125-
- inner: !Join
126-
- ':root,arn:${AWS::Partition}:iam::'
127-
- Ref: "AppDNSDelegatedAccounts"
121+
AWS:
122+
{{- range $account := .AppDNSDelegatedAccounts}}
123+
- !Sub arn:${AWS::Partition}:iam::{{$account}}:root
124+
{{- end}}
128125
Action:
129126
- sts:AssumeRole
130127
Path: /

templates/app/versions/v1.0.1/cf.yml renamed to templates/app/cf.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ AWSTemplateFormatVersion: '2010-09-09'{{$accounts := .Accounts}}{{$app := .App}}
55
# to support the CodePipeline for a workspace
66
Description: Cross-regional resources to support the CodePipeline for a workspace
77
Metadata:
8-
TemplateVersion: 'v1.0.1'
8+
TemplateVersion: 'v1.0.2'
99
Version: {{.Version}}
1010
Services:{{if not $services}} []{{else}}{{range $service := $services}}
1111
- {{$service}}{{end}}{{end}}

0 commit comments

Comments
 (0)