Skip to content

Commit 6a758d5

Browse files
Lou1415926Lou1415926
authored
chore(deploy): refactor deployment input so that app-related information is grouped together (#2621)
Previously only environment deployment input needs `AppDNSName` and `AppDelegationRole`. Now in order to enable alias for RD service, these information are needed for RD service deployment as well. This PR refactors the `deploy` package to avoid repetitive code that would have been added without the refactoring. 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 b32983c commit 6a758d5

File tree

17 files changed

+237
-173
lines changed

17 files changed

+237
-173
lines changed

‎internal/pkg/cli/env_init.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -560,16 +560,18 @@ func (o *initEnvOpts) deployEnv(app *config.Application, customResourcesURLs map
560560
return fmt.Errorf("get identity: %w", err)
561561
}
562562
deployEnvInput := &deploy.CreateEnvironmentInput{
563-
Name: o.name,
564-
AppName: o.appName,
565-
Prod: o.isProduction,
566-
ToolsAccountPrincipalARN: caller.RootUserARN,
567-
AppDNSName: app.Domain,
568-
AdditionalTags: app.Tags,
569-
CustomResourcesURLs: customResourcesURLs,
570-
AdjustVPCConfig: o.adjustVPCConfig(),
571-
ImportVPCConfig: o.importVPCConfig(),
572-
Version: deploy.LatestEnvTemplateVersion,
563+
Name: o.name,
564+
App: deploy.AppInformation{
565+
Name: o.appName,
566+
DNSName: app.Domain,
567+
AccountPrincipalARN: caller.RootUserARN,
568+
},
569+
Prod: o.isProduction,
570+
AdditionalTags: app.Tags,
571+
CustomResourcesURLs: customResourcesURLs,
572+
AdjustVPCConfig: o.adjustVPCConfig(),
573+
ImportVPCConfig: o.importVPCConfig(),
574+
Version: deploy.LatestEnvTemplateVersion,
573575
}
574576

575577
if err := o.cleanUpDanglingRoles(o.appName, o.name); err != nil {

‎internal/pkg/cli/env_init_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -885,11 +885,13 @@ func TestInitEnvOpts_Execute(t *testing.T) {
885885
},
886886
expectDeployer: func(m *mocks.Mockdeployer) {
887887
m.EXPECT().DeployAndRenderEnvironment(gomock.Any(), &deploy.CreateEnvironmentInput{
888-
Name: "test",
889-
AppName: "phonetool",
890-
ToolsAccountPrincipalARN: "some arn",
891-
CustomResourcesURLs: map[string]string{"mockCustomResource": "mockURL"},
892-
Version: deploy.LatestEnvTemplateVersion,
888+
Name: "test",
889+
App: deploy.AppInformation{
890+
Name: "phonetool",
891+
AccountPrincipalARN: "some arn",
892+
},
893+
CustomResourcesURLs: map[string]string{"mockCustomResource": "mockURL"},
894+
Version: deploy.LatestEnvTemplateVersion,
893895
}).Return(&cloudformation.ErrStackAlreadyExists{})
894896
m.EXPECT().GetEnvironment("phonetool", "test").Return(&config.Environment{
895897
AccountID: "1234",

‎internal/pkg/cli/env_upgrade.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ func newEnvUpgradeOpts(vars envUpgradeVars) (*envUpgradeOpts, error) {
7979
sel: selector.NewSelect(prompt.New(), store),
8080
legacyEnvTemplater: stack.NewEnvStackConfig(&deploy.CreateEnvironmentInput{
8181
Version: deploy.LegacyEnvTemplateVersion,
82-
AppName: vars.appName,
82+
App: deploy.AppInformation{
83+
Name: vars.appName,
84+
},
8385
}),
8486
prog: termprogress.NewSpinner(log.DiagnosticWriter),
8587
uploader: template.New(),
@@ -274,8 +276,10 @@ func (o *envUpgradeOpts) upgradeEnvironment(upgrader envUpgrader, conf *config.E
274276
}
275277

276278
if err := upgrader.UpgradeEnvironment(&deploy.CreateEnvironmentInput{
277-
Version: toVersion,
278-
AppName: conf.App,
279+
Version: toVersion,
280+
App: deploy.AppInformation{
281+
Name: conf.App,
282+
},
279283
Name: conf.Name,
280284
CustomResourcesURLs: customResourcesURLs,
281285
ImportVPCConfig: importedVPC,
@@ -299,8 +303,10 @@ func (o *envUpgradeOpts) upgradeLegacyEnvironment(upgrader legacyEnvUpgrader, co
299303
}
300304
if isDefaultEnv {
301305
if err := upgrader.UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
302-
Version: toVersion,
303-
AppName: conf.App,
306+
Version: toVersion,
307+
App: deploy.AppInformation{
308+
Name: conf.App,
309+
},
304310
Name: conf.Name,
305311
CustomResourcesURLs: customResourcesURLs,
306312
CFNServiceRoleARN: conf.ExecutionRoleARN,
@@ -343,8 +349,10 @@ func (o *envUpgradeOpts) upgradeLegacyEnvironmentWithVPCOverrides(upgrader legac
343349
fromVersion, toVersion string, albWorkloads []string) error {
344350
if conf.CustomConfig != nil {
345351
if err := upgrader.UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
346-
Version: toVersion,
347-
AppName: conf.App,
352+
Version: toVersion,
353+
App: deploy.AppInformation{
354+
Name: conf.App,
355+
},
348356
Name: conf.Name,
349357
ImportVPCConfig: conf.CustomConfig.ImportVPC,
350358
AdjustVPCConfig: conf.CustomConfig.VPCConfig,

‎internal/pkg/cli/env_upgrade_test.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,10 @@ func TestEnvUpgradeOpts_Execute(t *testing.T) {
280280
mockUpgrader := mocks.NewMockenvTemplateUpgrader(ctrl)
281281
mockUpgrader.EXPECT().UpgradeEnvironment(&deploy.CreateEnvironmentInput{
282282
Version: deploy.LatestEnvTemplateVersion,
283-
AppName: "phonetool",
284-
Name: "test",
283+
App: deploy.AppInformation{
284+
Name: "phonetool",
285+
},
286+
Name: "test",
285287
ImportVPCConfig: &config.ImportVPC{
286288
ID: "abc",
287289
},
@@ -354,8 +356,10 @@ func TestEnvUpgradeOpts_Execute(t *testing.T) {
354356
mockUpgrader := mocks.NewMockenvTemplateUpgrader(ctrl)
355357
mockUpgrader.EXPECT().EnvironmentTemplate("phonetool", "test").Return("template", nil)
356358
mockUpgrader.EXPECT().UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
357-
Version: deploy.LatestEnvTemplateVersion,
358-
AppName: "phonetool",
359+
Version: deploy.LatestEnvTemplateVersion,
360+
App: deploy.AppInformation{
361+
Name: "phonetool",
362+
},
359363
Name: "test",
360364
CFNServiceRoleARN: "execARN",
361365
CustomResourcesURLs: map[string]string{"mockCustomResource": "mockURL"},
@@ -421,8 +425,10 @@ func TestEnvUpgradeOpts_Execute(t *testing.T) {
421425
mockUpgrader := mocks.NewMockenvTemplateUpgrader(ctrl)
422426
mockUpgrader.EXPECT().EnvironmentTemplate("phonetool", "test").Return("modified template", nil)
423427
mockUpgrader.EXPECT().UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
424-
Version: deploy.LatestEnvTemplateVersion,
425-
AppName: "phonetool",
428+
Version: deploy.LatestEnvTemplateVersion,
429+
App: deploy.AppInformation{
430+
Name: "phonetool",
431+
},
426432
Name: "test",
427433
CFNServiceRoleARN: "execARN",
428434
ImportVPCConfig: &config.ImportVPC{

‎internal/pkg/cli/svc_deploy.go

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,19 @@ type deployWkldVars struct {
4949
type deploySvcOpts struct {
5050
deployWkldVars
5151

52-
store store
53-
ws wsSvcDirReader
54-
imageBuilderPusher imageBuilderPusher
55-
unmarshal func([]byte) (manifest.WorkloadManifest, error)
56-
s3 artifactUploader
57-
cmd runner
58-
addons templater
59-
appCFN appResourcesGetter
60-
svcCFN cloudformation.CloudFormation
61-
sessProvider sessionProvider
62-
envUpgradeCmd actionCommand
52+
store store
53+
ws wsSvcDirReader
54+
imageBuilderPusher imageBuilderPusher
55+
unmarshal func([]byte) (manifest.WorkloadManifest, error)
56+
s3 artifactUploader
57+
cmd runner
58+
addons templater
59+
appCFN appResourcesGetter
60+
svcCFN cloudformation.CloudFormation
61+
sessProvider sessionProvider
62+
envUpgradeCmd actionCommand
6363
newAppVersionGetter func(string) (versionGetter, error)
64-
endpointGetter endpointGetter
64+
endpointGetter endpointGetter
6565

6666
spinner progress
6767
sel wsSelector
@@ -455,7 +455,12 @@ func (o *deploySvcOpts) stackConfiguration(addonsURL string) (cloudformation.Sta
455455
conf, err = stack.NewLoadBalancedWebService(t, o.targetEnvironment.Name, o.targetEnvironment.App, *rc)
456456
}
457457
case *manifest.RequestDrivenWebService:
458-
conf, err = stack.NewRequestDrivenWebService(t, o.targetEnvironment.Name, o.targetEnvironment.App, *rc)
458+
appInfo := deploy.AppInformation{
459+
Name: o.targetEnvironment.App,
460+
DNSName: o.targetApp.Domain,
461+
AccountPrincipalARN: o.targetApp.AccountID,
462+
}
463+
conf, err = stack.NewRequestDrivenWebService(t, o.targetEnvironment.Name, appInfo, *rc)
459464
case *manifest.BackendService:
460465
conf, err = stack.NewBackendService(t, o.targetEnvironment.Name, o.targetEnvironment.App, *rc)
461466
default:

‎internal/pkg/cli/svc_package.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,12 @@ func newPackageSvcOpts(vars packageSvcVars) (*packageSvcOpts, error) {
125125
}
126126
}
127127
case *manifest.RequestDrivenWebService:
128-
serializer, err = stack.NewRequestDrivenWebService(t, env.Name, app.Name, rc)
128+
appInfo := deploy.AppInformation{
129+
Name: env.App,
130+
DNSName: app.Domain,
131+
AccountPrincipalARN: app.AccountID,
132+
}
133+
serializer, err = stack.NewRequestDrivenWebService(t, env.Name, appInfo, rc)
129134
if err != nil {
130135
return nil, fmt.Errorf("init request-driven web service stack serializer: %w", err)
131136
}

‎internal/pkg/deploy/app.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@
55
// This file defines application deployment resources.
66
package deploy
77

8+
import (
9+
"fmt"
10+
11+
"github.com/aws/aws-sdk-go/aws/arn"
12+
)
13+
14+
const appDNSDelegationRoleName = "DNSDelegationRole"
15+
816
// CreateAppInput holds the fields required to create an application stack set.
917
type CreateAppInput struct {
1018
Name string // Name of the application that needs to be created.
@@ -24,3 +32,28 @@ const (
2432
// AliasLeastAppTemplateVersion is the least version number available for HTTPS alias.
2533
AliasLeastAppTemplateVersion = "v1.0.0"
2634
)
35+
36+
// AppInformation holds information about the application that need to be propagated to the env stacks and workload stacks.
37+
type AppInformation struct {
38+
AccountPrincipalARN string
39+
DNSName string
40+
Name string
41+
}
42+
43+
// DNSDelegationRole returns the ARN of the app's DNS delegation role.
44+
func (a *AppInformation) DNSDelegationRole() string {
45+
if a.AccountPrincipalARN == "" || a.DNSName == "" {
46+
return ""
47+
}
48+
49+
appRole, err := arn.Parse(a.AccountPrincipalARN)
50+
if err != nil {
51+
return ""
52+
}
53+
return fmt.Sprintf("arn:%s:iam::%s:role/%s", appRole.Partition, appRole.AccountID, DNSDelegationRoleName(a.Name))
54+
}
55+
56+
// DNSDelegationRoleName returns the DNSDelegation role name of the app.
57+
func DNSDelegationRoleName(appName string) string {
58+
return fmt.Sprintf("%s-%s", appName, appDNSDelegationRoleName)
59+
}

‎internal/pkg/deploy/app_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package deploy
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestAppInformation_DNSDelegationRole(t *testing.T) {
10+
testCases := map[string]struct {
11+
in *AppInformation
12+
want string
13+
}{
14+
"without tools account ARN": {
15+
want: "",
16+
in: &AppInformation{
17+
AccountPrincipalARN: "",
18+
DNSName: "ecs.aws",
19+
},
20+
},
21+
"without DNS": {
22+
want: "",
23+
in: &AppInformation{
24+
AccountPrincipalARN: "",
25+
DNSName: "ecs.aws",
26+
},
27+
},
28+
"with invalid tools principal": {
29+
want: "",
30+
in: &AppInformation{
31+
AccountPrincipalARN: "0000000",
32+
DNSName: "ecs.aws",
33+
},
34+
},
35+
"with dns and tools principal": {
36+
want: "arn:aws:iam::0000000:role/-DNSDelegationRole",
37+
38+
in: &AppInformation{
39+
AccountPrincipalARN: "arn:aws:iam::0000000:root",
40+
DNSName: "ecs.aws",
41+
},
42+
},
43+
}
44+
45+
for name, tc := range testCases {
46+
t.Run(name, func(t *testing.T) {
47+
require.Equal(t, tc.want, tc.in.DNSDelegationRole())
48+
})
49+
}
50+
}

‎internal/pkg/deploy/cloudformation/env.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,21 @@ func (cf CloudFormation) DeployAndRenderEnvironment(out progress.FileWriter, env
5252
// DeleteEnvironment deletes the CloudFormation stack of an environment.
5353
func (cf CloudFormation) DeleteEnvironment(appName, envName, cfnExecRoleARN string) error {
5454
conf := stack.NewEnvStackConfig(&deploy.CreateEnvironmentInput{
55-
AppName: appName,
56-
Name: envName,
55+
App: deploy.AppInformation{
56+
Name: appName,
57+
},
58+
Name: envName,
5759
})
5860
return cf.cfnClient.DeleteAndWaitWithRoleARN(conf.StackName(), cfnExecRoleARN)
5961
}
6062

6163
// GetEnvironment returns the Environment metadata from the CloudFormation stack.
6264
func (cf CloudFormation) GetEnvironment(appName, envName string) (*config.Environment, error) {
6365
conf := stack.NewEnvStackConfig(&deploy.CreateEnvironmentInput{
64-
AppName: appName,
65-
Name: envName,
66+
App: deploy.AppInformation{
67+
Name: appName,
68+
},
69+
Name: envName,
6670
})
6771
descr, err := cf.cfnClient.Describe(conf.StackName())
6872
if err != nil {

‎internal/pkg/deploy/cloudformation/env_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import (
1919
)
2020

2121
var mockCreateEnvInput = deploy.CreateEnvironmentInput{
22-
AppName: "phonetool",
22+
App: deploy.AppInformation{
23+
Name: "phonetool",
24+
},
2325
Name: "test",
2426
Version: "v1.0.0",
2527
CustomResourcesURLs: map[string]string{

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ const (
6060
appDomainNameKey = "AppDomainName"
6161
appDomainHostedZoneIDKey = "AppDomainHostedZoneID"
6262
appNameKey = "AppName"
63-
appDNSDelegationRoleName = "DNSDelegationRole"
6463

6564
// arn:${partition}:iam::${account}:role/${roleName}
6665
fmtStackSetAdminRoleARN = "arn:%s:iam::%s:role/%s"
@@ -152,7 +151,7 @@ func (c *AppStackConfig) Parameters() ([]*cloudformation.Parameter, error) {
152151
},
153152
{
154153
ParameterKey: aws.String(appDNSDelegationRoleParamName),
155-
ParameterValue: aws.String(dnsDelegationRoleName(c.Name)),
154+
ParameterValue: aws.String(deploy.DNSDelegationRoleName(c.Name)),
156155
},
157156
}, nil
158157
}
@@ -268,7 +267,3 @@ func DNSDelegatedAccountsForStack(stack *cloudformation.Stack) []string {
268267

269268
return []string{}
270269
}
271-
272-
func dnsDelegationRoleName(appName string) string {
273-
return fmt.Sprintf("%s-%s", appName, appDNSDelegationRoleName)
274-
}

0 commit comments

Comments
 (0)