From 9d6124f3bcde20448e997714094602c8d7846449 Mon Sep 17 00:00:00 2001 From: Thuan Vo Date: Mon, 22 Sep 2025 16:45:20 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20backwards=20compatibility?= =?UTF-8?q?=20for=20AWS=20service=20endpoint=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In AWS SDKv1, each service exports a constant EndpointsID to look up the custom service endpoint, for example, see ref [0]. In AWS SDKv2, these contants are no longer available. Thus, for backwards compatibility, we copy those constants from the SDKv1 and map them to the corresponding ServiceID in SDK v2. Additionally, in AWS SDKv1, elb and elbv2 uses the same identifier (i.e. same EndpointsID), thus the same custom endpoint [1][2]. For backwards compatibility, if elbv2 endpoint is undefined, elbv2 endpoint resolver should fall back to elb endpoint if any. This also fixes the bug where CAPA does not recognize services that define its serviceID with more than 1 word due to the incorrect assumption [3]. References: [0] https://github.com/aws/aws-sdk-go/blob/070853e88d22854d2355c2543d0958a5f76ad407/service/resourcegroupstaggingapi/service.go#L33-L34 [1] https://github.com/aws/aws-sdk-go/blob/070853e88d22854d2355c2543d0958a5f76ad407/service/elbv2/service.go#L32-L33 [2] https://github.com/aws/aws-sdk-go/blob/070853e88d22854d2355c2543d0958a5f76ad407/service/elb/service.go#L32-L33 [2] https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/88cb4b92b1a76591623e9d5ef347bfdc22010622/pkg/cloud/endpoints/endpoints.go#L90-L94 --- pkg/cloud/endpoints/endpoints.go | 38 +++++++++++++-- pkg/cloud/endpoints/endpoints_test.go | 70 ++++++++++++++++++++++----- 2 files changed, 91 insertions(+), 17 deletions(-) diff --git a/pkg/cloud/endpoints/endpoints.go b/pkg/cloud/endpoints/endpoints.go index 7c4356d452..29996737a1 100644 --- a/pkg/cloud/endpoints/endpoints.go +++ b/pkg/cloud/endpoints/endpoints.go @@ -46,6 +46,18 @@ var ( errServiceEndpointServiceID = errors.New("must use a valid serviceID from the AWS GO SDK") errServiceEndpointDuplicateServiceID = errors.New("same serviceID defined twice for signing region") serviceEndpointsMap = map[string]serviceEndpoint{} + compatServiceIDMap = map[string]string{ + "s3": s3.ServiceID, + "elasticloadbalancing": elb.ServiceID, + "ec2": ec2.ServiceID, + "tagging": rgapi.ServiceID, + "sqs": sqs.ServiceID, + "events": eventbridge.ServiceID, + "eks": eks.ServiceID, + "ssm": ssm.ServiceID, + "sts": sts.ServiceID, + "secretsmanager": secretsmanager.ServiceID, + } ) // serviceEndpoint contains AWS Service resolution information for SDK V2. @@ -87,11 +99,13 @@ func ParseFlag(serviceEndpoints string) error { } seenServices = append(seenServices, serviceID) - // convert service ID to UpperCase as service IDs in AWS SDK GO V2 are UpperCase & Go map is Case Sensitve - // This is for backward compabitibility - // Ref: SDK V2 https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/ec2#pkg-constants - // Ref: SDK V1 https://pkg.go.dev/github.com/aws/aws-sdk-go/aws/endpoints#pkg-constants - serviceID = strings.ToUpper(serviceID) + // In v1 sdk, a constant EndpointsID is exported in each service to look up the custom service endpoint. + // For example: https://github.com/aws/aws-sdk-go/blob/070853e88d22854d2355c2543d0958a5f76ad407/service/resourcegroupstaggingapi/service.go#L33-L34 + // In v2 SDK, these constants are no longer available. + // For backwards compatibility, we copy those constants from the SDK v1 and map it to ServiceID in SDK v2. + if v2serviceID, ok := compatServiceIDMap[serviceID]; ok { + serviceID = v2serviceID + } URL, err := url.ParseRequestURI(kv[1]) if err != nil { @@ -104,6 +118,20 @@ func ParseFlag(serviceEndpoints string) error { } serviceEndpointsMap[serviceID] = endpoint } + + // In v1 SDK, elb and elbv2 uses the same identifier, thus the same endpoint. + // elbv2: https://github.com/aws/aws-sdk-go/blob/070853e88d22854d2355c2543d0958a5f76ad407/service/elbv2/service.go#L32-L33 + // elb: https://github.com/aws/aws-sdk-go/blob/070853e88d22854d2355c2543d0958a5f76ad407/service/elb/service.go#L32-L33 + // For backwards compatibility, if elbv2 endpoint is undefined, the elbv2 endpoint resolver should fall back to elb endpoint if any. + if _, ok := serviceEndpointsMap[elbv2.ServiceID]; !ok { + if elbEp, ok := serviceEndpointsMap[elb.ServiceID]; ok { + serviceEndpointsMap[elbv2.ServiceID] = serviceEndpoint{ + ServiceID: elbv2.ServiceID, + URL: elbEp.URL, + SigningRegion: elbEp.SigningRegion, + } + } + } } return nil } diff --git a/pkg/cloud/endpoints/endpoints_test.go b/pkg/cloud/endpoints/endpoints_test.go index 42d368100f..7c3c76b68a 100644 --- a/pkg/cloud/endpoints/endpoints_test.go +++ b/pkg/cloud/endpoints/endpoints_test.go @@ -19,28 +19,60 @@ package endpoints import ( "errors" "testing" + + "github.com/google/go-cmp/cmp" ) func TestParseFlags(t *testing.T) { testCases := []struct { - name string - flagToParse string - expectedError error + name string + flagToParse string + expectedError error + expectedServiceEndpointsMap map[string]serviceEndpoint }{ { - name: "no configuration", - flagToParse: "", - expectedError: nil, + name: "no configuration", + flagToParse: "", + expectedServiceEndpointsMap: make(map[string]serviceEndpoint), }, { - name: "single region, single service", - flagToParse: "us-iso:ec2=https://localhost:8080", - expectedError: nil, + name: "single region, single service", + flagToParse: "us-iso:ec2=https://localhost:8080", + expectedServiceEndpointsMap: map[string]serviceEndpoint{"EC2": {ServiceID: "EC2", URL: "https://localhost:8080", SigningRegion: "us-iso"}}, }, { - name: "single region, multiple services", - flagToParse: "us-iso:ec2=https://localhost:8080,sts=https://elbhost:8080", - expectedError: nil, + name: "single region, multiple services with v1 service IDs", + flagToParse: "us-iso:s3=https://s3.com,elasticloadbalancing=https://elb.com,ec2=https://ec2.com,tagging=https://tagging.com,sqs=https://sqs.com,events=https://events.com,eks=https://eks.com,ssm=https://ssm.com,sts=https://sts.com,secretsmanager=https://secretmanager.com", + expectedServiceEndpointsMap: map[string]serviceEndpoint{ + "EC2": {ServiceID: "EC2", URL: "https://ec2.com", SigningRegion: "us-iso"}, + "EKS": {ServiceID: "EKS", URL: "https://eks.com", SigningRegion: "us-iso"}, + "Elastic Load Balancing": {ServiceID: "Elastic Load Balancing", URL: "https://elb.com", SigningRegion: "us-iso"}, + "Elastic Load Balancing v2": {ServiceID: "Elastic Load Balancing v2", URL: "https://elb.com", SigningRegion: "us-iso"}, + "EventBridge": {ServiceID: "EventBridge", URL: "https://events.com", SigningRegion: "us-iso"}, + "Resource Groups Tagging API": {ServiceID: "Resource Groups Tagging API", URL: "https://tagging.com", SigningRegion: "us-iso"}, + "S3": {ServiceID: "S3", URL: "https://s3.com", SigningRegion: "us-iso"}, + "SQS": {ServiceID: "SQS", URL: "https://sqs.com", SigningRegion: "us-iso"}, + "SSM": {ServiceID: "SSM", URL: "https://ssm.com", SigningRegion: "us-iso"}, + "STS": {ServiceID: "STS", URL: "https://sts.com", SigningRegion: "us-iso"}, + "Secrets Manager": {ServiceID: "Secrets Manager", URL: "https://secretmanager.com", SigningRegion: "us-iso"}, + }, + }, + { + name: "single region, multiple services with v2 service IDs", + flagToParse: "us-iso:S3=https://s3.com,Elastic Load Balancing=https://elb.com,Elastic Load Balancing v2=https://elbv2.com,EC2=https://ec2.com,Resource Groups Tagging API=https://tagging.com,SQS=https://sqs.com,EventBridge=https://events.com,EKS=https://eks.com,SSM=https://ssm.com,STS=https://sts.com,Secrets Manager=https://secretmanager.com", + expectedServiceEndpointsMap: map[string]serviceEndpoint{ + "EC2": {ServiceID: "EC2", URL: "https://ec2.com", SigningRegion: "us-iso"}, + "EKS": {ServiceID: "EKS", URL: "https://eks.com", SigningRegion: "us-iso"}, + "Elastic Load Balancing": {ServiceID: "Elastic Load Balancing", URL: "https://elb.com", SigningRegion: "us-iso"}, + "Elastic Load Balancing v2": {ServiceID: "Elastic Load Balancing v2", URL: "https://elbv2.com", SigningRegion: "us-iso"}, + "EventBridge": {ServiceID: "EventBridge", URL: "https://events.com", SigningRegion: "us-iso"}, + "Resource Groups Tagging API": {ServiceID: "Resource Groups Tagging API", URL: "https://tagging.com", SigningRegion: "us-iso"}, + "S3": {ServiceID: "S3", URL: "https://s3.com", SigningRegion: "us-iso"}, + "SQS": {ServiceID: "SQS", URL: "https://sqs.com", SigningRegion: "us-iso"}, + "SSM": {ServiceID: "SSM", URL: "https://ssm.com", SigningRegion: "us-iso"}, + "STS": {ServiceID: "STS", URL: "https://sts.com", SigningRegion: "us-iso"}, + "Secrets Manager": {ServiceID: "Secrets Manager", URL: "https://secretmanager.com", SigningRegion: "us-iso"}, + }, }, { name: "single region, duplicate service", @@ -56,6 +88,10 @@ func TestParseFlags(t *testing.T) { name: "multiples regions", flagToParse: "us-iso:ec2=https://localhost:8080,sts=https://elbhost:8080;gb-iso:ec2=https://localhost:8080,sts=https://elbhost:8080", expectedError: nil, + expectedServiceEndpointsMap: map[string]serviceEndpoint{ + "EC2": {ServiceID: "EC2", URL: "https://localhost:8080", SigningRegion: "gb-iso"}, + "STS": {ServiceID: "STS", URL: "https://elbhost:8080", SigningRegion: "gb-iso"}, + }, }, { name: "invalid config", @@ -66,11 +102,21 @@ func TestParseFlags(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + defer t.Cleanup(func() { + serviceEndpointsMap = make(map[string]serviceEndpoint) + }) + err := ParseFlag(tc.flagToParse) if !errors.Is(err, tc.expectedError) { t.Fatalf("did not expect correct error: got %v, expected %v", err, tc.expectedError) } + + if err == nil { + if !cmp.Equal(serviceEndpointsMap, tc.expectedServiceEndpointsMap) { + t.Fatalf("expected serviceEndpointsMap: %#v, but got: %#v", tc.expectedServiceEndpointsMap, serviceEndpointsMap) + } + } }) } }