Merge pull request #2342 from AndrewSirenko/imds-fix

Skip IMDS call if AWS_EC2_METADATA_DISABLED=true

Author: k8s-ci-robot

pkg/cloud/metadata/metadata.go

@@ -42,17 +42,26 @@ type MetadataServiceConfig struct {
 
 var _ MetadataService = &Metadata{}
 
+// NewMetadataService retrieves instance Metadata from one of the client in MetadataServiceConfig.
+// It prefers EC2MetadataClient (IMDS) in order to get an accurate number of attached devices.
 func NewMetadataService(cfg MetadataServiceConfig, region string) (MetadataService, error) {
-	metadata, err := retrieveEC2Metadata(cfg.EC2MetadataClient, region)
-	if err == nil {
-		klog.InfoS("Retrieved metadata from IMDS")
-		return metadata.overrideRegion(region), nil
+	// Don't make an IMDS call if we know it's disabled
+	if os.Getenv("AWS_EC2_METADATA_DISABLED") == "true" {
+		klog.V(2).InfoS("Environment variable AWS_EC2_METADATA_DISABLED set to 'true'. Will not rely on IMDS for instance metadata")
+	} else {
+		klog.V(2).InfoS("Attempting to retrieve instance metadata from IMDS")
+		metadata, err := retrieveEC2Metadata(cfg.EC2MetadataClient, region)
+		if err == nil {
+			klog.V(2).InfoS("Retrieved metadata from IMDS")
+			return metadata.overrideRegion(region), nil
+		}
+		klog.ErrorS(err, "Retrieving IMDS metadata failed, falling back to Kubernetes metadata")
 	}
-	klog.ErrorS(err, "Retrieving IMDS metadata failed, falling back to Kubernetes metadata")
 
-	metadata, err = retrieveK8sMetadata(cfg.K8sAPIClient)
+	klog.V(2).InfoS("Attempting to retrieve instance metadata from Kubernetes API")
+	metadata, err := retrieveK8sMetadata(cfg.K8sAPIClient)
 	if err == nil {
-		klog.InfoS("Retrieved metadata from Kubernetes")
+		klog.V(2).InfoS("Retrieved metadata from Kubernetes")
 		return metadata.overrideRegion(region), nil
 	}
 	klog.ErrorS(err, "Retrieving Kubernetes metadata failed")
@@ -61,11 +70,6 @@ func NewMetadataService(cfg MetadataServiceConfig, region string) (MetadataServi
 }
 
 func retrieveEC2Metadata(ec2MetadataClient EC2MetadataClient, region string) (*Metadata, error) {
-	envValue := os.Getenv("AWS_EC2_METADATA_DISABLED")
-	if envValue != "" {
-		klog.InfoS("The AWS_EC2_METADATA_DISABLED environment variable disables access to EC2 IMDS", "enabled", envValue)
-	}
-
 	svc, err := ec2MetadataClient()
 	if err != nil {
 		klog.ErrorS(err, "failed to initialize EC2 Metadata client")

pkg/cloud/metadata/metadata_test.go

@@ -39,15 +39,14 @@ func TestNewMetadataService(t *testing.T) {
 
 	testCases := []struct {
 		name             string
-		region           string
+		imdsDisabled     bool
 		ec2MetadataError error
 		k8sAPIError      error
 		expectedMetadata *Metadata
 		expectedError    error
 	}{
 		{
-			name:   "TestNewMetadataService: EC2 metadata available",
-			region: "us-west-2",
+			name: "TestNewMetadataService: EC2 metadata available",
 			expectedMetadata: &Metadata{
 				InstanceID:             "i-1234567890abcdef0",
 				InstanceType:           "c5.xlarge",
@@ -57,9 +56,20 @@ func TestNewMetadataService(t *testing.T) {
 				NumBlockDeviceMappings: 2,
 			},
 		},
+		{
+			name:         "TestNewMetadataService: AWS_EC2_METADATA_DISABLED=true, K8s API available",
+			imdsDisabled: true,
+			expectedMetadata: &Metadata{
+				InstanceID:             "i-1234567890abcdef0",
+				InstanceType:           "c5.xlarge",
+				Region:                 "us-west-2",
+				AvailabilityZone:       "us-west-2a",
+				NumAttachedENIs:        1,
+				NumBlockDeviceMappings: 0,
+			},
+		},
 		{
 			name:             "TestNewMetadataService: EC2 metadata error, K8s API available",
-			region:           "us-west-2",
 			ec2MetadataError: errors.New("EC2 metadata error"),
 			expectedMetadata: &Metadata{
 				InstanceID:             "i-1234567890abcdef0",
@@ -72,7 +82,6 @@ func TestNewMetadataService(t *testing.T) {
 		},
 		{
 			name:             "TestNewMetadataService: EC2 metadata error, K8s API error",
-			region:           "us-west-2",
 			ec2MetadataError: errors.New("EC2 metadata error"),
 			k8sAPIError:      errors.New("K8s API error"),
 			expectedError:    errors.New("IMDS metadata and Kubernetes metadata are both unavailable"),
@@ -103,8 +112,13 @@ func TestNewMetadataService(t *testing.T) {
 			}
 
 			t.Setenv("CSI_NODE_NAME", "test-node")
+			if tc.imdsDisabled {
+				t.Setenv("AWS_EC2_METADATA_DISABLED", "true")
+			} else {
+				t.Setenv("AWS_EC2_METADATA_DISABLED", "false")
+			}
 
-			if tc.ec2MetadataError == nil {
+			if tc.ec2MetadataError == nil && !tc.imdsDisabled {
 				mockEC2Metadata.EXPECT().GetInstanceIdentityDocument(gomock.Any(), &imds.GetInstanceIdentityDocumentInput{}).Return(&imds.GetInstanceIdentityDocumentOutput{
 					InstanceIdentityDocument: imds.InstanceIdentityDocument{
 						InstanceID:       "i-1234567890abcdef0",
@@ -132,7 +146,7 @@ func TestNewMetadataService(t *testing.T) {
 				K8sAPIClient: mockK8sClient,
 			}
 
-			metadata, err := NewMetadataService(cfg, tc.region)
+			metadata, err := NewMetadataService(cfg, "us-west-2")
 
 			if tc.expectedError != nil {
 				require.EqualError(t, err, tc.expectedError.Error())