chore(session): try retrieving credentials within 10s if not err (#3645)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

Author: paragbhingre

Makefile

@@ -144,6 +144,7 @@ site-local:
 gen-mocks: tools
 	GOBIN=${GOBIN} go get github.com/golang/mock/mockgen
 	# TODO: make this more extensible?
+	${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/aws/sessions/mocks/mock_sessions.go -source=./internal/pkg/aws/sessions/sessions.go
 	${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/cli/mocks/mock_rg.go -source=./internal/pkg/cli/env_delete.go resourceGetter
 	${GOBIN}/mockgen -source=./internal/pkg/term/progress/spinner.go -package=mocks -destination=./internal/pkg/term/progress/mocks/mock_spinner.go
 	${GOBIN}/mockgen -source=./internal/pkg/term/progress/render.go -package=mocks -destination=./internal/pkg/term/progress/mocks/mock_render.go

internal/pkg/aws/sessions/errors.go

@@ -5,6 +5,7 @@ package sessions
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/aws/copilot-cli/internal/pkg/term/color"
 )
@@ -24,3 +25,33 @@ func (e *errMissingRegion) RecommendActions() string { // implements new actionR
 - Alternatively, you can run %s to set the environment variable.
 More information: https://aws.github.io/copilot-cli/docs/credentials/`, color.HighlightCode("export AWS_REGION=<application region>"))
 }
+
+type errCredRetrieval struct {
+	profile   string
+	parentErr error
+}
+
+// Implements error interface.
+func (e *errCredRetrieval) Error() string {
+	return e.parentErr.Error()
+}
+
+// RecommendActions returns recommended actions to be taken after the error.
+// Implements main.actionRecommender interface.
+func (e *errCredRetrieval) RecommendActions() string {
+	notice := "It looks like your credential settings are misconfigured or missing"
+	if e.profile != "" {
+		notice = fmt.Sprintf("It looks like your profile [%s] is misconfigured or missing", e.profile)
+	}
+	return fmt.Sprintf(`%s:
+https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html#specifying-credentials
+- We recommend including your credentials in the shared credentials file.
+- Alternatively, you can also set credentials through 
+	* Environment Variables
+	* EC2 Instance Metadata (credentials only)
+More information: https://aws.github.io/copilot-cli/docs/credentials/`, notice)
+}
+
+func isCredRetrievalErr(err error) bool {
+	return strings.Contains(err.Error(), "context deadline exceeded") || strings.Contains(err.Error(), "NoCredentialProviders")
+}

internal/pkg/aws/sessions/mocks/mock_sessions.go

@@ -39,7 +39,12 @@ type Provider struct {
 	defaultSess *session.Session
 
 	// Metadata associated with the provider.
-	userAgentExtras []string
+	userAgentExtras  []string
+	sessionValidator sessionValidator
+}
+
+type sessionValidator interface {
+	ValidateCredentials(sess *session.Session) (credentials.Value, error)
 }
 
 var instance *Provider
@@ -48,7 +53,9 @@ var once sync.Once
 // ImmutableProvider returns an immutable session Provider with the options applied.
 func ImmutableProvider(options ...func(*Provider)) *Provider {
 	once.Do(func() {
-		instance = &Provider{}
+		instance = &Provider{
+			sessionValidator: &validator{},
+		}
 		for _, option := range options {
 			option(instance)
 		}
@@ -104,6 +111,12 @@ func (p *Provider) FromProfile(name string) (*session.Session, error) {
 	if aws.StringValue(sess.Config.Region) == "" {
 		return nil, &errMissingRegion{}
 	}
+	if _, err := p.sessionValidator.ValidateCredentials(sess); err != nil {
+		if isCredRetrievalErr(err) {
+			return nil, &errCredRetrieval{profile: name, parentErr: err}
+		}
+		return nil, err
+	}
 	sess.Handlers.Build.PushBackNamed(p.userAgentHandler())
 	return sess, nil
 }
@@ -155,6 +168,13 @@ func (p *Provider) defaultSession() (*session.Session, error) {
 	if err != nil {
 		return nil, err
 	}
+	if _, err = p.sessionValidator.ValidateCredentials(sess); err != nil {
+		if isCredRetrievalErr(err) {
+			return nil, &errCredRetrieval{parentErr: err}
+		}
+		return nil, err
+	}
+
 	sess.Handlers.Build.PushBackNamed(p.userAgentHandler())
 	p.defaultSess = sess
 	return sess, nil
@@ -202,3 +222,11 @@ func (p *Provider) userAgentHandler() request.NamedHandler {
 		Fn:   request.MakeAddToUserAgentHandler(userAgentProductName, version.Version, extras...),
 	}
 }
+
+type validator struct{}
+
+func (v *validator) ValidateCredentials(sess *session.Session) (credentials.Value, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), credsTimeout)
+	defer cancel()
+	return sess.Config.Credentials.GetWithContext(ctx)
+}

internal/pkg/aws/sessions/sessions.go

@@ -4,7 +4,10 @@
 package sessions
 
 import (
+	"context"
 	"errors"
+	"github.com/aws/copilot-cli/internal/pkg/aws/sessions/mocks"
+	"github.com/golang/mock/gomock"
 	"os"
 	"testing"
 
@@ -171,6 +174,13 @@ func TestProvider_FromProfile(t *testing.T) {
 	})
 
 	t.Run("region information present", func(t *testing.T) {
+
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		m := mocks.NewMocksessionValidator(ctrl)
+		m.EXPECT().ValidateCredentials(gomock.Any()).Return(credentials.Value{}, nil)
+
 		ogRegion := os.Getenv("AWS_REGION")
 		defer func() {
 			err := restoreEnvVar("AWS_REGION", ogRegion)
@@ -183,12 +193,47 @@ func TestProvider_FromProfile(t *testing.T) {
 		require.NoError(t, err)
 
 		// WHEN
-		sess, err := ImmutableProvider().FromProfile("walk-like-an-egyptian")
+		provider := &Provider{
+			sessionValidator: m,
+		}
+
+		sess, err := provider.FromProfile("walk-like-an-egyptian")
 
 		// THEN
 		require.NoError(t, err)
 		require.Equal(t, "us-west-2", *sess.Config.Region)
 	})
+
+	t.Run("session credentials are incorrect", func(t *testing.T) {
+
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		m := mocks.NewMocksessionValidator(ctrl)
+		m.EXPECT().ValidateCredentials(gomock.Any()).Return(credentials.Value{}, context.DeadlineExceeded)
+
+		ogRegion := os.Getenv("AWS_REGION")
+		defer func() {
+			err := restoreEnvVar("AWS_REGION", ogRegion)
+			require.NoError(t, err)
+		}()
+
+		// Since "walk-like-an-egyptian" is (very likely) a non-existent profile, whether the region information
+		// is missing depends on whether the `AWS_REGION` environment variable is set.
+		err := os.Setenv("AWS_REGION", "us-west-2")
+		require.NoError(t, err)
+
+		// WHEN
+		provider := &Provider{
+			sessionValidator: m,
+		}
+
+		sess, err := provider.FromProfile("walk-like-an-egyptian")
+
+		// THEN
+		require.EqualError(t, err, "context deadline exceeded")
+		require.Nil(t, sess)
+	})
 }
 
 func restoreEnvVar(key string, originalValue string) error {

internal/pkg/aws/sessions/sessions_test.go

@@ -6,11 +6,11 @@ package cli
 import (
 	"errors"
 	"fmt"
+	"github.com/aws/aws-sdk-go/aws"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws/session"
 
-	"github.com/aws/copilot-cli/internal/pkg/aws/sessions"
 	"github.com/aws/copilot-cli/internal/pkg/cli/mocks"
 	"github.com/aws/copilot-cli/internal/pkg/config"
 	"github.com/aws/copilot-cli/internal/pkg/term/log"
@@ -292,7 +292,7 @@ func TestDeleteJobOpts_Ask(t *testing.T) {
 type deleteJobMocks struct {
 	store          *mocks.Mockstore
 	secretsmanager *mocks.MocksecretsManager
-	sessProvider   *sessions.Provider
+	sessProvider   *mocks.MocksessionProvider
 	appCFN         *mocks.MockjobRemoverFromApp
 	spinner        *mocks.Mockprogress
 	jobCFN         *mocks.MockwlDeleter
@@ -334,6 +334,8 @@ func TestDeleteJobOpts_Execute(t *testing.T) {
 				gomock.InOrder(
 					// appEnvironments
 					mocks.store.EXPECT().ListEnvironments(gomock.Eq(mockAppName)).Times(1).Return(mockEnvs, nil),
+
+					mocks.sessProvider.EXPECT().FromRole(gomock.Any(), gomock.Any()).Return(&session.Session{}, nil),
 					// deleteStacks
 					mocks.spinner.EXPECT().Start(fmt.Sprintf(fmtJobStackDeleteStart, mockJobName, mockEnvName)),
 					mocks.jobCFN.EXPECT().DeleteWorkload(gomock.Any()).Return(nil),
@@ -342,9 +344,10 @@ func TestDeleteJobOpts_Execute(t *testing.T) {
 					mocks.spinner.EXPECT().Start(fmt.Sprintf(fmtJobTasksStopStart, mockJobName, mockEnvName)),
 					mocks.ecs.EXPECT().StopWorkloadTasks(mockAppName, mockEnvName, mockJobName).Return(nil),
 					mocks.spinner.EXPECT().Stop(log.Ssuccessf(fmtJobTasksStopComplete, mockJobName, mockEnvName)),
+
+					mocks.sessProvider.EXPECT().DefaultWithRegion(gomock.Any()).Return(&session.Session{}, nil),
 					// emptyECRRepos
 					mocks.ecr.EXPECT().ClearRepository(mockRepo).Return(nil),
-
 					// removeJobFromApp
 					mocks.store.EXPECT().GetApplication(mockAppName).Return(mockApp, nil),
 					mocks.spinner.EXPECT().Start(fmt.Sprintf(fmtJobDeleteResourcesStart, mockJobName, mockAppName)),
@@ -368,6 +371,7 @@ func TestDeleteJobOpts_Execute(t *testing.T) {
 				gomock.InOrder(
 					// appEnvironments
 					mocks.store.EXPECT().GetEnvironment(mockAppName, mockEnvName).Times(1).Return(mockEnv, nil),
+					mocks.sessProvider.EXPECT().FromRole(gomock.Any(), gomock.Any()).Return(&session.Session{}, nil),
 					// deleteStacks
 					mocks.spinner.EXPECT().Start(fmt.Sprintf(fmtJobStackDeleteStart, mockJobName, mockEnvName)),
 					mocks.jobCFN.EXPECT().DeleteWorkload(gomock.Any()).Return(nil),
@@ -397,6 +401,11 @@ func TestDeleteJobOpts_Execute(t *testing.T) {
 				gomock.InOrder(
 					// appEnvironments
 					mocks.store.EXPECT().GetEnvironment(mockAppName, mockEnvName).Times(1).Return(mockEnv, nil),
+					mocks.sessProvider.EXPECT().FromRole(gomock.Any(), gomock.Any()).Return(&session.Session{
+						Config: &aws.Config{
+							Region: aws.String("mockRegion"),
+						},
+					}, nil),
 					// deleteStacks
 					mocks.spinner.EXPECT().Start(fmt.Sprintf(fmtJobStackDeleteStart, mockJobName, mockEnvName)),
 					mocks.jobCFN.EXPECT().DeleteWorkload(gomock.Any()).Return(testError),
@@ -413,6 +422,11 @@ func TestDeleteJobOpts_Execute(t *testing.T) {
 				gomock.InOrder(
 					// appEnvironments
 					mocks.store.EXPECT().GetEnvironment(mockAppName, mockEnvName).Times(1).Return(mockEnv, nil),
+					mocks.sessProvider.EXPECT().FromRole(gomock.Any(), gomock.Any()).Return(&session.Session{
+						Config: &aws.Config{
+							Region: aws.String("mockRegion"),
+						},
+					}, nil),
 					// deleteStacks
 					mocks.spinner.EXPECT().Start(fmt.Sprintf(fmtJobStackDeleteStart, mockJobName, mockEnvName)),
 					mocks.jobCFN.EXPECT().DeleteWorkload(gomock.Any()).Return(nil),
@@ -435,7 +449,7 @@ func TestDeleteJobOpts_Execute(t *testing.T) {
 			// GIVEN
 			mockstore := mocks.NewMockstore(ctrl)
 			mockSecretsManager := mocks.NewMocksecretsManager(ctrl)
-			mockSession := sessions.ImmutableProvider()
+			mockSession := mocks.NewMocksessionProvider(ctrl)
 			mockAppCFN := mocks.NewMockjobRemoverFromApp(ctrl)
 			mockJobCFN := mocks.NewMockwlDeleter(ctrl)
 			mockSpinner := mocks.NewMockprogress(ctrl)