Merge pull request #38880 from justinsb/peninsula_club

Automatic merge from submit-queue

AWS: trust region if found from AWS metadata

```release-note
AWS: trust region if found from AWS metadata
```

Means we can run in newly announced regions without a code change.

We don't register the ECR provider in new regions, so we will still need
a code change for now.

Fix #35014

pkg/cloudprovider/providers/aws/BUILD

@@ -18,6 +18,7 @@ go_library(
         "aws_utils.go",
         "device_allocator.go",
         "log_handler.go",
+        "regions.go",
         "retry_handler.go",
         "sets_ippermissions.go",
         "volumes.go",
@@ -53,6 +54,7 @@ go_test(
     srcs = [
         "aws_test.go",
         "device_allocator_test.go",
+        "regions_test.go",
         "retry_handler_test.go",
     ],
     library = ":go_default_library",

pkg/cloudprovider/providers/aws/aws.go

@@ -48,7 +48,6 @@ import (
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/api/v1/service"
 	"k8s.io/kubernetes/pkg/cloudprovider"
-	awscredentials "k8s.io/kubernetes/pkg/credentialprovider/aws"
 	"k8s.io/kubernetes/pkg/volume"
 )
 
@@ -182,7 +181,7 @@ const DefaultVolumeType = "gp2"
 // See http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/volume_limits.html#linux-specific-volume-limits
 const DefaultMaxEBSVolumes = 39
 
-// Used to call awscredentials.Init() just once
+// Used to call RecognizeWellKnownRegions just once
 var once sync.Once
 
 // Services is an abstraction over AWS, to allow mocking/other implementations
@@ -691,6 +690,7 @@ func init() {
 				},
 				&credentials.SharedCredentialsProvider{},
 			})
+
 		aws := newAWSSDKProvider(creds)
 		return newAWSCloud(config, aws)
 	})
@@ -732,15 +732,6 @@ func getAvailabilityZone(metadata EC2Metadata) (string, error) {
 	return metadata.GetMetadata("placement/availability-zone")
 }
 
-func isRegionValid(region string) bool {
-	for _, r := range awscredentials.AWSRegions {
-		if r == region {
-			return true
-		}
-	}
-	return false
-}
-
 // Derives the region from a valid az name.
 // Returns an error if the az is known invalid (empty)
 func azToRegion(az string) (string, error) {
@@ -777,9 +768,14 @@ func newAWSCloud(config io.Reader, 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 !valid {
+			// This _should_ now be unreachable, given we call RecognizeRegion
 			return nil, fmt.Errorf("not a valid AWS zone (unknown region): %s", zone)
 		}
 	} else {
@@ -848,9 +844,9 @@ func newAWSCloud(config io.Reader, awsServices Services) (*Cloud, error) {
 		glog.Infof("AWS cloud - no tag filtering")
 	}
 
-	// Register handler for ECR credentials
+	// Register regions, in particular for ECR credentials
 	once.Do(func() {
-		awscredentials.Init()
+		RecognizeWellKnownRegions()
 	})
 
 	return awsCloud, nil

pkg/cloudprovider/providers/aws/aws_test.go

@@ -210,11 +210,6 @@ func TestNewAWSCloud(t *testing.T) {
 			nil, NewFakeAWSServices().withAz(""),
 			true, "",
 		},
-		{
-			"Config specified invalid zone",
-			strings.NewReader("[global]\nzone = blahonga"), NewFakeAWSServices(),
-			true, "",
-		},
 		{
 			"Config specifies valid zone",
 			strings.NewReader("[global]\nzone = eu-west-1a"), NewFakeAWSServices(),

pkg/cloudprovider/providers/aws/regions.go

@@ -0,0 +1,94 @@
+/*
+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 (
+	"github.com/golang/glog"
+	"k8s.io/apimachinery/pkg/util/sets"
+	awscredentialprovider "k8s.io/kubernetes/pkg/credentialprovider/aws"
+	"sync"
+)
+
+// 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-south-1",
+	"ap-southeast-1",
+	"ap-southeast-2",
+	"ca-central-1",
+	"eu-central-1",
+	"eu-west-1",
+	"eu-west-2",
+	"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",
+	"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) {
+		glog.V(6).Infof("found AWS region %q again - ignoring", region)
+		return
+	}
+
+	glog.V(4).Infof("found AWS region %q", region)
+
+	awscredentialprovider.RegisterCredentialsProvider(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)
+}

pkg/cloudprovider/providers/aws/regions_test.go

@@ -0,0 +1,85 @@
+/*
+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().withAz(region + "a")
+	_, err := newAWSCloud(nil, awsServices)
+	if err != nil {
+		t.Errorf("error building AWS cloud: %v", err)
+	}
+
+	if !isRegionValid(region) {
+		t.Fatalf("newly discovered region not valid: %q", region)
+	}
+}

pkg/credentialprovider/aws/aws_credentials.go

@@ -30,27 +30,6 @@ import (
 	"k8s.io/kubernetes/pkg/credentialprovider"
 )
 
-// AWSRegions is the complete list of regions known to the AWS cloudprovider
-// and credentialprovider.
-var AWSRegions = [...]string{
-	"us-east-1",
-	"us-east-2",
-	"us-west-1",
-	"us-west-2",
-	"eu-west-1",
-	"eu-west-2",
-	"eu-central-1",
-	"ap-south-1",
-	"ap-southeast-1",
-	"ap-southeast-2",
-	"ap-northeast-1",
-	"ap-northeast-2",
-	"ca-central-1",
-	"cn-north-1",
-	"us-gov-west-1",
-	"sa-east-1",
-}
-
 const registryURLTemplate = "*.dkr.ecr.%s.amazonaws.com"
 
 // awsHandlerLogger is a handler that logs all AWS SDK requests
@@ -101,21 +80,20 @@ type ecrProvider struct {
 
 var _ credentialprovider.DockerConfigProvider = &ecrProvider{}
 
-// Init creates a lazy provider for each AWS region, in order to support
+// RegisterCredentialsProvider registers a credential provider for the specified region.
+// It creates a lazy provider for each AWS region, in order to support
 // cross-region ECR access. They have to be lazy because it's unlikely, but not
 // impossible, that we'll use more than one.
-// Not using the package init() function: this module should be initialized only
+// This should be called only if using the AWS cloud provider.
-// if using the AWS cloud provider. This way, we avoid timeouts waiting for a
+// This way, we avoid timeouts waiting for a non-existent provider.
-// non-existent provider.
+func RegisterCredentialsProvider(region string) {
-func Init() {
+	glog.V(4).Infof("registering credentials provider for AWS region %q", region)
-	for _, region := range AWSRegions {
-		credentialprovider.RegisterCredentialProvider("aws-ecr-"+region,
-			&lazyEcrProvider{
-				region:    region,
-				regionURL: fmt.Sprintf(registryURLTemplate, region),
-			})
-	}
 
+	credentialprovider.RegisterCredentialProvider("aws-ecr-"+region,
+		&lazyEcrProvider{
+			region:    region,
+			regionURL: fmt.Sprintf(registryURLTemplate, region),
+		})
 }
 
 // Enabled implements DockerConfigProvider.Enabled for the lazy provider.