Merge pull request #75990 from mcrute/remove-regions

Remove hard-coded region list from AWS cloud provider

staging/src/k8s.io/legacy-cloud-providers/aws/BUILD

@@ -19,7 +19,6 @@ go_library(
         "device_allocator.go",
         "instances.go",
         "log_handler.go",
-        "regions.go",
         "retry_handler.go",
         "sets_ippermissions.go",
         "tags.go",
@@ -74,7 +73,6 @@ go_test(
         "aws_test.go",
         "device_allocator_test.go",
         "instances_test.go",
-        "regions_test.go",
         "retry_handler_test.go",
         "tags_test.go",
     ],

staging/src/k8s.io/legacy-cloud-providers/aws/aws.go

@@ -270,9 +270,6 @@ const MaxReadThenCreateRetries = 30
 // need hardcoded defaults.
 const DefaultVolumeType = "gp2"
 
-// Used to call recognizeWellKnownRegions just once
-var once sync.Once
-
 // Services is an abstraction over AWS, to allow mocking/other implementations
 type Services interface {
 	Compute(region string) (EC2, error)
@@ -1214,14 +1211,8 @@ func newAWSCloud(cfg CloudConfig, awsServices Services) (*Cloud, error) {
 		return nil, err
 	}
 
-	// Trust that if we get a region from configuration or AWS metadata that it is valid,
-	// and register ECR providers
-	recognizeRegion(regionName)
-
 	if !cfg.Global.DisableStrictZoneCheck {
-		valid := isRegionValid(regionName)
+		if !isRegionValid(regionName, metadata) {
-		if !valid {
-			// This _should_ now be unreachable, given we call RecognizeRegion
 			return nil, fmt.Errorf("not a valid AWS zone (unknown region): %s", zone)
 		}
 	} else {
@@ -1303,14 +1294,43 @@ func newAWSCloud(cfg CloudConfig, awsServices Services) (*Cloud, error) {
 		}
 	}
 
-	// Register regions, in particular for ECR credentials
-	once.Do(func() {
-		recognizeWellKnownRegions()
-	})
-
 	return awsCloud, nil
 }
 
+// isRegionValid accepts an AWS region name and returns if the region is a
+// valid region known to the AWS SDK. Considers the region returned from the
+// EC2 metadata service to be a valid region as it's only available on a host
+// running in a valid AWS region.
+func isRegionValid(region string, metadata EC2Metadata) bool {
+	// Does the AWS SDK know about the region?
+	for _, p := range endpoints.DefaultPartitions() {
+		for r := range p.Regions() {
+			if r == region {
+				return true
+			}
+		}
+	}
+
+	// ap-northeast-3 is purposely excluded from the SDK because it
+	// requires an access request (for more details see):
+	// https://github.com/aws/aws-sdk-go/issues/1863
+	if region == "ap-northeast-3" {
+		return true
+	}
+
+	// Fallback to checking if the region matches the instance metadata region
+	// (ignoring any user overrides). This just accounts for running an old
+	// build of Kubernetes in a new region that wasn't compiled into the SDK
+	// when Kubernetes was built.
+	if az, err := getAvailabilityZone(metadata); err == nil {
+		if r, err := azToRegion(az); err == nil && region == r {
+			return true
+		}
+	}
+
+	return false
+}
+
 // Initialize passes a Kubernetes clientBuilder interface to the cloud provider
 func (c *Cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
 	c.clientBuilder = clientBuilder

staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go

@@ -1835,6 +1835,47 @@ func TestCreateDisk(t *testing.T) {
 	awsServices.ec2.(*MockedFakeEC2).AssertExpectations(t)
 }
 
+func TestRegionIsValid(t *testing.T) {
+	fake := newMockedFakeAWSServices("fakeCluster")
+	fake.selfInstance.Placement = &ec2.Placement{
+		AvailabilityZone: aws.String("pl-fake-999a"),
+	}
+
+	// This is the legacy list that was removed, using this to ensure we avoid
+	// region regressions if something goes wrong in the SDK
+	regions := []string{
+		"ap-northeast-1",
+		"ap-northeast-2",
+		"ap-northeast-3",
+		"ap-south-1",
+		"ap-southeast-1",
+		"ap-southeast-2",
+		"ca-central-1",
+		"eu-central-1",
+		"eu-west-1",
+		"eu-west-2",
+		"eu-west-3",
+		"sa-east-1",
+		"us-east-1",
+		"us-east-2",
+		"us-west-1",
+		"us-west-2",
+		"cn-north-1",
+		"cn-northwest-1",
+		"us-gov-west-1",
+		"ap-northeast-3",
+
+		// Ensures that we always trust what the metadata service returns
+		"pl-fake-999",
+	}
+
+	for _, region := range regions {
+		assert.True(t, isRegionValid(region, fake.metadata), "expected region '%s' to be valid but it was not", region)
+	}
+
+	assert.False(t, isRegionValid("pl-fake-991a", fake.metadata), "expected region 'pl-fake-991' to be invalid but it was not")
+}
+
 func TestGetCandidateZonesForDynamicVolume(t *testing.T) {
 	tests := []struct {
 		name          string

staging/src/k8s.io/legacy-cloud-providers/aws/regions.go

@@ -1,96 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package aws
-
-import (
-	"sync"
-
-	"k8s.io/klog"
-
-	"k8s.io/apimachinery/pkg/util/sets"
-)
-
-// wellKnownRegions is the complete list of regions known to the AWS cloudprovider
-// and credentialprovider.
-var wellKnownRegions = [...]string{
-	// from `aws ec2 describe-regions --region us-east-1 --query Regions[].RegionName | sort`
-	"ap-northeast-1",
-	"ap-northeast-2",
-	"ap-northeast-3",
-	"ap-south-1",
-	"ap-southeast-1",
-	"ap-southeast-2",
-	"ca-central-1",
-	"eu-central-1",
-	"eu-west-1",
-	"eu-west-2",
-	"eu-west-3",
-	"sa-east-1",
-	"us-east-1",
-	"us-east-2",
-	"us-west-1",
-	"us-west-2",
-
-	// these are not registered in many / most accounts
-	"cn-north-1",
-	"cn-northwest-1",
-	"us-gov-west-1",
-}
-
-// awsRegionsMutex protects awsRegions
-var awsRegionsMutex sync.Mutex
-
-// awsRegions is a set of recognized regions
-var awsRegions sets.String
-
-// recognizeRegion is called for each AWS region we know about.
-// It currently registers a credential provider for that region.
-// There are two paths to discovering a region:
-//  * we hard-code some well-known regions
-//  * if a region is discovered from instance metadata, we add that
-func recognizeRegion(region string) {
-	awsRegionsMutex.Lock()
-	defer awsRegionsMutex.Unlock()
-
-	if awsRegions == nil {
-		awsRegions = sets.NewString()
-	}
-
-	if awsRegions.Has(region) {
-		klog.V(6).Infof("found AWS region %q again - ignoring", region)
-		return
-	}
-
-	klog.V(4).Infof("found AWS region %q", region)
-
-	awsRegions.Insert(region)
-}
-
-// recognizeWellKnownRegions calls RecognizeRegion on each WellKnownRegion
-func recognizeWellKnownRegions() {
-	for _, region := range wellKnownRegions {
-		recognizeRegion(region)
-	}
-}
-
-// isRegionValid checks if the region is in the set of known regions
-func isRegionValid(region string) bool {
-	awsRegionsMutex.Lock()
-	defer awsRegionsMutex.Unlock()
-
-	return awsRegions.Has(region)
-}

staging/src/k8s.io/legacy-cloud-providers/aws/regions_test.go

@@ -1,85 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package aws
-
-import (
-	"testing"
-)
-
-// TestRegions does basic checking of region verification / addition
-func TestRegions(t *testing.T) {
-	recognizeWellKnownRegions()
-
-	tests := []struct {
-		Add            string
-		Lookup         string
-		ExpectIsRegion bool
-	}{
-		{
-			Lookup:         "us-east-1",
-			ExpectIsRegion: true,
-		},
-		{
-			Lookup:         "us-east-1a",
-			ExpectIsRegion: false,
-		},
-		{
-			Add:            "us-test-1",
-			Lookup:         "us-east-1",
-			ExpectIsRegion: true,
-		},
-		{
-			Lookup:         "us-test-1",
-			ExpectIsRegion: true,
-		},
-		{
-			Add:            "us-test-1",
-			Lookup:         "us-test-1",
-			ExpectIsRegion: true,
-		},
-	}
-
-	for _, test := range tests {
-		if test.Add != "" {
-			recognizeRegion(test.Add)
-		}
-
-		if test.Lookup != "" {
-			if isRegionValid(test.Lookup) != test.ExpectIsRegion {
-				t.Fatalf("region valid mismatch: %q", test.Lookup)
-			}
-		}
-	}
-}
-
-// TestRecognizesNewRegion verifies that we see a region from metadata, we recognize it as valid
-func TestRecognizesNewRegion(t *testing.T) {
-	region := "us-testrecognizesnewregion-1"
-	if isRegionValid(region) {
-		t.Fatalf("region already valid: %q", region)
-	}
-
-	awsServices := NewFakeAWSServices(TestClusterID).WithAz(region + "a")
-	_, err := newAWSCloud(CloudConfig{}, awsServices)
-	if err != nil {
-		t.Errorf("error building AWS cloud: %v", err)
-	}
-
-	if !isRegionValid(region) {
-		t.Fatalf("newly discovered region not valid: %q", region)
-	}
-}