diff --git a/internal/errors/error.go b/internal/errors/error.go index 5e94b8f..dfb6d9e 100644 --- a/internal/errors/error.go +++ b/internal/errors/error.go @@ -23,6 +23,11 @@ import ( ) const ( + // AwsErrorAuthFailure is returned by EC2 when credentials exist in IAM + // but have not yet propagated to EC2. This is a transient eventual + // consistency issue that can resolve itself by retrying the request. + AwsErrorAuthFailure = "AuthFailure" + // awsErrorAccessDenied is returned when the credentials do not have // the required permissions to do a specific action on a resource. // This error will persist until the IAM Role Policy is updated by diff --git a/plugin/service/host/plugin.go b/plugin/service/host/plugin.go index 23d1653..c5f491b 100644 --- a/plugin/service/host/plugin.go +++ b/plugin/service/host/plugin.go @@ -8,6 +8,7 @@ import ( goerrors "errors" "fmt" "strings" + "time" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/credentials/stscreds" @@ -828,10 +829,18 @@ func ec2ClientForTarget( return ec2.NewFromConfig(targetCfg, ec2Opts...), nil } +// dryRunValidationTimeout is the maximum time to wait for credentials to +// propagate across AWS services after rotation before giving up. +const dryRunValidationTimeout = 30 * time.Second + // dryRunValidation performs an AWS DescribeInstances call to verify the state's // credentials, the host listing functionality as well as the filters, if any // are passed in. This function can therefore be used for both host catalog and // host set validation. +// +// After credential rotation, IAM keys may be valid for STS (GetCallerIdentity) +// but not yet propagated to EC2. This function retries on AuthFailure to +// tolerate that eventual consistency window. func dryRunValidation(ctx context.Context, state *awsCatalogPersistedState, ec2Opts []ec2Option, filters ...types.Filter) *status.Status { if state == nil { return status.New(codes.InvalidArgument, "persisted state is required") @@ -842,12 +851,34 @@ func dryRunValidation(ctx context.Context, state *awsCatalogPersistedState, ec2O return status.New(codes.InvalidArgument, fmt.Sprintf("error getting EC2 client: %s", err)) } - _, err = ec2Client.DescribeInstances(ctx, &ec2.DescribeInstancesInput{Filters: filters}) - if err != nil { - return status.New(codes.FailedPrecondition, fmt.Sprintf("aws describe instances failed: %s", err)) - } + deadline := time.Now().Add(dryRunValidationTimeout) + delay := time.Second + for { + _, err = ec2Client.DescribeInstances(ctx, &ec2.DescribeInstancesInput{Filters: filters}) + if err == nil { + return nil + } - return nil + // AuthFailure indicates the credentials exist but haven't propagated + // to EC2 yet — a transient eventual consistency issue after rotation. + // Retry until the deadline. + if strings.Contains(err.Error(), errors.AwsErrorAuthFailure) && time.Now().Before(deadline) { + select { + case <-time.After(delay): + // Wait before retrying to give credentials time to propagate. + case <-ctx.Done(): + // The caller cancelled — break out of the select. + break + } + if ctx.Err() != nil { + break + } + continue + } + + break + } + return status.New(codes.FailedPrecondition, fmt.Sprintf("aws describe instances failed: %s", err)) } // appendDistinct will append the elements to the slice diff --git a/plugin/service/host/plugin_test.go b/plugin/service/host/plugin_test.go index d5be626..1bfabc1 100644 --- a/plugin/service/host/plugin_test.go +++ b/plugin/service/host/plugin_test.go @@ -18,6 +18,7 @@ import ( iamTypes "github.com/aws/aws-sdk-go-v2/service/iam/types" "github.com/google/go-cmp/cmp" "github.com/hashicorp/boundary-plugin-aws/internal/credential" + pluginErrors "github.com/hashicorp/boundary-plugin-aws/internal/errors" "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostcatalogs" "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostsets" pb "github.com/hashicorp/boundary/sdk/pbs/plugin" @@ -3121,4 +3122,66 @@ func TestDryRunValidation(t *testing.T) { }, []ec2Option{}) require.Nil(t, st) }) + + t.Run("authFailureThenSuccess", func(t *testing.T) { + // Simulates the IAM eventual consistency window: EC2 returns AuthFailure + // twice before the credentials propagate and the call succeeds. + state := &testMockEC2State{} + st := dryRunValidation(context.Background(), &awsCatalogPersistedState{ + AwsCredentialPersistedState: &credential.AwsCredentialPersistedState{ + CredentialsConfig: &awsutil.CredentialsConfig{ + AccessKey: "AKIAfoo", + SecretKey: "baz", + }, + }, + testEC2APIFunc: newTestMockEC2(state, + testMockEC2WithDescribeInstancesErrorSequence( + fmt.Errorf("%s: credentials not yet propagated", pluginErrors.AwsErrorAuthFailure), + fmt.Errorf("%s: credentials not yet propagated", pluginErrors.AwsErrorAuthFailure), + ), + testMockEC2WithDescribeInstancesOutput(&ec2.DescribeInstancesOutput{}), + ), + }, []ec2Option{}) + require.Nil(t, st) + require.Equal(t, 3, state.DescribeInstancesCallCount) + }) + + t.Run("authFailureContextCancelled", func(t *testing.T) { + // If the context is cancelled while waiting to retry, the function should + // return a FailedPrecondition error immediately rather than hanging. + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately so the first retry select hits ctx.Done() + st := dryRunValidation(ctx, &awsCatalogPersistedState{ + AwsCredentialPersistedState: &credential.AwsCredentialPersistedState{ + CredentialsConfig: &awsutil.CredentialsConfig{ + AccessKey: "AKIAfoo", + SecretKey: "baz", + }, + }, + testEC2APIFunc: newTestMockEC2(nil, + testMockEC2WithDescribeInstancesError(fmt.Errorf("%s: credentials not yet propagated", pluginErrors.AwsErrorAuthFailure)), + ), + }, []ec2Option{}) + require.NotNil(t, st) + require.Equal(t, codes.FailedPrecondition.String(), st.Code().String()) + }) + + t.Run("nonAuthFailureNoRetry", func(t *testing.T) { + // A non-AuthFailure error should fail immediately without retrying. + state := &testMockEC2State{} + st := dryRunValidation(context.Background(), &awsCatalogPersistedState{ + AwsCredentialPersistedState: &credential.AwsCredentialPersistedState{ + CredentialsConfig: &awsutil.CredentialsConfig{ + AccessKey: "AKIAfoo", + SecretKey: "baz", + }, + }, + testEC2APIFunc: newTestMockEC2(state, + testMockEC2WithDescribeInstancesError(fmt.Errorf("UnauthorizedOperation: access denied")), + ), + }, []ec2Option{}) + require.NotNil(t, st) + require.Equal(t, codes.FailedPrecondition.String(), st.Code().String()) + require.Equal(t, 1, state.DescribeInstancesCallCount) + }) } diff --git a/plugin/service/host/testing.go b/plugin/service/host/testing.go index 92f19ac..f1ee582 100644 --- a/plugin/service/host/testing.go +++ b/plugin/service/host/testing.go @@ -43,6 +43,10 @@ type testMockEC2 struct { DescribeInstancesOutput *ec2.DescribeInstancesOutput DescribeInstancesError error + + // DescribeInstancesErrorSequence, if set, returns each error in order on + // successive calls. Once exhausted, DescribeInstancesOutput is returned. + DescribeInstancesErrorSequence []error } type testMockEC2Option func(m *testMockEC2) error @@ -61,6 +65,16 @@ func testMockEC2WithDescribeInstancesError(e error) testMockEC2Option { } } +// testMockEC2WithDescribeInstancesErrorSequence sets a sequence of errors to +// return on successive DescribeInstances calls. Once the sequence is exhausted, +// the mock returns DescribeInstancesOutput (nil error). +func testMockEC2WithDescribeInstancesErrorSequence(errs ...error) testMockEC2Option { + return func(m *testMockEC2) error { + m.DescribeInstancesErrorSequence = errs + return nil + } +} + func newTestMockEC2(state *testMockEC2State, opts ...testMockEC2Option) ec2APIFunc { return func(cfgs ...aws.Config) (EC2API, error) { m := &testMockEC2{ @@ -96,6 +110,12 @@ func (m *testMockEC2) DescribeInstances(ctx context.Context, input *ec2.Describe m.State.DescribeInstancesInputs = append(m.State.DescribeInstancesInputs, input) } + if len(m.DescribeInstancesErrorSequence) > 0 { + err := m.DescribeInstancesErrorSequence[0] + m.DescribeInstancesErrorSequence = m.DescribeInstancesErrorSequence[1:] + return nil, err + } + if m.DescribeInstancesError != nil { return nil, m.DescribeInstancesError }