From a7ab2877fe92396d1c6b68c88e57b14a04736335 Mon Sep 17 00:00:00 2001 From: Damian Debkowski Date: Wed, 16 Oct 2024 14:00:20 -0700 Subject: [PATCH] refactor: optionally set custom endpoint resolver --- plugin/service/host/plugin.go | 41 ++++++++++++++++++++---------- plugin/service/host/plugin_test.go | 20 +++------------ plugin/service/host/state.go | 8 +++--- plugin/service/storage/state.go | 10 +++++--- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/plugin/service/host/plugin.go b/plugin/service/host/plugin.go index c573ae6..1b9f4a0 100644 --- a/plugin/service/host/plugin.go +++ b/plugin/service/host/plugin.go @@ -83,7 +83,11 @@ func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalo } // perform dry run to ensure we can interact with AWS as expected. - if st := dryRunValidation(ctx, catalogState, catalogAttributes); st != nil { + opts := []ec2Option{} + if catalogAttributes.DualStack { + opts = append(opts, WithDualStack(catalogAttributes.DualStack)) + } + if st := dryRunValidation(ctx, catalogState, opts); st != nil { return nil, st.Err() } @@ -166,7 +170,11 @@ func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalo if err != nil { return nil, errors.BadRequestStatus("error loading persisted state: %s", err) } - if st := dryRunValidation(ctx, newCatalogState, newCatalogAttributes); st != nil { + opts := []ec2Option{} + if newCatalogAttributes.DualStack { + opts = append(opts, WithDualStack(newCatalogAttributes.DualStack)) + } + if st := dryRunValidation(ctx, newCatalogState, opts); st != nil { return nil, st.Err() } @@ -208,7 +216,11 @@ func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalo } // perform dry run to ensure we can interact with AWS as expected. - if st := dryRunValidation(ctx, catalogState, newCatalogAttributes); st != nil { + opts := []ec2Option{} + if newCatalogAttributes.DualStack { + opts = append(opts, WithDualStack(newCatalogAttributes.DualStack)) + } + if st := dryRunValidation(ctx, catalogState, opts); st != nil { return nil, st.Err() } @@ -356,7 +368,11 @@ func (p *HostPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest return nil, errors.BadRequestStatus("error building set filters: %s", err) } - if st := dryRunValidation(ctx, catalogState, catalogAttributes, describeInstanceFilters...); st != nil { + opts := []ec2Option{} + if catalogAttributes.DualStack { + opts = append(opts, WithDualStack(catalogAttributes.DualStack)) + } + if st := dryRunValidation(ctx, catalogState, opts, describeInstanceFilters...); st != nil { return nil, st.Err() } return &pb.OnCreateSetResponse{}, nil @@ -416,7 +432,11 @@ func (p *HostPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest return nil, errors.BadRequestStatus("error building set filters: %s", err) } - if st := dryRunValidation(ctx, catalogState, catalogAttributes, describeInstanceFilters...); st != nil { + opts := []ec2Option{} + if catalogAttributes.DualStack { + opts = append(opts, WithDualStack(catalogAttributes.DualStack)) + } + if st := dryRunValidation(ctx, catalogState, opts, describeInstanceFilters...); st != nil { return nil, st.Err() } @@ -677,19 +697,12 @@ func awsInstanceToHost(instance types.Instance) (*pb.ListHostsResponseHost, erro // 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. -func dryRunValidation(ctx context.Context, state *awsCatalogPersistedState, catalogAttributes *CatalogAttributes, filters ...types.Filter) *status.Status { +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") } - if catalogAttributes == nil { - return status.New(codes.InvalidArgument, "catalog attributes is required") - } - opts := []ec2Option{} - if catalogAttributes.DualStack { - opts = append(opts, WithDualStack(catalogAttributes.DualStack)) - } - ec2Client, err := state.EC2Client(ctx, opts...) + ec2Client, err := state.EC2Client(ctx, ec2Opts...) if err != nil { return status.New(codes.InvalidArgument, fmt.Sprintf("error getting EC2 client: %s", err)) } diff --git a/plugin/service/host/plugin_test.go b/plugin/service/host/plugin_test.go index 6346263..5ade636 100644 --- a/plugin/service/host/plugin_test.go +++ b/plugin/service/host/plugin_test.go @@ -2754,20 +2754,6 @@ func TestDryRunValidation(t *testing.T) { require.Equal(t, "persisted state is required", st.Message()) }) - t.Run("nil catalog state", func(t *testing.T) { - st := dryRunValidation(context.Background(), &awsCatalogPersistedState{ - AwsCredentialPersistedState: &credential.AwsCredentialPersistedState{ - CredentialsConfig: &awsutil.CredentialsConfig{}, - }, - testEC2APIFunc: func(...aws.Config) (EC2API, error) { - return nil, fmt.Errorf("oops ec2 client err") - }, - }, nil) - require.NotNil(t, st) - require.Equal(t, codes.InvalidArgument.String(), st.Code().String()) - require.Equal(t, "catalog attributes is required", st.Message()) - }) - t.Run("ec2ClientErr", func(t *testing.T) { st := dryRunValidation(context.Background(), &awsCatalogPersistedState{ AwsCredentialPersistedState: &credential.AwsCredentialPersistedState{ @@ -2776,7 +2762,7 @@ func TestDryRunValidation(t *testing.T) { testEC2APIFunc: func(...aws.Config) (EC2API, error) { return nil, fmt.Errorf("oops ec2 client err") }, - }, &CatalogAttributes{}) + }, []ec2Option{}) require.NotNil(t, st) require.Equal(t, codes.InvalidArgument.String(), st.Code().String()) require.Equal(t, "error getting EC2 client: oops ec2 client err", st.Message()) @@ -2791,7 +2777,7 @@ func TestDryRunValidation(t *testing.T) { }, }, testEC2APIFunc: newTestMockEC2(nil, testMockEC2WithDescribeInstancesError(fmt.Errorf("oops describe instances error"))), - }, &CatalogAttributes{}) + }, []ec2Option{}) require.NotNil(t, st) require.Equal(t, codes.FailedPrecondition.String(), st.Code().String()) require.Equal(t, "aws describe instances failed: oops describe instances error", st.Message()) @@ -2806,7 +2792,7 @@ func TestDryRunValidation(t *testing.T) { }, }, testEC2APIFunc: newTestMockEC2(nil, testMockEC2WithDescribeInstancesOutput(&ec2.DescribeInstancesOutput{})), - }, &CatalogAttributes{}) + }, []ec2Option{}) require.Nil(t, st) }) } diff --git a/plugin/service/host/state.go b/plugin/service/host/state.go index bc1278e..a2e772e 100644 --- a/plugin/service/host/state.go +++ b/plugin/service/host/state.go @@ -86,9 +86,11 @@ func (s *awsCatalogPersistedState) EC2Client(ctx context.Context, opt ...ec2Opti return nil, fmt.Errorf("nil aws configuration") } var ec2Opts []func(*ec2.Options) - ec2Opts = append(ec2Opts, ec2.WithEndpointResolverV2(&endpointResolver{ - dualStack: opts.withDualStack, - })) + if opts.withDualStack { + ec2Opts = append(ec2Opts, ec2.WithEndpointResolverV2(&endpointResolver{ + dualStack: opts.withDualStack, + })) + } if s.testEC2APIFunc != nil { return s.testEC2APIFunc(*awsCfg) } diff --git a/plugin/service/storage/state.go b/plugin/service/storage/state.go index 701a8b2..d82d011 100644 --- a/plugin/service/storage/state.go +++ b/plugin/service/storage/state.go @@ -92,10 +92,12 @@ func (s *awsStoragePersistedState) S3Client(ctx context.Context, opt ...s3Option return nil, fmt.Errorf("nil aws configuration") } var s3Opts []func(*s3.Options) - s3Opts = append(s3Opts, s3.WithEndpointResolverV2(&endpointResolver{ - endpoint: opts.withEndpoint, - dualStack: opts.withDualStack, - })) + if opts.withEndpoint != "" || opts.withDualStack { + s3Opts = append(s3Opts, s3.WithEndpointResolverV2(&endpointResolver{ + endpoint: opts.withEndpoint, + dualStack: opts.withDualStack, + })) + } if s.testS3APIFunc != nil { return s.testS3APIFunc(*awsCfg) }