Try deleting machine pool user data file from S3 when pruning an old …

…launch template version, add S3 storage tests (#601)

api/v1beta2/tags.go

@@ -195,6 +195,12 @@ const (
 	// of the bootstrap secret that was used to create the user data for the latest launch
 	// template version.
 	LaunchTemplateBootstrapDataSecret = NameAWSProviderPrefix + "bootstrap-data-secret"
+
+	// LaunchTemplateBootstrapDataHash is the tag we use to store the hash of the raw bootstrap data.
+	// If bootstrap data is stored in S3, this hash relates to that data, not to the EC2 instance
+	// user data which only references the S3 object. We store this tag on launch template versions
+	// so that S3 bootstrap data objects can be deleted when they get outdated.
+	LaunchTemplateBootstrapDataHash = NameAWSProviderPrefix + "bootstrap-data-hash"
 )
 
 // ClusterTagKey generates the key for resources associated with a cluster.

exp/controllers/awsmachinepool_controller.go

@@ -412,7 +412,7 @@ func (r *AWSMachinePoolReconciler) reconcileDelete(machinePoolScope *scope.Machi
 	}
 
 	launchTemplateID := machinePoolScope.AWSMachinePool.Status.LaunchTemplateID
-	launchTemplate, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
+	launchTemplate, _, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
 	if err != nil {
 		return err
 	}

exp/controllers/awsmanagedmachinepool_controller.go

@@ -269,7 +269,7 @@ func (r *AWSManagedMachinePoolReconciler) reconcileDelete(
 
 	if machinePoolScope.ManagedMachinePool.Spec.AWSLaunchTemplate != nil {
 		launchTemplateID := machinePoolScope.ManagedMachinePool.Status.LaunchTemplateID
-		launchTemplate, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
+		launchTemplate, _, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
 		if err != nil {
 			return err
 		}

pkg/cloud/services/ec2/launchtemplate_test.go

@@ -80,12 +80,19 @@ users:
 
 var testUserDataHash = userdata.ComputeHash([]byte(testUserData))
 
-func defaultEC2AndUserDataSecretKeyTags(name string, clusterName string, userDataSecretKey types.NamespacedName) []*ec2.Tag {
+var testBootstrapData = []byte("different from testUserData since bootstrap data may be in S3 while EC2 user data points to that S3 object")
+var testBootstrapDataHash = userdata.ComputeHash(testBootstrapData)
+
+func defaultEC2AndDataTags(name string, clusterName string, userDataSecretKey types.NamespacedName, bootstrapDataHash string) []*ec2.Tag {
 	tags := defaultEC2Tags(name, clusterName)
 	tags = append(tags, &ec2.Tag{
 		Key:   aws.String(infrav1.LaunchTemplateBootstrapDataSecret),
 		Value: aws.String(userDataSecretKey.String()),
 	})
+	tags = append(tags, &ec2.Tag{
+		Key:   aws.String(infrav1.LaunchTemplateBootstrapDataHash),
+		Value: aws.String(bootstrapDataHash),
+	})
 	sortTags(tags)
 	return tags
 }
@@ -295,20 +302,21 @@ func TestGetLaunchTemplate(t *testing.T) {
 				tc.expect(mockEC2Client.EXPECT())
 			}
 
-			launchTemplate, userData, _, err := s.GetLaunchTemplate(tc.launchTemplateName)
+			launchTemplate, userData, _, _, err := s.GetLaunchTemplate(tc.launchTemplateName)
 			tc.check(g, launchTemplate, userData, err)
 		})
 	}
 }
 
 func TestServiceSDKToLaunchTemplate(t *testing.T) {
 	tests := []struct {
-		name              string
-		input             *ec2.LaunchTemplateVersion
-		wantLT            *expinfrav1.AWSLaunchTemplate
-		wantHash          string
-		wantDataSecretKey *types.NamespacedName
-		wantErr           bool
+		name                  string
+		input                 *ec2.LaunchTemplateVersion
+		wantLT                *expinfrav1.AWSLaunchTemplate
+		wantUserDataHash      string
+		wantDataSecretKey     *types.NamespacedName
+		wantBootstrapDataHash *string
+		wantErr               bool
 	}{
 		{
 			name: "lots of input",
@@ -350,8 +358,9 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) {
 				SSHKeyName:         aws.String("foo-keyname"),
 				VersionNumber:      aws.Int64(1),
 			},
-			wantHash:          testUserDataHash,
-			wantDataSecretKey: nil, // respective tag is not given
+			wantUserDataHash:      testUserDataHash,
+			wantDataSecretKey:     nil, // respective tag is not given
+			wantBootstrapDataHash: nil, // respective tag is not given
 		},
 		{
 			name: "tag of bootstrap secret",
@@ -388,6 +397,10 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) {
 									Key:   aws.String("sigs.k8s.io/cluster-api-provider-aws/bootstrap-data-secret"),
 									Value: aws.String("bootstrap-secret-ns/bootstrap-secret"),
 								},
+								{
+									Key:   aws.String("sigs.k8s.io/cluster-api-provider-aws/bootstrap-data-hash"),
+									Value: aws.String(testBootstrapDataHash),
+								},
 							},
 						},
 					},
@@ -404,26 +417,29 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) {
 				SSHKeyName:         aws.String("foo-keyname"),
 				VersionNumber:      aws.Int64(1),
 			},
-			wantHash:          testUserDataHash,
-			wantDataSecretKey: &types.NamespacedName{Namespace: "bootstrap-secret-ns", Name: "bootstrap-secret"},
+			wantUserDataHash:      testUserDataHash,
+			wantDataSecretKey:     &types.NamespacedName{Namespace: "bootstrap-secret-ns", Name: "bootstrap-secret"},
+			wantBootstrapDataHash: &testBootstrapDataHash,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
 			s := &Service{}
-			gotLT, gotHash, gotDataSecretKey, err := s.SDKToLaunchTemplate(tt.input)
+			gotLT, gotUserDataHash, gotDataSecretKey, gotBootstrapDataHash, err := s.SDKToLaunchTemplate(tt.input)
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("error mismatch: got %v, wantErr %v", err, tt.wantErr)
 			}
 			if !cmp.Equal(gotLT, tt.wantLT) {
 				t.Fatalf("launchTemplate mismatch: got %v, want %v", gotLT, tt.wantLT)
 			}
-			if !cmp.Equal(gotHash, tt.wantHash) {
-				t.Fatalf("userDataHash mismatch: got %v, want %v", gotHash, tt.wantHash)
+			if !cmp.Equal(gotUserDataHash, tt.wantUserDataHash) {
+				t.Fatalf("userDataHash mismatch: got %v, want %v", gotUserDataHash, tt.wantUserDataHash)
 			}
 			if !cmp.Equal(gotDataSecretKey, tt.wantDataSecretKey) {
 				t.Fatalf("userDataSecretKey mismatch: got %v, want %v", gotDataSecretKey, tt.wantDataSecretKey)
 			}
+			g.Expect(gotBootstrapDataHash).To(Equal(tt.wantBootstrapDataHash))
 		})
 	}
 }
@@ -845,7 +861,7 @@ func TestCreateLaunchTemplate(t *testing.T) {
 						TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
 							{
 								ResourceType: aws.String(ec2.ResourceTypeInstance),
-								Tags:         defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
+								Tags:         defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
 							},
 							{
 								ResourceType: aws.String(ec2.ResourceTypeVolume),
@@ -905,7 +921,7 @@ func TestCreateLaunchTemplate(t *testing.T) {
 						TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
 							{
 								ResourceType: aws.String(ec2.ResourceTypeInstance),
-								Tags:         defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
+								Tags:         defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
 							},
 							{
 								ResourceType: aws.String(ec2.ResourceTypeVolume),
@@ -967,7 +983,7 @@ func TestCreateLaunchTemplate(t *testing.T) {
 						TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
 							{
 								ResourceType: aws.String(ec2.ResourceTypeInstance),
-								Tags:         defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
+								Tags:         defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
 							},
 							{
 								ResourceType: aws.String(ec2.ResourceTypeVolume),
@@ -1022,7 +1038,7 @@ func TestCreateLaunchTemplate(t *testing.T) {
 				tc.expect(g, mockEC2Client.EXPECT())
 			}
 
-			launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, userData)
+			launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, userData, testBootstrapDataHash)
 			tc.check(g, launchTemplate, err)
 		})
 	}
@@ -1050,7 +1066,7 @@ func TestLaunchTemplateDataCreation(t *testing.T) {
 			Namespace: "bootstrap-secret-ns",
 			Name:      "bootstrap-secret",
 		}
-		launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, nil)
+		launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, nil, "")
 		g.Expect(err).To(HaveOccurred())
 		g.Expect(launchTemplate).Should(BeEmpty())
 	})
@@ -1104,7 +1120,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) {
 						TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
 							{
 								ResourceType: aws.String(ec2.ResourceTypeInstance),
-								Tags:         defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
+								Tags:         defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
 							},
 							{
 								ResourceType: aws.String(ec2.ResourceTypeVolume),
@@ -1155,7 +1171,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) {
 						TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
 							{
 								ResourceType: aws.String(ec2.ResourceTypeInstance),
-								Tags:         defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
+								Tags:         defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
 							},
 							{
 								ResourceType: aws.String(ec2.ResourceTypeVolume),
@@ -1202,10 +1218,10 @@ func TestCreateLaunchTemplateVersion(t *testing.T) {
 				tc.expect(mockEC2Client.EXPECT())
 			}
 			if tc.wantErr {
-				g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData)).To(HaveOccurred())
+				g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData, testBootstrapDataHash)).To(HaveOccurred())
 				return
 			}
-			g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData)).NotTo(HaveOccurred())
+			g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData, testBootstrapDataHash)).NotTo(HaveOccurred())
 		})
 	}
 }
@@ -1218,6 +1234,7 @@ func TestBuildLaunchTemplateTagSpecificationRequest(t *testing.T) {
 		Namespace: "bootstrap-secret-ns",
 		Name:      "bootstrap-secret",
 	}
+	bootstrapDataHash := userdata.ComputeHash([]byte("shell-script"))
 	testCases := []struct {
 		name  string
 		check func(g *WithT, m []*ec2.LaunchTemplateTagSpecificationRequest)
@@ -1228,7 +1245,7 @@ func TestBuildLaunchTemplateTagSpecificationRequest(t *testing.T) {
 				expected := []*ec2.LaunchTemplateTagSpecificationRequest{
 					{
 						ResourceType: aws.String(ec2.ResourceTypeInstance),
-						Tags:         defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
+						Tags:         defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, bootstrapDataHash),
 					},
 					{
 						ResourceType: aws.String(ec2.ResourceTypeVolume),
@@ -1258,7 +1275,7 @@ func TestBuildLaunchTemplateTagSpecificationRequest(t *testing.T) {
 			g.Expect(err).NotTo(HaveOccurred())
 
 			s := NewService(cs)
-			tc.check(g, s.buildLaunchTemplateTagSpecificationRequest(ms, userDataSecretKey))
+			tc.check(g, s.buildLaunchTemplateTagSpecificationRequest(ms, userDataSecretKey, bootstrapDataHash))
 		})
 	}
 }

pkg/cloud/services/interfaces.go

@@ -17,6 +17,7 @@ limitations under the License.
 package services
 
 import (
+	"github.com/aws/aws-sdk-go/service/ec2"
 	apimachinerytypes "k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 
@@ -70,12 +71,12 @@ type EC2Interface interface {
 	DetachSecurityGroupsFromNetworkInterface(groups []string, interfaceID string) error
 
 	DiscoverLaunchTemplateAMI(scope scope.LaunchTemplateScope) (*string, error)
-	GetLaunchTemplate(id string) (lt *expinfrav1.AWSLaunchTemplate, userDataHash string, userDataSecretKey *apimachinerytypes.NamespacedName, err error)
+	GetLaunchTemplate(id string) (lt *expinfrav1.AWSLaunchTemplate, userDataHash string, userDataSecretKey *apimachinerytypes.NamespacedName, bootstrapDataHash *string, err error)
 	GetLaunchTemplateID(id string) (string, error)
 	GetLaunchTemplateLatestVersion(id string) (string, error)
-	CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) (string, error)
-	CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) error
-	PruneLaunchTemplateVersions(id string) error
+	CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte, bootstrapDataHash string) (string, error)
+	CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte, bootstrapDataHash string) error
+	PruneLaunchTemplateVersions(id string) (*ec2.LaunchTemplateVersion, error)
 	DeleteLaunchTemplate(id string) error
 	LaunchTemplateNeedsUpdate(scope scope.LaunchTemplateScope, incoming *expinfrav1.AWSLaunchTemplate, existing *expinfrav1.AWSLaunchTemplate) (bool, error)
 	DeleteBastion() error
@@ -132,4 +133,5 @@ type ObjectStoreInterface interface {
 	Delete(m *scope.MachineScope) error
 	Create(m *scope.MachineScope, data []byte) (objectURL string, err error)
 	CreateForMachinePool(scope scope.LaunchTemplateScope, data []byte) (objectURL string, err error)
+	DeleteForMachinePool(scope scope.LaunchTemplateScope, bootstrapDataHash string) error
 }