fix: custom resource upload should be namespaced by env

internal/pkg/aws/s3/s3.go

@@ -5,8 +5,6 @@
 package s3
 
 import (
-	"archive/zip"
-	"bytes"
 	"fmt"
 	"io"
 	"strings"
@@ -56,28 +54,6 @@ func New(s *session.Session) *S3 {
 	}
 }
 
-// ZipAndUpload zips all files and uploads the zipped file to an S3 bucket under the specified key.
-// Per s3's recommendation https://docs.aws.amazon.com/AmazonS3/latest/userguide/about-object-ownership.html:
-// The bucket owner, in addition to the object owner, is granted full control.
-func (s *S3) ZipAndUpload(bucket, key string, files ...NamedBinary) (string, error) {
-	buf := new(bytes.Buffer)
-	w := zip.NewWriter(buf)
-	for _, file := range files {
-		f, err := w.Create(file.Name())
-		if err != nil {
-			return "", fmt.Errorf("create zip file %s: %w", file.Name(), err)
-		}
-		_, err = f.Write(file.Content())
-		if err != nil {
-			return "", fmt.Errorf("write zip file %s: %w", file.Name(), err)
-		}
-	}
-	if err := w.Close(); err != nil {
-		return "", err
-	}
-	return s.upload(bucket, key, buf)
-}
-
 // Upload uploads a file to an S3 bucket under the specified key.
 // Per s3's recommendation https://docs.aws.amazon.com/AmazonS3/latest/userguide/about-object-ownership.html:
 // The bucket owner, in addition to the object owner, is granted full control.

internal/pkg/aws/s3/s3_test.go

@@ -4,11 +4,9 @@
 package s3
 
 import (
-	"archive/zip"
 	"bytes"
 	"errors"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"testing"
 
@@ -22,77 +20,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestS3_ZipAndUpload(t *testing.T) {
-	testCases := map[string]struct {
-		mockS3ManagerClient func(m *mocks.Mocks3ManagerAPI)
-
-		wantedURL string
-		wantError error
-	}{
-		"return error if upload fails": {
-			mockS3ManagerClient: func(m *mocks.Mocks3ManagerAPI) {
-				m.EXPECT().Upload(gomock.Any()).Do(func(in *s3manager.UploadInput, _ ...func(*s3manager.Uploader)) {
-					require.Equal(t, aws.StringValue(in.Bucket), "mockBucket")
-					require.Equal(t, aws.StringValue(in.Key), "mockFileName")
-				}).Return(nil, errors.New("some error"))
-			},
-			wantError: fmt.Errorf("upload mockFileName to bucket mockBucket: some error"),
-		},
-		"should upload to the s3 bucket": {
-			mockS3ManagerClient: func(m *mocks.Mocks3ManagerAPI) {
-				m.EXPECT().Upload(gomock.Any()).Do(func(in *s3manager.UploadInput, _ ...func(*s3manager.Uploader)) {
-					b, err := ioutil.ReadAll(in.Body)
-					require.NoError(t, err)
-					reader, err := zip.NewReader(bytes.NewReader(b), int64(len(b)))
-					require.NoError(t, err)
-					for _, f := range reader.File {
-						require.Equal(t, f.Name, "foo")
-						rc, err := f.Open()
-						require.NoError(t, err)
-						buf := &bytes.Buffer{}
-						_, err = io.CopyN(buf, rc, 3)
-						require.NoError(t, err)
-						require.Equal(t, buf.String(), "bar")
-						rc.Close()
-						fmt.Println()
-					}
-					require.Equal(t, aws.StringValue(in.Bucket), "mockBucket")
-					require.Equal(t, aws.StringValue(in.Key), "mockFileName")
-					require.Equal(t, s3.ObjectCannedACLBucketOwnerFullControl, aws.StringValue(in.ACL))
-				}).Return(&s3manager.UploadOutput{
-					Location: "mockURL",
-				}, nil)
-			},
-			wantedURL: "mockURL",
-		},
-	}
-
-	for name, tc := range testCases {
-		t.Run(name, func(t *testing.T) {
-			// GIVEN
-			ctrl := gomock.NewController(t)
-			defer ctrl.Finish()
-
-			mockS3ManagerClient := mocks.NewMocks3ManagerAPI(ctrl)
-			tc.mockS3ManagerClient(mockS3ManagerClient)
-
-			service := S3{
-				s3Manager: mockS3ManagerClient,
-			}
-
-			gotURL, gotErr := service.ZipAndUpload("mockBucket", "mockFileName", namedBinary{})
-
-			if gotErr != nil {
-				require.EqualError(t, gotErr, tc.wantError.Error())
-			} else {
-				require.Equal(t, gotErr, nil)
-				require.Equal(t, gotURL, tc.wantedURL)
-			}
-		})
-
-	}
-}
-
 func TestS3_Upload(t *testing.T) {
 	testCases := map[string]struct {
 		mockS3ManagerClient func(m *mocks.Mocks3ManagerAPI)
@@ -151,12 +78,6 @@ func TestS3_Upload(t *testing.T) {
 	}
 }
 
-type namedBinary struct{}
-
-func (n namedBinary) Name() string { return "foo" }
-
-func (n namedBinary) Content() []byte { return []byte("bar") }
-
 func TestS3_EmptyBucket(t *testing.T) {
 	batchObject1 := make([]*s3.ObjectVersion, 1000)
 	batchObject2 := make([]*s3.ObjectVersion, 10)

internal/pkg/cli/deploy/env.go

@@ -89,7 +89,7 @@ func NewEnvDeployer(in *NewEnvDeployerInput) (*envDeployer, error) {
 		env: in.Env,
 
 		templateFS:       template.New(),
-		s3:               s3.New(envRegionSession),
+		s3:               s3.New(envManagerSession),
 		prefixListGetter: ec2.New(envRegionSession),
 
 		appCFN:      deploycfn.New(defaultSession, deploycfn.WithProgressTracker(os.Stderr)),
@@ -120,7 +120,7 @@ func (d *envDeployer) UploadArtifacts() (map[string]string, error) {
 func (d *envDeployer) uploadCustomResources(bucket string) (map[string]string, error) {
 	crs, err := customresource.Env(d.templateFS)
 	if err != nil {
-		return nil, fmt.Errorf("read custom resources for environments: %w", err)
+		return nil, fmt.Errorf("read custom resources for environment %s: %w", d.env.Name, err)
 	}
 	urls, err := customresource.Upload(func(key string, dat io.Reader) (url string, err error) {
 		return d.s3.Upload(bucket, key, dat)

internal/pkg/cli/deploy/env_test.go

@@ -79,7 +79,7 @@ func TestEnvDeployer_UploadArtifacts(t *testing.T) {
 				require.NoError(t, err)
 				m.s3.EXPECT().Upload("mockS3Bucket", gomock.Any(), gomock.Any()).DoAndReturn(func(_, key string, _ io.Reader) (url string, err error) {
 					for _, cr := range crs {
-						if strings.Contains(key, strings.ToLower(cr.FunctionName())) {
+						if strings.Contains(key, strings.ToLower(cr.Name())) {
 							return "", nil
 						}
 					}

internal/pkg/cli/deploy/svc.go

@@ -97,7 +97,6 @@ type imageBuilderPusher interface {
 
 type uploader interface {
 	Upload(bucket, key string, data io.Reader) (string, error)
-	ZipAndUpload(bucket, key string, files ...s3.NamedBinary) (string, error)
 }
 
 type templater interface {
@@ -208,9 +207,6 @@ func newWorkloadDeployer(in *WorkloadDeployerInput) (*workloadDeployer, error) {
 	if err != nil {
 		return nil, fmt.Errorf("create default: %w", err)
 	}
-	if err != nil {
-		return nil, fmt.Errorf("create env session with region %s: %w", in.Env.Region, err)
-	}
 	envSession, err := in.SessionProvider.FromRole(in.Env.ManagerRoleARN, in.Env.Region)
 	if err != nil {
 		return nil, fmt.Errorf("create env session with region %s: %w", in.Env.Region, err)

internal/pkg/cli/deploy/svc_test.go

@@ -167,7 +167,7 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 				require.NoError(t, err)
 				m.mockUploader.EXPECT().Upload(mockS3Bucket, gomock.Any(), gomock.Any()).DoAndReturn(func(_, key string, _ io.Reader) (url string, err error) {
 					for _, cr := range crs {
-						if strings.Contains(key, strings.ToLower(cr.FunctionName())) {
+						if strings.Contains(key, strings.ToLower(cr.Name())) {
 							return "", nil
 						}
 					}
@@ -179,7 +179,9 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 					svcDeployer: &svcDeployer{
 						workloadDeployer: deployer,
 					},
-					customResources: customresource.LBWS,
+					customResources: func(fs template.Reader) ([]*customresource.CustomResource, error) {
+						return customresource.LBWS(fs)
+					},
 				}
 			},
 		},
@@ -193,7 +195,7 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 				require.NoError(t, err)
 				m.mockUploader.EXPECT().Upload(mockS3Bucket, gomock.Any(), gomock.Any()).DoAndReturn(func(_, key string, _ io.Reader) (url string, err error) {
 					for _, cr := range crs {
-						if strings.Contains(key, strings.ToLower(cr.FunctionName())) {
+						if strings.Contains(key, strings.ToLower(cr.Name())) {
 							return "", nil
 						}
 					}
@@ -205,7 +207,9 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 					svcDeployer: &svcDeployer{
 						workloadDeployer: deployer,
 					},
-					customResources: customresource.Backend,
+					customResources: func(fs template.Reader) ([]*customresource.CustomResource, error) {
+						return customresource.Backend(fs)
+					},
 				}
 			},
 		},
@@ -219,7 +223,7 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 				require.NoError(t, err)
 				m.mockUploader.EXPECT().Upload(mockS3Bucket, gomock.Any(), gomock.Any()).DoAndReturn(func(_, key string, _ io.Reader) (url string, err error) {
 					for _, cr := range crs {
-						if strings.Contains(key, strings.ToLower(cr.FunctionName())) {
+						if strings.Contains(key, strings.ToLower(cr.Name())) {
 							return "", nil
 						}
 					}
@@ -231,7 +235,9 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 					svcDeployer: &svcDeployer{
 						workloadDeployer: deployer,
 					},
-					customResources: customresource.Worker,
+					customResources: func(fs template.Reader) ([]*customresource.CustomResource, error) {
+						return customresource.Worker(fs)
+					},
 				}
 			},
 		},
@@ -245,7 +251,7 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 				require.NoError(t, err)
 				m.mockUploader.EXPECT().Upload(mockS3Bucket, gomock.Any(), gomock.Any()).DoAndReturn(func(_, key string, _ io.Reader) (url string, err error) {
 					for _, cr := range crs {
-						if strings.Contains(key, strings.ToLower(cr.FunctionName())) {
+						if strings.Contains(key, strings.ToLower(cr.Name())) {
 							return "", nil
 						}
 					}
@@ -257,7 +263,9 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 					svcDeployer: &svcDeployer{
 						workloadDeployer: deployer,
 					},
-					customResources: customresource.RDWS,
+					customResources: func(fs template.Reader) ([]*customresource.CustomResource, error) {
+						return customresource.RDWS(fs)
+					},
 				}
 			},
 		},
@@ -271,7 +279,7 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 				require.NoError(t, err)
 				m.mockUploader.EXPECT().Upload(mockS3Bucket, gomock.Any(), gomock.Any()).DoAndReturn(func(_, key string, _ io.Reader) (url string, err error) {
 					for _, cr := range crs {
-						if strings.Contains(key, strings.ToLower(cr.FunctionName())) {
+						if strings.Contains(key, strings.ToLower(cr.Name())) {
 							return "", nil
 						}
 					}
@@ -281,7 +289,9 @@ func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
 			mockServiceDeployer: func(deployer *workloadDeployer) artifactsUploader {
 				return &jobDeployer{
 					workloadDeployer: deployer,
-					customResources:  customresource.ScheduledJob,
+					customResources: func(fs template.Reader) ([]*customresource.CustomResource, error) {
+						return customresource.ScheduledJob(fs)
+					},
 				}
 			},
 		},

internal/pkg/deploy/upload/customresource/customresource.go

@@ -63,14 +63,13 @@ type CustomResource struct {
 	zip *bytes.Buffer
 }
 
-// FunctionName is the name of the Lambda function.
-func (cr *CustomResource) FunctionName() string {
+// Name returns the name of the custom resource.
+func (cr *CustomResource) Name() string {
 	return cr.name
 }
 
-// ArtifactPath returns the S3 key for the zipped custom resource object.
-func (cr *CustomResource) ArtifactPath() string {
-	return artifactpath.CustomResource(strings.ToLower(cr.FunctionName()), cr.zip.Bytes())
+func (cr *CustomResource) artifactPath() string {
+	return artifactpath.CustomResource(strings.ToLower(cr.Name()), cr.zip.Bytes())
 }
 
 // zipReader returns a reader for the zip archive from all the files in the custom resource.
@@ -84,15 +83,15 @@ func (cr *CustomResource) init() error {
 	for _, file := range cr.files {
 		f, err := w.Create(file.name)
 		if err != nil {
-			return fmt.Errorf("create zip file %q for custom resource %q: %v", file.name, cr.FunctionName(), err)
+			return fmt.Errorf("create zip file %q for custom resource %q: %v", file.name, cr.Name(), err)
 		}
 		_, err = f.Write(file.content)
 		if err != nil {
-			return fmt.Errorf("write zip file %q for custom resource %q: %v", file.name, cr.FunctionName(), err)
+			return fmt.Errorf("write zip file %q for custom resource %q: %v", file.name, cr.Name(), err)
 		}
 	}
 	if err := w.Close(); err != nil {
-		return fmt.Errorf("close zip file for custom resource %q: %v", cr.FunctionName(), err)
+		return fmt.Errorf("close zip file for custom resource %q: %v", cr.Name(), err)
 	}
 	cr.zip = buf
 	return nil
@@ -166,11 +165,11 @@ type UploadFunc func(key string, contents io.Reader) (url string, err error)
 func Upload(upload UploadFunc, crs []*CustomResource) (map[string]string, error) {
 	urls := make(map[string]string)
 	for _, cr := range crs {
-		url, err := upload(cr.ArtifactPath(), cr.zipReader())
+		url, err := upload(cr.artifactPath(), cr.zipReader())
 		if err != nil {
-			return nil, fmt.Errorf("upload custom resource %q: %w", cr.FunctionName(), err)
+			return nil, fmt.Errorf("upload custom resource %q: %w", cr.Name(), err)
 		}
-		urls[cr.FunctionName()] = url
+		urls[cr.Name()] = url
 	}
 	return urls, nil
 }