✨ Support ignition v3 and presigned URLs

vincepri · vincepri · commit a65f1f5d7875 · 2023-10-09T14:29:52.000-07:00
Signed-off-by: Vince Prignano &lt;vincepri@redhat.com&gt;
diff --git a/api/v1beta1/conversion.go b/api/v1beta1/conversion.go
@@ -94,3 +94,7 @@ func Convert_v1beta2_IPv6_To_v1beta1_IPv6(in *v1beta2.IPv6, out *IPv6, s convers
 func Convert_v1beta2_NetworkSpec_To_v1beta1_NetworkSpec(in *v1beta2.NetworkSpec, out *NetworkSpec, s conversion.Scope) error {
 	return autoConvert_v1beta2_NetworkSpec_To_v1beta1_NetworkSpec(in, out, s)
 }
+
+func Convert_v1beta2_S3Bucket_To_v1beta1_S3Bucket(in *v1beta2.S3Bucket, out *S3Bucket, s conversion.Scope) error {
+	return autoConvert_v1beta2_S3Bucket_To_v1beta1_S3Bucket(in, out, s)
+}
diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go
diff --git a/api/v1beta2/awscluster_types.go b/api/v1beta2/awscluster_types.go
@@ -239,11 +239,22 @@ type AWSClusterStatus struct {
 type S3Bucket struct {
 	// ControlPlaneIAMInstanceProfile is a name of the IAMInstanceProfile, which will be allowed
 	// to read control-plane node bootstrap data from S3 Bucket.
-	ControlPlaneIAMInstanceProfile string `json:"controlPlaneIAMInstanceProfile"`
+	// +optional
+	ControlPlaneIAMInstanceProfile string `json:"controlPlaneIAMInstanceProfile,omitempty"`
 
 	// NodesIAMInstanceProfiles is a list of IAM instance profiles, which will be allowed to read
 	// worker nodes bootstrap data from S3 Bucket.
-	NodesIAMInstanceProfiles []string `json:"nodesIAMInstanceProfiles"`
+	// +optional
+	NodesIAMInstanceProfiles []string `json:"nodesIAMInstanceProfiles,omitempty"`
+
+	// PresignedURLDuration defines the duration for which presigned URLs are valid.
+	//
+	// This is used to generate presigned URLs for S3 Bucket objects, which are used by
+	// control-plane and worker nodes to fetch bootstrap data.
+	//
+	// When enabled, the IAM instance profiles specified are not used.
+	// +optional
+	PresignedURLDuration *metav1.Duration `json:"presignedURLDuration,omitempty"`
 
 	// Name defines name of S3 Bucket to be created.
 	// +kubebuilder:validation:MinLength:=3
diff --git a/api/v1beta2/awsmachine_types.go b/api/v1beta2/awsmachine_types.go
@@ -195,7 +195,7 @@ type Ignition struct {
 	//
 	// +optional
 	// +kubebuilder:default="2.3"
-	// +kubebuilder:validation:Enum="2.3"
+	// +kubebuilder:validation:Enum="2.3";"3.0";"3.1";"3.2";"3.3";"3.4"
 	Version string `json:"version,omitempty"`
 }
 
diff --git a/api/v1beta2/s3bucket.go b/api/v1beta2/s3bucket.go
@@ -43,20 +43,22 @@ func (b *S3Bucket) Validate() []*field.Error {
 			"can be set only if the BootstrapFormatIgnition feature gate is enabled"))
 	}
 
-	if b.ControlPlaneIAMInstanceProfile == "" {
-		errs = append(errs,
-			field.Required(field.NewPath("spec", "s3Bucket", "controlPlaneIAMInstanceProfiles"), "can't be empty"))
-	}
-
-	if len(b.NodesIAMInstanceProfiles) == 0 {
-		errs = append(errs,
-			field.Required(field.NewPath("spec", "s3Bucket", "nodesIAMInstanceProfiles"), "can't be empty"))
-	}
+	if b.PresignedURLDuration == nil {
+		if b.ControlPlaneIAMInstanceProfile == "" {
+			errs = append(errs,
+				field.Required(field.NewPath("spec", "s3Bucket", "controlPlaneIAMInstanceProfiles"), "can't be empty"))
+		}
 
-	for i, iamInstanceProfile := range b.NodesIAMInstanceProfiles {
-		if iamInstanceProfile == "" {
+		if len(b.NodesIAMInstanceProfiles) == 0 {
 			errs = append(errs,
-				field.Required(field.NewPath("spec", "s3Bucket", fmt.Sprintf("nodesIAMInstanceProfiles[%d]", i)), "can't be empty"))
+				field.Required(field.NewPath("spec", "s3Bucket", "nodesIAMInstanceProfiles"), "can't be empty"))
+		}
+
+		for i, iamInstanceProfile := range b.NodesIAMInstanceProfiles {
+			if iamInstanceProfile == "" {
+				errs = append(errs,
+					field.Required(field.NewPath("spec", "s3Bucket", fmt.Sprintf("nodesIAMInstanceProfiles[%d]", i)), "can't be empty"))
+			}
 		}
 	}
 
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1476,10 +1476,15 @@ spec:
                     items:
                       type: string
                     type: array
+                  presignedURLDuration:
+                    description: "PresignedURLDuration defines the duration for which
+                      presigned URLs are valid. \n This is used to generate presigned
+                      URLs for S3 Bucket objects, which are used by control-plane
+                      and worker nodes to fetch bootstrap data. \n When enabled, the
+                      IAM instance profiles specified are not used."
+                    type: string
                 required:
-                - controlPlaneIAMInstanceProfile
                 - name
-                - nodesIAMInstanceProfiles
                 type: object
               sshKeyName:
                 description: SSHKeyName is the name of the ssh key to attach to the
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml
@@ -1092,10 +1092,16 @@ spec:
                             items:
                               type: string
                             type: array
+                          presignedURLDuration:
+                            description: "PresignedURLDuration defines the duration
+                              for which presigned URLs are valid. \n This is used
+                              to generate presigned URLs for S3 Bucket objects, which
+                              are used by control-plane and worker nodes to fetch
+                              bootstrap data. \n When enabled, the IAM instance profiles
+                              specified are not used."
+                            type: string
                         required:
-                        - controlPlaneIAMInstanceProfile
                         - name
-                        - nodesIAMInstanceProfiles
                         type: object
                       sshKeyName:
                         description: SSHKeyName is the name of the ssh key to attach
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml
@@ -638,6 +638,11 @@ spec:
                       used to generate bootstrap data.
                     enum:
                     - "2.3"
+                    - "3.0"
+                    - "3.1"
+                    - "3.2"
+                    - "3.3"
+                    - "3.4"
                     type: string
                 type: object
               imageLookupBaseOS:
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml
@@ -584,6 +584,11 @@ spec:
                               will be used to generate bootstrap data.
                             enum:
                             - "2.3"
+                            - "3.0"
+                            - "3.1"
+                            - "3.2"
+                            - "3.3"
+                            - "3.4"
                             type: string
                         type: object
                       imageLookupBaseOS:
diff --git a/controllers/awsmachine_controller.go b/controllers/awsmachine_controller.go
@@ -23,7 +23,9 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
-	ignTypes "github.com/flatcar/ignition/config/v2_3/types"
+	"github.com/blang/semver"
+	ignTypes "github.com/coreos/ignition/config/v2_3/types"
+	ignV3Types "github.com/coreos/ignition/v2/config/v3_4/types"
 	"github.com/go-logr/logr"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -753,26 +755,56 @@ func (r *AWSMachineReconciler) ignitionUserData(scope *scope.MachineScope, objec
 		return nil, errors.Wrap(err, "creating userdata object")
 	}
 
-	ignData := &ignTypes.Config{
-		Ignition: ignTypes.Ignition{
-			Version: "2.3.0",
-			Config: ignTypes.IgnitionConfig{
-				Append: []ignTypes.ConfigReference{
-					{
-						Source: objectURL,
+	ignVersion := getIgnitionVersion(scope)
+	semver, err := semver.ParseTolerant(ignVersion)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to parse ignition version %q", ignVersion)
+	}
+
+	switch semver.Major {
+	case 2:
+		ignData := &ignTypes.Config{
+			Ignition: ignTypes.Ignition{
+				Version: semver.String(),
+				Config: ignTypes.IgnitionConfig{
+					Append: []ignTypes.ConfigReference{
+						{
+							Source: objectURL,
+						},
 					},
 				},
 			},
-		},
-	}
+		}
 
-	ignitionUserData, err := json.Marshal(ignData)
-	if err != nil {
-		r.Recorder.Eventf(scope.AWSMachine, corev1.EventTypeWarning, "FailedGenerateIgnition", err.Error())
-		return nil, errors.Wrap(err, "serializing generated data")
+		return json.Marshal(ignData)
+	case 3:
+		ignData := &ignV3Types.Config{
+			Ignition: ignV3Types.Ignition{
+				Version: semver.String(),
+				Config: ignV3Types.IgnitionConfig{
+					Merge: []ignV3Types.Resource{
+						{
+							Source: aws.String(objectURL),
+						},
+					},
+				},
+			},
+		}
+
+		return json.Marshal(ignData)
+	default:
+		return nil, errors.Errorf("unsupported ignition version %q", ignVersion)
 	}
+}
 
-	return ignitionUserData, nil
+func getIgnitionVersion(scope *scope.MachineScope) string {
+	if scope.AWSMachine.Spec.Ignition == nil {
+		scope.AWSMachine.Spec.Ignition = &infrav1.Ignition{}
+	}
+	if scope.AWSMachine.Spec.Ignition.Version == "" {
+		scope.AWSMachine.Spec.Ignition.Version = infrav1.DefaultIgnitionVersion
+	}
+	return scope.AWSMachine.Spec.Ignition.Version
 }
 
 func (r *AWSMachineReconciler) deleteBootstrapData(machineScope *scope.MachineScope, clusterScope cloud.ClusterScoper, objectStoreScope scope.S3Scope) error {
diff --git a/controllers/awsmachine_controller_unit_test.go b/controllers/awsmachine_controller_unit_test.go
@@ -1307,6 +1307,39 @@ func TestAWSMachineReconciler(t *testing.T) {
 				_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
 				g.Expect(err).To(BeNil())
 			})
+
+			t.Run("should leverage AWS S3 with presigned urls", func(t *testing.T) {
+				g := NewWithT(t)
+				awsMachine := getAWSMachine()
+				setup(t, g, awsMachine)
+				defer teardown(t, g)
+				getInstances(t, g)
+				useIgnition(t, g)
+
+				if cs.AWSCluster.Spec.S3Bucket == nil {
+					cs.AWSCluster.Spec.S3Bucket = &infrav1.S3Bucket{}
+				}
+				cs.AWSCluster.Spec.S3Bucket.PresignedURLDuration = &metav1.Duration{Duration: 1 * time.Hour}
+
+				instance = &infrav1.Instance{
+					ID:    "myMachine",
+					State: infrav1.InstanceStatePending,
+				}
+				presigned := "https://cluster-api-aws.s3.us-west-2.amazonaws.com/bootstrap-data.yaml?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA3SGQVQG7FGA6KKA6%2F20221104%2Fus-west-2%2Fs3%2Faws4_request&X-Amz-Date=20221104T140227Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=b228dbec8c1008c80c162e1210e4503dceead1e4d4751b4d9787314fd6da4d55"
+
+				objectStoreSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return(presigned, nil).Times(1)
+				ec2Svc.EXPECT().CreateInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(instance, nil).AnyTimes()
+				ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
+				ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
+				ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
+
+				ms.AWSMachine.ObjectMeta.Labels = map[string]string{
+					clusterv1.MachineControlPlaneLabel: "",
+				}
+
+				_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
+				g.Expect(err).To(BeNil())
+			})
 		})
 
 		t.Run("there's a node ref and a secret ARN", func(t *testing.T) {
diff --git a/go.mod b/go.mod
diff --git a/go.sum b/go.sum
diff --git a/pkg/cloud/services/s3/s3.go b/pkg/cloud/services/s3/s3.go

Original file line number	Diff line number	Diff line change
`@@ -94,3 +94,7 @@ func Convert_v1beta2_IPv6_To_v1beta1_IPv6(in v1beta2.IPv6, out IPv6, s convers`
`94`	`94`	`func Convert_v1beta2_NetworkSpec_To_v1beta1_NetworkSpec(in v1beta2.NetworkSpec, out NetworkSpec, s conversion.Scope) error {`
`95`	`95`	`return autoConvert_v1beta2_NetworkSpec_To_v1beta1_NetworkSpec(in, out, s)`
`96`	`96`	`}`
	`97`	`+`
	`98`	`+func Convert_v1beta2_S3Bucket_To_v1beta1_S3Bucket(in v1beta2.S3Bucket, out S3Bucket, s conversion.Scope) error {`
	`99`	`+ return autoConvert_v1beta2_S3Bucket_To_v1beta1_S3Bucket(in, out, s)`
	`100`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -195,7 +195,7 @@ type Ignition struct {`
`195`	`195`	`//`
`196`	`196`	`// +optional`
`197`	`197`	`// +kubebuilder:default="2.3"`
`198`		`- // +kubebuilder:validation:Enum="2.3"`
	`198`	`+ // +kubebuilder:validation:Enum="2.3";"3.0";"3.1";"3.2";"3.3";"3.4"`
`199`	`199`	Version string `json:"version,omitempty"`
`200`	`200`	`}`
`201`	`201`