Skip to content

Commit

Permalink
chore(deploy): refactor deployment input so that app-related informat…
Browse files Browse the repository at this point in the history
…ion is grouped together (aws#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.
  • Loading branch information
Lou1415926 committed Jul 20, 2021
1 parent b32983c commit 6a758d5
Show file tree
Hide file tree
Showing 17 changed files with 237 additions and 173 deletions.
22 changes: 12 additions & 10 deletions internal/pkg/cli/env_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,16 +560,18 @@ func (o *initEnvOpts) deployEnv(app *config.Application, customResourcesURLs map
return fmt.Errorf("get identity: %w", err)
}
deployEnvInput := &deploy.CreateEnvironmentInput{
Name: o.name,
AppName: o.appName,
Prod: o.isProduction,
ToolsAccountPrincipalARN: caller.RootUserARN,
AppDNSName: app.Domain,
AdditionalTags: app.Tags,
CustomResourcesURLs: customResourcesURLs,
AdjustVPCConfig: o.adjustVPCConfig(),
ImportVPCConfig: o.importVPCConfig(),
Version: deploy.LatestEnvTemplateVersion,
Name: o.name,
App: deploy.AppInformation{
Name: o.appName,
DNSName: app.Domain,
AccountPrincipalARN: caller.RootUserARN,
},
Prod: o.isProduction,
AdditionalTags: app.Tags,
CustomResourcesURLs: customResourcesURLs,
AdjustVPCConfig: o.adjustVPCConfig(),
ImportVPCConfig: o.importVPCConfig(),
Version: deploy.LatestEnvTemplateVersion,
}

if err := o.cleanUpDanglingRoles(o.appName, o.name); err != nil {
Expand Down
12 changes: 7 additions & 5 deletions internal/pkg/cli/env_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,11 +885,13 @@ func TestInitEnvOpts_Execute(t *testing.T) {
},
expectDeployer: func(m *mocks.Mockdeployer) {
m.EXPECT().DeployAndRenderEnvironment(gomock.Any(), &deploy.CreateEnvironmentInput{
Name: "test",
AppName: "phonetool",
ToolsAccountPrincipalARN: "some arn",
CustomResourcesURLs: map[string]string{"mockCustomResource": "mockURL"},
Version: deploy.LatestEnvTemplateVersion,
Name: "test",
App: deploy.AppInformation{
Name: "phonetool",
AccountPrincipalARN: "some arn",
},
CustomResourcesURLs: map[string]string{"mockCustomResource": "mockURL"},
Version: deploy.LatestEnvTemplateVersion,
}).Return(&cloudformation.ErrStackAlreadyExists{})
m.EXPECT().GetEnvironment("phonetool", "test").Return(&config.Environment{
AccountID: "1234",
Expand Down
22 changes: 15 additions & 7 deletions internal/pkg/cli/env_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ func newEnvUpgradeOpts(vars envUpgradeVars) (*envUpgradeOpts, error) {
sel: selector.NewSelect(prompt.New(), store),
legacyEnvTemplater: stack.NewEnvStackConfig(&deploy.CreateEnvironmentInput{
Version: deploy.LegacyEnvTemplateVersion,
AppName: vars.appName,
App: deploy.AppInformation{
Name: vars.appName,
},
}),
prog: termprogress.NewSpinner(log.DiagnosticWriter),
uploader: template.New(),
Expand Down Expand Up @@ -274,8 +276,10 @@ func (o *envUpgradeOpts) upgradeEnvironment(upgrader envUpgrader, conf *config.E
}

if err := upgrader.UpgradeEnvironment(&deploy.CreateEnvironmentInput{
Version: toVersion,
AppName: conf.App,
Version: toVersion,
App: deploy.AppInformation{
Name: conf.App,
},
Name: conf.Name,
CustomResourcesURLs: customResourcesURLs,
ImportVPCConfig: importedVPC,
Expand All @@ -299,8 +303,10 @@ func (o *envUpgradeOpts) upgradeLegacyEnvironment(upgrader legacyEnvUpgrader, co
}
if isDefaultEnv {
if err := upgrader.UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
Version: toVersion,
AppName: conf.App,
Version: toVersion,
App: deploy.AppInformation{
Name: conf.App,
},
Name: conf.Name,
CustomResourcesURLs: customResourcesURLs,
CFNServiceRoleARN: conf.ExecutionRoleARN,
Expand Down Expand Up @@ -343,8 +349,10 @@ func (o *envUpgradeOpts) upgradeLegacyEnvironmentWithVPCOverrides(upgrader legac
fromVersion, toVersion string, albWorkloads []string) error {
if conf.CustomConfig != nil {
if err := upgrader.UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
Version: toVersion,
AppName: conf.App,
Version: toVersion,
App: deploy.AppInformation{
Name: conf.App,
},
Name: conf.Name,
ImportVPCConfig: conf.CustomConfig.ImportVPC,
AdjustVPCConfig: conf.CustomConfig.VPCConfig,
Expand Down
18 changes: 12 additions & 6 deletions internal/pkg/cli/env_upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,10 @@ func TestEnvUpgradeOpts_Execute(t *testing.T) {
mockUpgrader := mocks.NewMockenvTemplateUpgrader(ctrl)
mockUpgrader.EXPECT().UpgradeEnvironment(&deploy.CreateEnvironmentInput{
Version: deploy.LatestEnvTemplateVersion,
AppName: "phonetool",
Name: "test",
App: deploy.AppInformation{
Name: "phonetool",
},
Name: "test",
ImportVPCConfig: &config.ImportVPC{
ID: "abc",
},
Expand Down Expand Up @@ -354,8 +356,10 @@ func TestEnvUpgradeOpts_Execute(t *testing.T) {
mockUpgrader := mocks.NewMockenvTemplateUpgrader(ctrl)
mockUpgrader.EXPECT().EnvironmentTemplate("phonetool", "test").Return("template", nil)
mockUpgrader.EXPECT().UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
Version: deploy.LatestEnvTemplateVersion,
AppName: "phonetool",
Version: deploy.LatestEnvTemplateVersion,
App: deploy.AppInformation{
Name: "phonetool",
},
Name: "test",
CFNServiceRoleARN: "execARN",
CustomResourcesURLs: map[string]string{"mockCustomResource": "mockURL"},
Expand Down Expand Up @@ -421,8 +425,10 @@ func TestEnvUpgradeOpts_Execute(t *testing.T) {
mockUpgrader := mocks.NewMockenvTemplateUpgrader(ctrl)
mockUpgrader.EXPECT().EnvironmentTemplate("phonetool", "test").Return("modified template", nil)
mockUpgrader.EXPECT().UpgradeLegacyEnvironment(&deploy.CreateEnvironmentInput{
Version: deploy.LatestEnvTemplateVersion,
AppName: "phonetool",
Version: deploy.LatestEnvTemplateVersion,
App: deploy.AppInformation{
Name: "phonetool",
},
Name: "test",
CFNServiceRoleARN: "execARN",
ImportVPCConfig: &config.ImportVPC{
Expand Down
31 changes: 18 additions & 13 deletions internal/pkg/cli/svc_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@ type deployWkldVars struct {
type deploySvcOpts struct {
deployWkldVars

store store
ws wsSvcDirReader
imageBuilderPusher imageBuilderPusher
unmarshal func([]byte) (manifest.WorkloadManifest, error)
s3 artifactUploader
cmd runner
addons templater
appCFN appResourcesGetter
svcCFN cloudformation.CloudFormation
sessProvider sessionProvider
envUpgradeCmd actionCommand
store store
ws wsSvcDirReader
imageBuilderPusher imageBuilderPusher
unmarshal func([]byte) (manifest.WorkloadManifest, error)
s3 artifactUploader
cmd runner
addons templater
appCFN appResourcesGetter
svcCFN cloudformation.CloudFormation
sessProvider sessionProvider
envUpgradeCmd actionCommand
newAppVersionGetter func(string) (versionGetter, error)
endpointGetter endpointGetter
endpointGetter endpointGetter

spinner progress
sel wsSelector
Expand Down Expand Up @@ -455,7 +455,12 @@ func (o *deploySvcOpts) stackConfiguration(addonsURL string) (cloudformation.Sta
conf, err = stack.NewLoadBalancedWebService(t, o.targetEnvironment.Name, o.targetEnvironment.App, *rc)
}
case *manifest.RequestDrivenWebService:
conf, err = stack.NewRequestDrivenWebService(t, o.targetEnvironment.Name, o.targetEnvironment.App, *rc)
appInfo := deploy.AppInformation{
Name: o.targetEnvironment.App,
DNSName: o.targetApp.Domain,
AccountPrincipalARN: o.targetApp.AccountID,
}
conf, err = stack.NewRequestDrivenWebService(t, o.targetEnvironment.Name, appInfo, *rc)
case *manifest.BackendService:
conf, err = stack.NewBackendService(t, o.targetEnvironment.Name, o.targetEnvironment.App, *rc)
default:
Expand Down
7 changes: 6 additions & 1 deletion internal/pkg/cli/svc_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,12 @@ func newPackageSvcOpts(vars packageSvcVars) (*packageSvcOpts, error) {
}
}
case *manifest.RequestDrivenWebService:
serializer, err = stack.NewRequestDrivenWebService(t, env.Name, app.Name, rc)
appInfo := deploy.AppInformation{
Name: env.App,
DNSName: app.Domain,
AccountPrincipalARN: app.AccountID,
}
serializer, err = stack.NewRequestDrivenWebService(t, env.Name, appInfo, rc)
if err != nil {
return nil, fmt.Errorf("init request-driven web service stack serializer: %w", err)
}
Expand Down
33 changes: 33 additions & 0 deletions internal/pkg/deploy/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
// This file defines application deployment resources.
package deploy

import (
"fmt"

"github.com/aws/aws-sdk-go/aws/arn"
)

const appDNSDelegationRoleName = "DNSDelegationRole"

// CreateAppInput holds the fields required to create an application stack set.
type CreateAppInput struct {
Name string // Name of the application that needs to be created.
Expand All @@ -24,3 +32,28 @@ const (
// AliasLeastAppTemplateVersion is the least version number available for HTTPS alias.
AliasLeastAppTemplateVersion = "v1.0.0"
)

// AppInformation holds information about the application that need to be propagated to the env stacks and workload stacks.
type AppInformation struct {
AccountPrincipalARN string
DNSName string
Name string
}

// DNSDelegationRole returns the ARN of the app's DNS delegation role.
func (a *AppInformation) DNSDelegationRole() string {
if a.AccountPrincipalARN == "" || a.DNSName == "" {
return ""
}

appRole, err := arn.Parse(a.AccountPrincipalARN)
if err != nil {
return ""
}
return fmt.Sprintf("arn:%s:iam::%s:role/%s", appRole.Partition, appRole.AccountID, DNSDelegationRoleName(a.Name))
}

// DNSDelegationRoleName returns the DNSDelegation role name of the app.
func DNSDelegationRoleName(appName string) string {
return fmt.Sprintf("%s-%s", appName, appDNSDelegationRoleName)
}
50 changes: 50 additions & 0 deletions internal/pkg/deploy/app_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package deploy

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestAppInformation_DNSDelegationRole(t *testing.T) {
testCases := map[string]struct {
in *AppInformation
want string
}{
"without tools account ARN": {
want: "",
in: &AppInformation{
AccountPrincipalARN: "",
DNSName: "ecs.aws",
},
},
"without DNS": {
want: "",
in: &AppInformation{
AccountPrincipalARN: "",
DNSName: "ecs.aws",
},
},
"with invalid tools principal": {
want: "",
in: &AppInformation{
AccountPrincipalARN: "0000000",
DNSName: "ecs.aws",
},
},
"with dns and tools principal": {
want: "arn:aws:iam::0000000:role/-DNSDelegationRole",

in: &AppInformation{
AccountPrincipalARN: "arn:aws:iam::0000000:root",
DNSName: "ecs.aws",
},
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
require.Equal(t, tc.want, tc.in.DNSDelegationRole())
})
}
}
12 changes: 8 additions & 4 deletions internal/pkg/deploy/cloudformation/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,21 @@ func (cf CloudFormation) DeployAndRenderEnvironment(out progress.FileWriter, env
// DeleteEnvironment deletes the CloudFormation stack of an environment.
func (cf CloudFormation) DeleteEnvironment(appName, envName, cfnExecRoleARN string) error {
conf := stack.NewEnvStackConfig(&deploy.CreateEnvironmentInput{
AppName: appName,
Name: envName,
App: deploy.AppInformation{
Name: appName,
},
Name: envName,
})
return cf.cfnClient.DeleteAndWaitWithRoleARN(conf.StackName(), cfnExecRoleARN)
}

// GetEnvironment returns the Environment metadata from the CloudFormation stack.
func (cf CloudFormation) GetEnvironment(appName, envName string) (*config.Environment, error) {
conf := stack.NewEnvStackConfig(&deploy.CreateEnvironmentInput{
AppName: appName,
Name: envName,
App: deploy.AppInformation{
Name: appName,
},
Name: envName,
})
descr, err := cf.cfnClient.Describe(conf.StackName())
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion internal/pkg/deploy/cloudformation/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import (
)

var mockCreateEnvInput = deploy.CreateEnvironmentInput{
AppName: "phonetool",
App: deploy.AppInformation{
Name: "phonetool",
},
Name: "test",
Version: "v1.0.0",
CustomResourcesURLs: map[string]string{
Expand Down
7 changes: 1 addition & 6 deletions internal/pkg/deploy/cloudformation/stack/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ const (
appDomainNameKey = "AppDomainName"
appDomainHostedZoneIDKey = "AppDomainHostedZoneID"
appNameKey = "AppName"
appDNSDelegationRoleName = "DNSDelegationRole"

// arn:${partition}:iam::${account}:role/${roleName}
fmtStackSetAdminRoleARN = "arn:%s:iam::%s:role/%s"
Expand Down Expand Up @@ -152,7 +151,7 @@ func (c *AppStackConfig) Parameters() ([]*cloudformation.Parameter, error) {
},
{
ParameterKey: aws.String(appDNSDelegationRoleParamName),
ParameterValue: aws.String(dnsDelegationRoleName(c.Name)),
ParameterValue: aws.String(deploy.DNSDelegationRoleName(c.Name)),
},
}, nil
}
Expand Down Expand Up @@ -268,7 +267,3 @@ func DNSDelegatedAccountsForStack(stack *cloudformation.Stack) []string {

return []string{}
}

func dnsDelegationRoleName(appName string) string {
return fmt.Sprintf("%s-%s", appName, appDNSDelegationRoleName)
}

0 comments on commit 6a758d5

Please sign in to comment.