diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 56c3af459b4..65c50db965c 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -576,17 +576,20 @@ type CloudConfig struct { // Region = region1 // URL = https://s3.foo.bar // SigningRegion = signing_region + // SigningMethod = signing_method // // [ServiceOverride "2"] // Service = ec2 // Region = region2 // URL = https://ec2.foo.bar // SigningRegion = signing_region + // SigningMethod = signing_method ServiceOverride map[string]*struct { Service string Region string URL string SigningRegion string + SigningMethod string } } @@ -596,32 +599,39 @@ func (cfg *CloudConfig) validateOverrides() error { } set := make(map[string]bool) for onum, ovrd := range cfg.ServiceOverride { + // Note: gcfg does not space trim, so we have to when comparing to empty string "" name := strings.TrimSpace(ovrd.Service) if name == "" { return fmt.Errorf("service name is missing [Service is \"\"] in override %s", onum) } + // insure the map service name is space trimmed + ovrd.Service = name + region := strings.TrimSpace(ovrd.Region) if region == "" { return fmt.Errorf("service region is missing [Region is \"\"] in override %s", onum) } + // insure the map region is space trimmed + ovrd.Region = region + url := strings.TrimSpace(ovrd.URL) - if url == "" { + if url== "" { return fmt.Errorf("url is missing [URL is \"\"] in override %s", onum) } signingRegion := strings.TrimSpace(ovrd.SigningRegion) if signingRegion == "" { return fmt.Errorf("signingRegion is missing [SigningRegion is \"\"] in override %s", onum) } - if _, ok := set[name+"_"+region]; ok { + signature := name+"_"+region + if set[signature] { return fmt.Errorf("duplicate entry found for service override [%s] (%s in %s)", onum, name, region) } - set[name+"_"+region] = true + set[signature] = true } return nil } -func (cfg *CloudConfig) getResolver() func(service, region string, - optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { +func (cfg *CloudConfig) getResolver() endpoints.ResolverFunc { defaultResolver := endpoints.DefaultResolver() defaultResolverFn := func(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { @@ -630,19 +640,20 @@ func (cfg *CloudConfig) getResolver() func(service, region string, if len(cfg.ServiceOverride) == 0 { return defaultResolverFn } - customResolverFn := func(service, region string, + + return func(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { for _, override := range cfg.ServiceOverride { if override.Service == service && override.Region == region { return endpoints.ResolvedEndpoint{ URL: override.URL, SigningRegion: override.SigningRegion, + SigningMethod: override.SigningMethod, }, nil } } return defaultResolver.EndpointFor(service, region, optFns...) } - return customResolverFn } // awsSdkEC2 is an implementation of the EC2 interface, backed by aws-sdk-go @@ -652,7 +663,7 @@ type awsSdkEC2 struct { // Interface to make the CloudConfig immutable for awsSDKProvider type awsCloudConfigProvider interface { - getResolver() func(string, string, ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) + getResolver() endpoints.ResolverFunc } type awsSDKProvider struct { @@ -732,7 +743,7 @@ func (p *awsSDKProvider) Compute(regionName string) (EC2, error) { Credentials: p.creds, } awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true). - WithEndpointResolver(endpoints.ResolverFunc(p.cfg.getResolver())) + WithEndpointResolver(p.cfg.getResolver()) sess, err := session.NewSession(awsConfig) if err != nil { @@ -754,7 +765,7 @@ func (p *awsSDKProvider) LoadBalancing(regionName string) (ELB, error) { Credentials: p.creds, } awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true). - WithEndpointResolver(endpoints.ResolverFunc(p.cfg.getResolver())) + WithEndpointResolver(p.cfg.getResolver()) sess, err := session.NewSession(awsConfig) if err != nil { @@ -772,7 +783,7 @@ func (p *awsSDKProvider) LoadBalancingV2(regionName string) (ELBV2, error) { Credentials: p.creds, } awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true). - WithEndpointResolver(endpoints.ResolverFunc(p.cfg.getResolver())) + WithEndpointResolver(p.cfg.getResolver()) sess, err := session.NewSession(awsConfig) if err != nil { @@ -791,7 +802,7 @@ func (p *awsSDKProvider) Autoscaling(regionName string) (ASG, error) { Credentials: p.creds, } awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true). - WithEndpointResolver(endpoints.ResolverFunc(p.cfg.getResolver())) + WithEndpointResolver(p.cfg.getResolver()) sess, err := session.NewSession(awsConfig) if err != nil { @@ -806,7 +817,7 @@ func (p *awsSDKProvider) Autoscaling(regionName string) (ASG, error) { func (p *awsSDKProvider) Metadata() (EC2Metadata, error) { sess, err := session.NewSession(&aws.Config{ - EndpointResolver: endpoints.ResolverFunc(p.cfg.getResolver()), + EndpointResolver: p.cfg.getResolver(), }) if err != nil { return nil, fmt.Errorf("unable to initialize AWS session: %v", err) @@ -822,7 +833,7 @@ func (p *awsSDKProvider) KeyManagement(regionName string) (KMS, error) { Credentials: p.creds, } awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true). - WithEndpointResolver(endpoints.ResolverFunc(p.cfg.getResolver())) + WithEndpointResolver(p.cfg.getResolver()) sess, err := session.NewSession(awsConfig) if err != nil { diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index 4a7ea0217c0..66256760320 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -189,6 +189,7 @@ func TestReadAWSCloudConfig(t *testing.T) { type ServiceDescriptor struct { name string region string + signingRegion, signingMethod string } func TestOverridesActiveConfig(t *testing.T) { @@ -220,6 +221,7 @@ func TestOverridesActiveConfig(t *testing.T) { Region=sregion URL=https://s3.foo.bar SigningRegion=sregion + SigningMethod = sign `), nil, true, false, @@ -234,6 +236,7 @@ func TestOverridesActiveConfig(t *testing.T) { Service=s3 URL=https://s3.foo.bar SigningRegion=sregion + SigningMethod = sign `), nil, true, false, @@ -245,9 +248,10 @@ func TestOverridesActiveConfig(t *testing.T) { [global] [ServiceOverride "1"] - Service=s3 + Service="s3" Region=sregion SigningRegion=sregion + SigningMethod = sign `), nil, true, false, @@ -262,6 +266,7 @@ func TestOverridesActiveConfig(t *testing.T) { Service=s3 Region=sregion URL=https://s3.foo.bar + SigningMethod = sign `), nil, true, false, @@ -273,14 +278,15 @@ func TestOverridesActiveConfig(t *testing.T) { [Global] [ServiceOverride "1"] - Service = s3 + Service = "s3 " Region = sregion URL = https://s3.foo.bar SigningRegion = sregion + SigningMethod = v4 `), nil, false, true, - []ServiceDescriptor{{name: "s3", region: "sregion"}}, + []ServiceDescriptor{{name: "s3", region: "sregion", signingRegion: "sregion", signingMethod: "v4"}}, }, { "Multiple Overridden Services", @@ -292,16 +298,19 @@ func TestOverridesActiveConfig(t *testing.T) { Service=s3 Region=sregion1 URL=https://s3.foo.bar - SigningRegion=sregion + SigningRegion=sregion1 + SigningMethod = v4 [ServiceOverride "2"] Service=ec2 Region=sregion2 URL=https://ec2.foo.bar - SigningRegion=sregion`), + SigningRegion=sregion2 + SigningMethod = v4`), nil, false, true, - []ServiceDescriptor{{"s3", "sregion1"}, {"ec2", "sregion2"}}, + []ServiceDescriptor{{name:"s3", region: "sregion1", signingRegion: "sregion1", signingMethod: "v4"}, + {name: "ec2", region: "sregion2", signingRegion: "sregion2", signingMethod: "v4"}}, }, { "Duplicate Services", @@ -314,12 +323,14 @@ func TestOverridesActiveConfig(t *testing.T) { Region=sregion1 URL=https://s3.foo.bar SigningRegion=sregion + SigningMethod = sign [ServiceOverride "2"] Service=s3 Region=sregion1 URL=https://s3.foo.bar - SigningRegion=sregion`), + SigningRegion=sregion + SigningMethod = sign`), nil, true, false, []ServiceDescriptor{}, @@ -333,17 +344,19 @@ func TestOverridesActiveConfig(t *testing.T) { Service=s3 Region=region1 URL=https://s3.foo.bar - SigningRegion=sregion + SigningRegion=sregion1 [ServiceOverride "2"] Service=ec2 Region=region2 URL=https://ec2.foo.bar SigningRegion=sregion + SigningMethod = v4 `), nil, false, true, - []ServiceDescriptor{{"s3", "region1"}, {"ec2", "region2"}}, + []ServiceDescriptor{{name: "s3", region: "region1", signingRegion: "sregion1", signingMethod: ""}, + {name:"ec2", region: "region2", signingRegion: "sregion", signingMethod: "v4"}}, }, { "Multiple regions, Same Service", @@ -354,17 +367,21 @@ func TestOverridesActiveConfig(t *testing.T) { Service=s3 Region=region1 URL=https://s3.foo.bar - SigningRegion=sregion + SigningRegion=sregion1 + SigningMethod = v3 [ServiceOverride "2"] Service=s3 Region=region2 URL=https://s3.foo.bar - SigningRegion=sregion + SigningRegion=sregion1 + SigningMethod = v4 + `), nil, false, true, - []ServiceDescriptor{{"s3", "region1"}, {"s3", "region2"}}, + []ServiceDescriptor{{name:"s3", region: "region1", signingRegion: "sregion1", signingMethod: "v3"}, + {name:"s3", region: "region2", signingRegion: "sregion1", signingMethod: "v4"}}, }, } @@ -393,6 +410,7 @@ func TestOverridesActiveConfig(t *testing.T) { Region string URL string SigningRegion string + SigningMethod string } for _, v := range cfg.ServiceOverride { if v.Service == sd.name && v.Region == sd.region { @@ -404,9 +422,13 @@ func TestOverridesActiveConfig(t *testing.T) { t.Errorf("Missing override for service %s in case %s", sd.name, test.name) } else { - if found.SigningRegion != "sregion" { - t.Errorf("Expected signing region 'sregion', received '%s' for case %s", - found.SigningRegion, test.name) + if found.SigningRegion != sd.signingRegion { + t.Errorf("Expected signing region '%s', received '%s' for case %s", + sd.signingRegion, found.SigningRegion, test.name) + } + if found.SigningMethod != sd.signingMethod { + t.Errorf("Expected signing method '%s', received '%s' for case %s", + sd.signingMethod, found.SigningRegion, test.name) } targetName := fmt.Sprintf("https://%s.foo.bar", sd.name) if found.URL != targetName { @@ -425,9 +447,13 @@ func TestOverridesActiveConfig(t *testing.T) { t.Errorf("Expected endpoint url: %s, received %s in case %s", targetName, ep1.URL, test.name) } - if ep1.SigningRegion != "sregion" { - t.Errorf("Expected signing region 'sregion', received '%s' in case %s", - ep1.SigningRegion, test.name) + if ep1.SigningRegion != sd.signingRegion { + t.Errorf("Expected signing region '%s', received '%s' in case %s", + sd.signingRegion, ep1.SigningRegion, test.name) + } + if ep1.SigningMethod != sd.signingMethod { + t.Errorf("Expected signing method '%s', received '%s' in case %s", + sd.signingMethod, ep1.SigningRegion, test.name) } } }