Add SigningMethod, make it optional, copy in verbatim

An empty SigningMethod will default properly.

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,
+func (cfg *CloudConfig) getResolver() endpoints.ResolverFunc {
-	optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) {
 	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 {

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" {
+						if found.SigningRegion != sd.signingRegion {
-							t.Errorf("Expected signing region 'sregion', received '%s' for case %s",
+							t.Errorf("Expected signing region '%s', received '%s' for case %s",
-								found.SigningRegion, test.name)
+								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" {
+							if ep1.SigningRegion != sd.signingRegion {
-								t.Errorf("Expected signing region 'sregion', received '%s' in case %s",
+								t.Errorf("Expected signing region '%s', received '%s' in case %s",
-									ep1.SigningRegion, test.name)
+									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)
 							}
 						}
 					}