*: prefer SSM param over AMI ID, do not include "SSM:Parameter" in CFN (

#187) * *: prefer SSM param over AMI ID, do not include "SSM:Parameter" in CFN if empty Signed-off-by: Gyuho Lee <leegyuho@amazon.com> * *: add CHANGELOG, update AWS SDK Go Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
aws · Nov 12, 2020 · 01d10ba · 01d10ba
1 parent cbc2bde
commit 01d10ba
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 33 deletions.
diff --git a/CHANGELOG/CHANGELOG-1.5.md b/CHANGELOG/CHANGELOG-1.5.md
@@ -3,6 +3,35 @@
 <hr>
 
 
+## [v1.5.5](https://github.com/aws/aws-k8s-tester/releases/tag/v1.5.5) (2020-11-12)
+
+See [code changes](https://github.com/aws/aws-k8s-tester/compare/v1.5.4...v1.5.5).
+
+### `ec2config`
+
+- Overwrite [ASG AMI ID if SSM parameter is specified](https://github.com/aws/aws-k8s-tester/pull/187).
+
+### `eksconfig`
+
+- Overwrite [node group AMI ID if SSM parameter is specified](https://github.com/aws/aws-k8s-tester/pull/187).
+
+### `eks`
+
+- Do [not include `AWS::SSM::Parameter` in node group CFN template if the parameter is empty](https://github.com/aws/aws-k8s-tester/pull/187).
+- Skip [deleting CMK, VPC, IAM role if EKS cluster delete fails](https://github.com/aws/aws-k8s-tester/pull/186).
+
+### Dependency
+
+- Upgrade [`github.com/aws/aws-sdk-go`](https://github.com/aws/aws-sdk-go/releases) from [`v1.35.25`](https://github.com/aws/aws-sdk-go/releases/tag/v1.35.25) to [`v1.35.27`](https://github.com/aws/aws-sdk-go/releases/tag/v1.35.27).
+
+### Go
+
+- Compile with [*Go 1.15.5*](https://golang.org/doc/devel/release.html#go1.15).
+
+
+<hr>
+
+
 ## [v1.5.4](https://github.com/aws/aws-k8s-tester/releases/tag/v1.5.4) (2020-11-11)
 
 See [code changes](https://github.com/aws/aws-k8s-tester/compare/v1.5.3...v1.5.4).

diff --git a/ec2config/config.go b/ec2config/config.go
@@ -278,6 +278,7 @@ type ASG struct {
 	AMIType string `json:"ami-type,omitempty"`
 	// ImageID is the Amazon Machine Image (AMI).
 	// This value overrides any AWS Systems Manager Parameter Store value.
+	// NOTE: THIS FIELD IS SET TO EMPTY IF "ImageIDSSMParameter" IS NOT EMPTY.
 	ImageID string `json:"image-id"`
 	// ImageIDSSMParameter is the AWS Systems Manager Parameter Store
 	// parameter of the AMI ID.

diff --git a/ec2config/validate-defaults.go b/ec2config/validate-defaults.go
@@ -379,6 +379,10 @@ func (cfg *Config) validateASGs() error {
 		if cur.ImageID == "" && cur.ImageIDSSMParameter == "" {
 			return fmt.Errorf("%q both ImageID and ImageIDSSMParameter are empty", cur.Name)
 		}
+		// prefer "ImageIDSSMParameter"
+		if cur.ImageID != "" && cur.ImageIDSSMParameter != "" {
+			cur.ImageID = ""
+		}
 
 		switch cur.AMIType {
 		case AMITypeBottleRocketCPU:

diff --git a/eks/ng/nodes.go b/eks/ng/nodes.go
@@ -33,14 +33,41 @@ e.g.
 aws ssm get-parameters --names /aws/service/ami-amazon-linux-latest/amzn2-ami-hvm-x86_64-gp2
 
 e.g.
-aws ssm get-parameters --names /aws/service/eks/optimized-ami/1.16/amazon-linux-2/recommended/image_id
-aws ssm get-parameters --names /aws/service/bottlerocket/aws-k8s-1.16/x86_64/latest/image_id
+aws ssm get-parameters --names /aws/service/eks/optimized-ami/1.18/amazon-linux-2/recommended/image_id
+aws ssm get-parameters --names /aws/service/bottlerocket/aws-k8s-1.18/x86_64/latest/image_id
 
 TODO
 
   BootstrapArguments:
     Type: String
     Description: Arguments to pass to the bootstrap script. See files/bootstrap.sh in https://github.com/awslabs/amazon-eks-ami
+
+
+NOTE for new regions
+"AWS::SSM::Parameter" may not be onboarded yet, so we need templatize CFN template
+so that we do not pass invalid "AWS::SSM::Parameter" at all in those regions
+
+  ImageID:
+    Type: String
+    Default: ""
+    Description: (Optional) Specify your own custom image ID. This value overrides any AWS Systems Manager Parameter Store value specified above.
+
+  ImageIDSSMParameter:
+    Type: AWS::SSM::Parameter::Value<AWS::EC2::Image::Id>
+    Default: /aws/service/eks/optimized-ami/1.18/amazon-linux-2/recommended/image_id
+    Description: AWS Systems Manager Parameter Store parameter of the AMI ID for the worker node instances.
+
+  HasImageID:
+    Fn::Not:
+      - Fn::Equals:
+          - Ref: ImageID
+          - ""
+
+        ImageId:
+          Fn::If:
+            - HasImageID
+            - !Ref ImageID
+            - !Ref ImageIDSSMParameter
 */
 
 // TemplateASG is the CloudFormation template for EKS node group.
@@ -83,15 +110,13 @@ Parameters:
     Type: AWS::EC2::KeyPair::KeyName
     Description: The EC2 Key Pair to allow SSH access to the instances
 
-  ImageID:
+{{ if ne .ImageID "" }}  ImageID:
     Type: String
     Default: ""
-    Description: (Optional) Specify your own custom image ID. This value overrides any AWS Systems Manager Parameter Store value specified above.
-
-  ImageIDSSMParameter:
-    Type: AWS::SSM::Parameter::Value<AWS::EC2::Image::Id>
-    Default: /aws/service/eks/optimized-ami/1.15/amazon-linux-2/recommended/image_id
-    Description: AWS Systems Manager Parameter Store parameter of the AMI ID for the worker node instances.
+    Description: Specify your own custom image ID. This value overrides any AWS Systems Manager Parameter Store value specified above.{{ end }}{{ if ne .ImageIDSSMParameter "" }}  ImageIDSSMParameter:
+  Type: AWS::SSM::Parameter::Value<AWS::EC2::Image::Id>
+  Default: /aws/service/eks/optimized-ami/1.18/amazon-linux-2/recommended/image_id
+  Description: AWS Systems Manager Parameter Store parameter of the AMI ID for the worker node instances.{{ end }}
 
   InstanceTypes:
     Type: CommaDelimitedList
@@ -129,12 +154,6 @@ Parameters:
 
 Conditions:
 
-  HasImageID:
-    Fn::Not:
-      - Fn::Equals:
-          - Ref: ImageID
-          - ""
-
   Has2InstanceTypes:
     Fn::Or:
       - Fn::Equals:
@@ -181,11 +200,7 @@ Resources:
       LaunchTemplateData:
         IamInstanceProfile:
           Arn: !GetAtt InstanceProfile.Arn
-        ImageId:
-          Fn::If:
-            - HasImageID
-            - !Ref ImageID
-            - !Ref ImageIDSSMParameter
+{{ if ne .ImageID "" }}        ImageId: !Ref ImageID{{ end }}{{ if ne .ImageIDSSMParameter "" }}        ImageId: !Ref ImageIDSSMParameter{{ end }}
         KeyName: !Ref RemoteAccessKeyName
         BlockDeviceMappings:
         - DeviceName: /dev/xvda
@@ -374,10 +389,12 @@ const asgTagDataNG = `      - Key: !Sub k8s.io/cluster-autoscaler/${ClusterName}
 `
 
 type templateASG struct {
-	Metadata           string
-	UserData           string
-	ASGDesiredCapacity int64
-	ASGTagData         string
+	ImageID             string
+	ImageIDSSMParameter string
+	Metadata            string
+	UserData            string
+	ASGDesiredCapacity  int64
+	ASGTagData          string
 }
 
 func (ts *tester) createASGs() error {
@@ -400,7 +417,9 @@ func (ts *tester) createASGs() error {
 		// "/aws/service/ami-amazon-linux-latest/amzn2-ami-hvm-x86_64-gp2"
 		// already includes SSM agent + AWS CLI
 		tg := templateASG{
-			ASGDesiredCapacity: cur.ASGDesiredCapacity,
+			ImageID:             cur.ImageID,
+			ImageIDSSMParameter: cur.ImageIDSSMParameter,
+			ASGDesiredCapacity:  cur.ASGDesiredCapacity,
 		}
 		switch cur.AMIType {
 		case ec2config.AMITypeBottleRocketCPU:
@@ -517,6 +536,9 @@ func (ts *tester) createASGs() error {
 				},
 			},
 		}
+
+		// "eksconfig" validate already ensures that either "ImageID" or "ImageIDSSMParameter" is non-empty
+		// both cannot be non-empty at the same time!
 		if cur.ImageID != "" {
 			ts.cfg.Logger.Info("added image ID", zap.String("image-id", cur.ImageID))
 			stackInput.Parameters = append(stackInput.Parameters, &cloudformation.Parameter{

diff --git a/eks/ng/nodes_test.go b/eks/ng/nodes_test.go
@@ -3,6 +3,7 @@ package ng
 import (
 	"bytes"
 	"fmt"
+	"strings"
 	"testing"
 	"text/template"
 )
@@ -17,11 +18,16 @@ func TestTemplateASG(t *testing.T) {
 
 	buf.Reset()
 	if err := tpl.Execute(buf, templateASG{
-		Metadata:           metadataAL2InstallSSM,
-		UserData:           userDataAL2InstallSSM,
-		ASGDesiredCapacity: 1,
+		ImageID:             "abc",
+		ImageIDSSMParameter: "",
+		Metadata:            metadataAL2InstallSSM,
+		UserData:            userDataAL2InstallSSM,
+		ASGDesiredCapacity:  1,
 	}); err != nil {
 		t.Fatal(err)
 	}
 	fmt.Println(buf.String())
+	if strings.Contains(buf.String(), "AWS::SSM::Parameter") {
+		t.Fatal("unexpected AWS::SSM::Parameter in CFN template")
+	}
 }
diff --git a/eksconfig/add-on-node-groups.go b/eksconfig/add-on-node-groups.go
@@ -280,6 +280,10 @@ func (cfg *Config) validateAddOnNodeGroups() error {
 		if cur.ImageID == "" && cur.ImageIDSSMParameter == "" {
 			return fmt.Errorf("%q both ImageID and ImageIDSSMParameter are empty", cur.Name)
 		}
+		// prefer "ImageIDSSMParameter"
+		if cur.ImageID != "" && cur.ImageIDSSMParameter != "" {
+			cur.ImageID = ""
+		}
 
 		if !cfg.AddOnNodeGroups.RoleCreate {
 			if cur.ClusterAutoscaler != nil && cur.ClusterAutoscaler.Enable {

diff --git a/go.mod b/go.mod
@@ -43,7 +43,7 @@ replace (
 )
 
 require (
-	github.com/aws/aws-sdk-go v1.35.25
+	github.com/aws/aws-sdk-go v1.35.27
 	github.com/briandowns/spinner v1.11.1
 	github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575
 	github.com/davecgh/go-spew v1.1.1

diff --git a/go.sum b/go.sum
@@ -102,8 +102,8 @@ github.com/auth0/go-jwt-middleware v0.0.0-20170425171159-5493cabe49f7/go.mod h1:
 github.com/aws/aws-sdk-go v1.6.10/go.mod h1:ZRmQr0FajVIyZ4ZzBYKG5P3ZqPz9IHG41ZoMu1ADI3k=
 github.com/aws/aws-sdk-go v1.15.11/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0=
 github.com/aws/aws-sdk-go v1.28.2/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
-github.com/aws/aws-sdk-go v1.35.25 h1:0+UC6ZquMOLvYABoz0olShCAe+M9oKllgPfr2hnv9zE=
-github.com/aws/aws-sdk-go v1.35.25/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
+github.com/aws/aws-sdk-go v1.35.27 h1:F0dUW+kouzchjt4X6kYfYMw1YtQPkA4pihpCDqQMrq8=
+github.com/aws/aws-sdk-go v1.35.27/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
 github.com/bazelbuild/bazel-gazelle v0.18.2/go.mod h1:D0ehMSbS+vesFsLGiD6JXu3mVEzOlfUl8wNnq+x/9p0=
 github.com/bazelbuild/bazel-gazelle v0.19.1-0.20191105222053-70208cbdc798/go.mod h1:rPwzNHUqEzngx1iVBfO/2X2npKaT3tqPqqHW6rVsn/A=
 github.com/bazelbuild/buildtools v0.0.0-20190731111112-f720930ceb60/go.mod h1:5JP0TXzWDHXv8qvxRC4InIazwdyDseBDbzESUMKk1yU=

diff --git a/vendor/github.com/aws/aws-sdk-go/aws/version.go b/vendor/github.com/aws/aws-sdk-go/aws/version.go
diff --git a/vendor/modules.txt b/vendor/modules.txt
@@ -84,7 +84,7 @@ github.com/andybalholm/brotli
 github.com/armon/circbuf
 # github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496
 github.com/asaskevich/govalidator
-# github.com/aws/aws-sdk-go v1.35.25
+# github.com/aws/aws-sdk-go v1.35.27
 ## explicit
 github.com/aws/aws-sdk-go/aws
 github.com/aws/aws-sdk-go/aws/arn