fix: support amazon-ecr-credential-helper & check for bucket existence before emptying the bucket (#2365)

### Summary

#2323 - Depending on the value of `credsStore` attribute (inside docker config file), the code either performs the `docker login` or skips it.
#2070 - Added code to check for the existence of the bucket using `s3.HeadBucket` API before proceed to empty the contents

### Issue number
Fixes  #2323 & #2070

----
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Author: hariohmprasath

internal/pkg/aws/s3/mocks/mock_s3.go

@@ -8,6 +8,7 @@ import (
 	"archive/zip"
 	"bytes"
 	"fmt"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"io"
 	"path"
 	"strconv"
@@ -22,6 +23,7 @@ import (
 
 const (
 	artifactDirName = "manual"
+	notFound = "NotFound"
 )
 
 type s3ManagerAPI interface {
@@ -31,6 +33,7 @@ type s3ManagerAPI interface {
 type s3API interface {
 	ListObjectVersions(input *s3.ListObjectVersionsInput) (*s3.ListObjectVersionsOutput, error)
 	DeleteObjects(input *s3.DeleteObjectsInput) (*s3.DeleteObjectsOutput, error)
+	HeadBucket (input *s3.HeadBucketInput) (*s3.HeadBucketOutput, error)
 }
 
 // NamedBinary is a named binary to be uploaded.
@@ -105,6 +108,17 @@ func (s *S3) ZipAndUpload(bucket, key string, files ...NamedBinary) (string, err
 func (s *S3) EmptyBucket(bucket string) error {
 	var listResp *s3.ListObjectVersionsOutput
 	var err error
+
+	// Bucket is exists check to make sure the bucket exists before proceeding in emptying it
+	isExists, err:= s.isBucketExists(bucket)
+	if err!= nil {
+		return fmt.Errorf("unable to determine the existance of bucket %s: %w", bucket, err)
+	}
+
+	if !isExists {
+		return nil
+	}
+
 	listParams := &s3.ListObjectVersionsInput{
 		Bucket: aws.String(bucket),
 	}
@@ -161,3 +175,19 @@ func ParseURL(url string) (bucket string, key string, err error) {
 	bucket, key = strings.Split(parsedURL[0], ".")[0], parsedURL[1]
 	return
 }
+
+// Check whether the bucket exists before proceeding with empty the bucket
+func (s *S3) isBucketExists(bucket string) (bool, error) {
+	input := &s3.HeadBucketInput{
+		Bucket: aws.String(bucket),
+	}
+	_, err := s.s3Client.HeadBucket(input)
+	if err != nil {
+		if aerr, ok := err.(awserr.Error); ok && aerr.Code() == notFound {
+			return false, nil
+		}
+		return false, err
+	}
+
+	return true, nil
+}

internal/pkg/aws/s3/s3.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"io"
 	"io/ioutil"
 	"strconv"
@@ -216,6 +217,9 @@ func TestS3_EmptyBucket(t *testing.T) {
 		"should delete all objects within the bucket": {
 			inBucket: "mockBucket",
 			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, nil)
 				m.EXPECT().ListObjectVersions(&s3.ListObjectVersionsInput{
 					Bucket: aws.String("mockBucket"),
 				}).Return(&s3.ListObjectVersionsOutput{
@@ -235,6 +239,9 @@ func TestS3_EmptyBucket(t *testing.T) {
 		"should batch delete all objects within the bucket": {
 			inBucket: "mockBucket",
 			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, nil)
 				m.EXPECT().ListObjectVersions(&s3.ListObjectVersionsInput{
 					Bucket: aws.String("mockBucket"),
 				}).Return(&s3.ListObjectVersionsOutput{
@@ -266,6 +273,9 @@ func TestS3_EmptyBucket(t *testing.T) {
 		"should delete all objects within the bucket including delete markers": {
 			inBucket: "mockBucket",
 			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, nil)
 				m.EXPECT().ListObjectVersions(&s3.ListObjectVersionsInput{
 					Bucket: aws.String("mockBucket"),
 				}).Return(&s3.ListObjectVersionsOutput{
@@ -286,6 +296,9 @@ func TestS3_EmptyBucket(t *testing.T) {
 		"should wrap up error if fail to list objects": {
 			inBucket: "mockBucket",
 			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, nil)
 				m.EXPECT().ListObjectVersions(&s3.ListObjectVersionsInput{
 					Bucket: aws.String("mockBucket"),
 				}).Return(nil, errors.New("some error"))
@@ -296,6 +309,9 @@ func TestS3_EmptyBucket(t *testing.T) {
 		"should not invoke DeleteObjects if bucket is empty": {
 			inBucket: "mockBucket",
 			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, nil)
 				m.EXPECT().ListObjectVersions(gomock.Any()).Return(&s3.ListObjectVersionsOutput{
 					IsTruncated: aws.Bool(false),
 				}, nil)
@@ -306,6 +322,9 @@ func TestS3_EmptyBucket(t *testing.T) {
 		"should wrap up error if fail to delete objects": {
 			inBucket: "mockBucket",
 			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, nil)
 				m.EXPECT().ListObjectVersions(&s3.ListObjectVersionsInput{
 					Bucket: aws.String("mockBucket"),
 				}).Return(&s3.ListObjectVersionsOutput{
@@ -322,6 +341,27 @@ func TestS3_EmptyBucket(t *testing.T) {
 
 			wantErr: fmt.Errorf("delete objects from bucket mockBucket: some error"),
 		},
+		"should not proceed with deletion as the bucket doesnt exists":{
+			inBucket: "mockBucket",
+			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, awserr.New(notFound, "message", nil))
+			},
+
+			wantErr: nil,
+		},
+		"should throw error while perform bucket exists check":{
+			inBucket: "mockBucket",
+			mockS3Client: func(m *mocks.Mocks3API) {
+				m.EXPECT().HeadBucket(&s3.HeadBucketInput{
+					Bucket: aws.String("mockBucket"),
+				}).Return(nil, awserr.New("Unknown", "message", nil))
+			},
+
+			wantErr: fmt.Errorf("unable to determine the existance of bucket %s: %w", "mockBucket",
+				awserr.New("Unknown", "message", nil)),
+		},
 	}
 
 	for name, tc := range testCases {

internal/pkg/aws/s3/s3_test.go

@@ -8,6 +8,8 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"os/exec"
 	"path/filepath"
 	"sort"
@@ -19,12 +21,14 @@ type DockerCommand struct {
 	runner
 	// Override in unit tests.
 	buf *bytes.Buffer
+	homePath string
 }
 
 // NewDockerCommand returns a DockerCommand.
 func NewDockerCommand() DockerCommand {
 	return DockerCommand{
 		runner: NewCmd(),
+		homePath: userHomeDirectory(),
 	}
 }
 
@@ -39,6 +43,15 @@ type BuildArguments struct {
 	Args       map[string]string // Optional. Build args to pass via `--build-arg` flags. Equivalent to ARG directives in dockerfile.
 }
 
+type dockerConfig struct {
+	CredsStore  string            `json:"credsStore,omitempty"`
+	CredHelpers map[string]string `json:"credHelpers,omitempty"`
+}
+
+const(
+	credStoreECRLogin = "ecr-login" // set on `credStore` attribute in docker configuration file
+)
+
 // Build will run a `docker build` command for the given ecr repo URI and build arguments.
 func (c DockerCommand) Build(in *BuildArguments) error {
 	dfDir := in.Context
@@ -159,3 +172,61 @@ func imageName(uri, tag string) string {
 	}
 	return fmt.Sprintf("%s:%s", uri, tag)
 }
+
+// IsEcrCredentialHelperEnabled return true if ecr-login is enabled either globally or registry level
+func (c DockerCommand) IsEcrCredentialHelperEnabled(uri string) bool {
+	// Make sure the program is able to obtain the home directory
+	splits := strings.Split(uri, "/")
+	if c.homePath == "" || len(splits) == 0 {
+		return false
+	}
+
+	// Look into the default locations
+	pathsToTry := []string{filepath.Join(".docker", "config.json"), ".dockercfg"}
+	for _, path := range pathsToTry {
+		content, err := ioutil.ReadFile(filepath.Join(c.homePath, path))
+		if err != nil {
+			// if we can't read the file keep going
+			continue
+		}
+
+		config, err := parseCredFromDockerConfig(content)
+		if err != nil {
+			continue
+		}
+
+		if config.CredsStore == credStoreECRLogin || config.CredHelpers[splits[0]] == credStoreECRLogin {
+			return true
+		}
+	}
+
+	return false
+}
+
+func parseCredFromDockerConfig(config []byte) (*dockerConfig, error) {
+	/*
+	Sample docker config file
+    {
+        "credsStore" : "ecr-login",
+        "credHelpers": {
+            "dummyaccountId.dkr.ecr.region.amazonaws.com": "ecr-login"
+        }
+    }
+	*/
+	cred := dockerConfig{}
+	err := json.Unmarshal(config, &cred)
+	if err != nil {
+		return nil, err
+	}
+
+	return &cred, nil
+}
+
+func userHomeDirectory() string {
+	home, err := os.UserHomeDir()
+	if err!= nil{
+		return ""
+	}
+
+	return home
+}

internal/pkg/exec/docker.go

@@ -7,7 +7,9 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"github.com/spf13/afero"
 	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -350,3 +352,91 @@ func TestDockerCommand_CheckDockerEngineRunning(t *testing.T) {
 		})
 	}
 }
+
+func TestIsEcrCredentialHelperEnabled(t *testing.T){
+	var mockRunner *Mockrunner
+	workspace := "test/copilot/.docker"
+	registry := "dummyaccountid.dkr.ecr.region.amazonaws.com"
+	uri := fmt.Sprintf("%s/ui/app", registry)
+
+	testCases := map[string]struct {
+		setupMocks func(controller *gomock.Controller)
+		inBuffer   *bytes.Buffer
+		mockFileSystem      func(fs afero.Fs)
+		postExec      func(fs afero.Fs)
+		isEcrRepo bool
+ 	}{
+		"ecr-login check global level": {
+			mockFileSystem: func(fs afero.Fs) {
+				fs.MkdirAll(workspace, 0755)
+				afero.WriteFile(fs, filepath.Join(workspace, "config.json"), []byte(fmt.Sprintf("{\"credsStore\":\"%s\"}", credStoreECRLogin)), 0644)
+			},
+			setupMocks: func(c *gomock.Controller) {
+				mockRunner = NewMockrunner(c)
+			},
+			postExec: func(fs afero.Fs){
+				fs.RemoveAll(workspace)
+			},
+			isEcrRepo: true,
+		},
+		"ecr-login check registry level": {
+			mockFileSystem: func(fs afero.Fs) {
+				fs.MkdirAll(workspace, 0755)
+				afero.WriteFile(fs, filepath.Join(workspace, "config.json"), []byte(fmt.Sprintf("{\"credhelpers\":{\"%s\": \"%s\"}}", registry, credStoreECRLogin)), 0644)
+			},
+			setupMocks: func(c *gomock.Controller) {
+				mockRunner = NewMockrunner(c)
+			},
+			postExec: func(fs afero.Fs){
+				fs.RemoveAll(workspace)
+			},
+			isEcrRepo: true,
+		},
+		"default login check registry level": {
+			mockFileSystem: func(fs afero.Fs) {
+				fs.MkdirAll(workspace, 0755)
+				afero.WriteFile(fs, filepath.Join(workspace, "config.json"), []byte(fmt.Sprintf("{\"credhelpers\":{\"%s\": \"%s\"}}", registry, "desktop")), 0644)
+			},
+			setupMocks: func(c *gomock.Controller) {
+				mockRunner = NewMockrunner(c)
+			},
+			postExec: func(fs afero.Fs){
+				fs.RemoveAll(workspace)
+			},
+			isEcrRepo: false,
+		},
+		"no file check": {
+			mockFileSystem: func(fs afero.Fs) {
+				fs.MkdirAll(workspace, 0755)
+			},
+			setupMocks: func(c *gomock.Controller) {
+				mockRunner = NewMockrunner(c)
+			},
+			postExec: func(fs afero.Fs){
+				fs.RemoveAll(workspace)
+			},
+			isEcrRepo: false,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			// Create an empty FileSystem
+			fs := afero.NewOsFs()
+
+			controller := gomock.NewController(t)
+			tc.setupMocks(controller)
+			tc.mockFileSystem(fs)
+			s := DockerCommand{
+				runner: mockRunner,
+				buf:    tc.inBuffer,
+				homePath: "test/copilot",
+			}
+
+			credStore := s.IsEcrCredentialHelperEnabled(uri)
+			tc.postExec(fs)
+
+			require.Equal(t, tc.isEcrRepo, credStore)
+		})
+	}
+}