Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow multiple environments per VPC #2394

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,7 @@ golang.org/x/tools v0.0.0-20200501065659-ab2804fb9c9d/go.mod h1:EkVYQZoAsY45+roY
golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200512131952-2bc93b1c0c88/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200804011535-6c149bb5ef0d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e h1:4nW4NLDYnU28ojHaHO8OVxFHk/aQ33U01a9cjED+pzE=
golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down
1 change: 0 additions & 1 deletion internal/pkg/aws/cloudformation/cloudformation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,6 @@ func TestCloudFormation_Exists(t *testing.T) {
require.True(t, exists)
})
}

func TestCloudFormation_TemplateBody(t *testing.T) {
testCases := map[string]struct {
createMock func(ctrl *gomock.Controller) client
Expand Down
4 changes: 4 additions & 0 deletions internal/pkg/cli/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,10 @@ type appResourcesGetter interface {
GetRegionalAppResources(app *config.Application) ([]*stack.AppRegionalResources, error)
}

type envCFDescriber interface {
EnvironmentUsesLegacySvcDiscovery(app, env string) (bool, error)
}

type taskDeployer interface {
DeployTask(out termprogress.FileWriter, input *deploy.CreateTaskResourcesInput, opts ...awscloudformation.StackOption) error
}
Expand Down
17 changes: 13 additions & 4 deletions internal/pkg/cli/job_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type deployJobOpts struct {
addons templater
appCFN appResourcesGetter
jobCFN cloudformation.CloudFormation
envCFN envCFDescriber
imageBuilderPusher imageBuilderPusher
sessProvider sessionProvider
s3 artifactUploader
Expand Down Expand Up @@ -207,6 +208,8 @@ func (o *deployJobOpts) configureClients() error {
// CF client against env account profile AND target environment region
o.jobCFN = cloudformation.New(envSession)

o.envCFN = cloudformation.New(envSession)

addonsSvc, err := addon.New(o.name)
if err != nil {
return fmt.Errorf("initiate addons service: %w", err)
Expand Down Expand Up @@ -300,10 +303,15 @@ func (o *deployJobOpts) stackConfiguration(addonsURL string) (cloudformation.Sta
}

func (o *deployJobOpts) runtimeConfig(addonsURL string) (*stack.RuntimeConfig, error) {
svcDiscovery, err := envUsesLegacySvcDiscovery(o.envCFN, o.appName, o.envName)
if err != nil {
return nil, err
}
if !o.buildRequired {
return &stack.RuntimeConfig{
AddonsTemplateURL: addonsURL,
AdditionalTags: tags.Merge(o.targetApp.Tags, o.resourceTags),
AddonsTemplateURL: addonsURL,
AdditionalTags: tags.Merge(o.targetApp.Tags, o.resourceTags),
LegacyServiceDiscovery: svcDiscovery,
Copy link
Contributor

Choose a reason for hiding this comment

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

does copilot job use service discovery 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We inject the endpoint as an environment variable, so we still have to update this here.

}, nil
}
resources, err := o.appCFN.GetAppResourcesByRegion(o.targetApp, o.targetEnvironment.Region)
Expand All @@ -324,8 +332,9 @@ func (o *deployJobOpts) runtimeConfig(addonsURL string) (*stack.RuntimeConfig, e
ImageTag: o.imageTag,
Digest: o.imageDigest,
},
AddonsTemplateURL: addonsURL,
AdditionalTags: tags.Merge(o.targetApp.Tags, o.resourceTags),
AddonsTemplateURL: addonsURL,
AdditionalTags: tags.Merge(o.targetApp.Tags, o.resourceTags),
LegacyServiceDiscovery: svcDiscovery,
}, nil
}

Expand Down
14 changes: 6 additions & 8 deletions internal/pkg/cli/job_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import (
"io/ioutil"
"os"

"github.com/aws/copilot-cli/internal/pkg/aws/sessions"
"github.com/aws/copilot-cli/internal/pkg/exec"

"github.com/aws/copilot-cli/internal/pkg/aws/sessions"
"github.com/aws/copilot-cli/internal/pkg/config"
"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation"
"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack"
"github.com/aws/copilot-cli/internal/pkg/manifest"
"github.com/aws/copilot-cli/internal/pkg/term/prompt"
Expand Down Expand Up @@ -60,11 +59,7 @@ func newPackageJobOpts(vars packageJobVars) (*packageJobOpts, error) {
if err != nil {
return nil, fmt.Errorf("connect to config store: %w", err)
}
p := sessions.NewProvider()
sess, err := p.Default()
if err != nil {
return nil, fmt.Errorf("retrieve default session: %w", err)
}

prompter := prompt.New()
opts := &packageJobOpts{
packageJobVars: vars,
Expand Down Expand Up @@ -98,12 +93,15 @@ func newPackageJobOpts(vars packageJobVars) (*packageJobOpts, error) {
initAddonsClient: initPackageAddonsClient,
ws: ws,
store: o.store,
appCFN: cloudformation.New(sess),
stackWriter: os.Stdout,
paramsWriter: ioutil.Discard,
addonsWriter: ioutil.Discard,
fs: &afero.Afero{Fs: afero.NewOsFs()},
stackSerializer: o.stackSerializer,
configure: func(o *packageSvcOpts) error {
return o.configureClients()
},
sessProvider: sessions.NewProvider(),
}
}
return opts, nil
Expand Down
38 changes: 38 additions & 0 deletions internal/pkg/cli/mocks/mock_interfaces.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 15 additions & 2 deletions internal/pkg/cli/svc_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type deploySvcOpts struct {
addons templater
appCFN appResourcesGetter
svcCFN cloudformation.CloudFormation
envCFN envCFDescriber
sessProvider sessionProvider
envUpgradeCmd actionCommand
appVersionGetter versionGetter
Expand Down Expand Up @@ -259,6 +260,8 @@ func (o *deploySvcOpts) configureClients() error {
// CF client against env account profile AND target environment region
o.svcCFN = cloudformation.New(envSession)

o.envCFN = cloudformation.New(envSession)

addonsSvc, err := addon.New(o.name)
if err != nil {
return fmt.Errorf("initiate addons service: %w", err)
Expand Down Expand Up @@ -378,11 +381,20 @@ func (o *deploySvcOpts) manifest() (interface{}, error) {
return mft, nil
}

func envUsesLegacySvcDiscovery(cf envCFDescriber, app, env string) (bool, error) {
return cf.EnvironmentUsesLegacySvcDiscovery(app, env)
}

func (o *deploySvcOpts) runtimeConfig(addonsURL string) (*stack.RuntimeConfig, error) {
svcDiscovery, err := envUsesLegacySvcDiscovery(o.envCFN, o.appName, o.envName)
if err != nil {
return nil, err
}
if !o.buildRequired {
return &stack.RuntimeConfig{
AddonsTemplateURL: addonsURL,
AdditionalTags: tags.Merge(o.targetApp.Tags, o.resourceTags),
AddonsTemplateURL: addonsURL,
AdditionalTags: tags.Merge(o.targetApp.Tags, o.resourceTags),
LegacyServiceDiscovery: svcDiscovery,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this fits under RuntimeConfig. The original idea behind this struct was to capture additional configuration passed by the client when calling deploy: 1) the url for addons/ template 2) if they use the deploy --resource-tags flag.

Maybe we can push down the LegacyServiceDiscovery one level below into the stack pkg? so that the clients have to do less work

}, nil
}
resources, err := o.appCFN.GetAppResourcesByRegion(o.targetApp, o.targetEnvironment.Region)
Expand All @@ -405,6 +417,7 @@ func (o *deploySvcOpts) runtimeConfig(addonsURL string) (*stack.RuntimeConfig, e
ImageTag: o.imageTag,
Digest: o.imageDigest,
},
LegacyServiceDiscovery: svcDiscovery,
}, nil
}

Expand Down
48 changes: 44 additions & 4 deletions internal/pkg/cli/svc_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ func TestSvcDeployOpts_pushAddonsTemplateToS3Bucket(t *testing.T) {
mockAppResourcesGetter func(m *mocks.MockappResourcesGetter)
mockS3Svc func(m *mocks.MockartifactUploader)
mockAddons func(m *mocks.Mocktemplater)
mockEnvCFDescriber func(m *mocks.MockenvCFDescriber)

wantPath string
wantErr error
Expand Down Expand Up @@ -404,9 +405,9 @@ func TestSvcDeployOpts_pushAddonsTemplateToS3Bucket(t *testing.T) {
mockAddons: func(m *mocks.Mocktemplater) {
m.EXPECT().Template().Return("some data", nil)
},
mockS3Svc: func(m *mocks.MockartifactUploader) {},

wantErr: fmt.Errorf("get app resources: some error"),
mockS3Svc: func(m *mocks.MockartifactUploader) {},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {},
wantErr: fmt.Errorf("get app resources: some error"),
},
"should return error if fail to upload to S3 bucket": {
inputSvc: "mockSvc",
Expand Down Expand Up @@ -517,6 +518,7 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
mockWorkspace func(m *mocks.MockwsSvcDirReader)
mockAppResourcesGetter func(m *mocks.MockappResourcesGetter)
mockAppVersionGetter func(m *mocks.MockversionGetter)
mockEnvCFDescriber func(m *mocks.MockenvCFDescriber)

wantErr error
}{
Expand All @@ -526,6 +528,7 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
},
mockAppResourcesGetter: func(m *mocks.MockappResourcesGetter) {},
mockAppVersionGetter: func(m *mocks.MockversionGetter) {},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {},
wantErr: fmt.Errorf("read service %s manifest file: %w", mockSvcName, mockError),
},
"fail to get app resources": {
Expand All @@ -545,10 +548,32 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
Name: mockAppName,
}, "us-west-2").Return(nil, mockError)
},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {
m.EXPECT().EnvironmentUsesLegacySvcDiscovery(mockAppName, mockEnvName).Return(true, nil)
},
mockAppVersionGetter: func(m *mocks.MockversionGetter) {},
wantErr: fmt.Errorf("get application %s resources from region us-west-2: %w", mockAppName, mockError),
},
"cannot to find ECR repo": {
"fail to get environment": {
inBuildRequire: true,
inEnvironment: &config.Environment{
Name: mockEnvName,
Region: "us-west-2",
},
inApp: &config.Application{
Name: mockAppName,
},
mockWorkspace: func(m *mocks.MockwsSvcDirReader) {
m.EXPECT().ReadServiceManifest(mockSvcName).Return([]byte{}, nil)
},
mockAppResourcesGetter: func(m *mocks.MockappResourcesGetter) {},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {
m.EXPECT().EnvironmentUsesLegacySvcDiscovery(mockAppName, mockEnvName).Return(false, errors.New("some error"))
},
mockAppVersionGetter: func(m *mocks.MockversionGetter) {},
wantErr: errors.New("some error"),
},
"cannot find ECR repo": {
inBuildRequire: true,
inEnvironment: &config.Environment{
Name: mockEnvName,
Expand All @@ -569,6 +594,9 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
RepositoryURLs: map[string]string{},
}, nil)
},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {
m.EXPECT().EnvironmentUsesLegacySvcDiscovery(mockAppName, mockEnvName).Return(true, nil)
},
mockAppVersionGetter: func(m *mocks.MockversionGetter) {},
wantErr: fmt.Errorf("ECR repository not found for service mockSvc in region us-west-2 and account 1234567890"),
},
Expand All @@ -589,6 +617,9 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
mockAppVersionGetter: func(m *mocks.MockversionGetter) {
m.EXPECT().Version().Return("", mockError)
},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {
m.EXPECT().EnvironmentUsesLegacySvcDiscovery(mockAppName, mockEnvName).Return(false, nil)
},
wantErr: fmt.Errorf("get version for app %s: %w", mockAppName, mockError),
},
"fail to enable https alias because of incompatible app version": {
Expand All @@ -608,6 +639,9 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
mockAppVersionGetter: func(m *mocks.MockversionGetter) {
m.EXPECT().Version().Return("v0.0.0", nil)
},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {
m.EXPECT().EnvironmentUsesLegacySvcDiscovery(mockAppName, mockEnvName).Return(false, nil)
},
wantErr: fmt.Errorf(`enable "http.alias": the application version should be at least %s`, deploy.AliasLeastAppTemplateVersion),
},
"success": {
Expand All @@ -622,6 +656,9 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
mockWorkspace: func(m *mocks.MockwsSvcDirReader) {
m.EXPECT().ReadServiceManifest(mockSvcName).Return([]byte{}, nil)
},
mockEnvCFDescriber: func(m *mocks.MockenvCFDescriber) {
m.EXPECT().EnvironmentUsesLegacySvcDiscovery(mockAppName, mockEnvName).Return(false, nil)
},
mockAppResourcesGetter: func(m *mocks.MockappResourcesGetter) {},
mockAppVersionGetter: func(m *mocks.MockversionGetter) {},
},
Expand All @@ -634,10 +671,12 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {

mockWorkspace := mocks.NewMockwsSvcDirReader(ctrl)
mockAppResourcesGetter := mocks.NewMockappResourcesGetter(ctrl)
mockEnvCFDescriber := mocks.NewMockenvCFDescriber(ctrl)
mockAppVersionGetter := mocks.NewMockversionGetter(ctrl)
tc.mockWorkspace(mockWorkspace)
tc.mockAppResourcesGetter(mockAppResourcesGetter)
tc.mockAppVersionGetter(mockAppVersionGetter)
tc.mockEnvCFDescriber(mockEnvCFDescriber)

opts := deploySvcOpts{
deployWkldVars: deployWkldVars{
Expand All @@ -648,6 +687,7 @@ func TestSvcDeployOpts_stackConfiguration(t *testing.T) {
ws: mockWorkspace,
buildRequired: tc.inBuildRequire,
appCFN: mockAppResourcesGetter,
envCFN: mockEnvCFDescriber,
appVersionGetter: mockAppVersionGetter,
targetApp: tc.inApp,
targetEnvironment: tc.inEnvironment,
Expand Down
Loading