diff --git a/api/v1beta1/conversion.go b/api/v1beta1/conversion.go index f18fdc678b..2b124f027a 100644 --- a/api/v1beta1/conversion.go +++ b/api/v1beta1/conversion.go @@ -98,3 +98,7 @@ func Convert_v1beta2_NetworkSpec_To_v1beta1_NetworkSpec(in *v1beta2.NetworkSpec, 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) } + +func Convert_v1beta2_Ignition_To_v1beta1_Ignition(in *v1beta2.Ignition, out *Ignition, s conversion.Scope) error { + return autoConvert_v1beta2_Ignition_To_v1beta1_Ignition(in, out, s) +} diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 3a64943cae..6fab23cc8a 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -445,11 +445,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta2.Ignition)(nil), (*Ignition)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta2_Ignition_To_v1beta1_Ignition(a.(*v1beta2.Ignition), b.(*Ignition), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*IngressRule)(nil), (*v1beta2.IngressRule)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_IngressRule_To_v1beta2_IngressRule(a.(*IngressRule), b.(*v1beta2.IngressRule), scope) }); err != nil { @@ -560,6 +555,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta2.Ignition)(nil), (*Ignition)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta2_Ignition_To_v1beta1_Ignition(a.(*v1beta2.Ignition), b.(*Ignition), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta2.IngressRule)(nil), (*IngressRule)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta2_IngressRule_To_v1beta1_IngressRule(a.(*v1beta2.IngressRule), b.(*IngressRule), scope) }); err != nil { @@ -1360,7 +1360,15 @@ func autoConvert_v1beta1_AWSMachineSpec_To_v1beta2_AWSMachineSpec(in *AWSMachine if err := Convert_v1beta1_CloudInit_To_v1beta2_CloudInit(&in.CloudInit, &out.CloudInit, s); err != nil { return err } - out.Ignition = (*v1beta2.Ignition)(unsafe.Pointer(in.Ignition)) + if in.Ignition != nil { + in, out := &in.Ignition, &out.Ignition + *out = new(v1beta2.Ignition) + if err := Convert_v1beta1_Ignition_To_v1beta2_Ignition(*in, *out, s); err != nil { + return err + } + } else { + out.Ignition = nil + } out.SpotMarketOptions = (*v1beta2.SpotMarketOptions)(unsafe.Pointer(in.SpotMarketOptions)) out.Tenancy = in.Tenancy return nil @@ -1409,7 +1417,15 @@ func autoConvert_v1beta2_AWSMachineSpec_To_v1beta1_AWSMachineSpec(in *v1beta2.AW if err := Convert_v1beta2_CloudInit_To_v1beta1_CloudInit(&in.CloudInit, &out.CloudInit, s); err != nil { return err } - out.Ignition = (*Ignition)(unsafe.Pointer(in.Ignition)) + if in.Ignition != nil { + in, out := &in.Ignition, &out.Ignition + *out = new(Ignition) + if err := Convert_v1beta2_Ignition_To_v1beta1_Ignition(*in, *out, s); err != nil { + return err + } + } else { + out.Ignition = nil + } out.SpotMarketOptions = (*SpotMarketOptions)(unsafe.Pointer(in.SpotMarketOptions)) // WARNING: in.PlacementGroupName requires manual conversion: does not exist in peer-type out.Tenancy = in.Tenancy @@ -1921,14 +1937,10 @@ func Convert_v1beta1_Ignition_To_v1beta2_Ignition(in *Ignition, out *v1beta2.Ign func autoConvert_v1beta2_Ignition_To_v1beta1_Ignition(in *v1beta2.Ignition, out *Ignition, s conversion.Scope) error { out.Version = in.Version + // WARNING: in.StorageType requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta2_Ignition_To_v1beta1_Ignition is an autogenerated conversion function. -func Convert_v1beta2_Ignition_To_v1beta1_Ignition(in *v1beta2.Ignition, out *Ignition, s conversion.Scope) error { - return autoConvert_v1beta2_Ignition_To_v1beta1_Ignition(in, out, s) -} - func autoConvert_v1beta1_IngressRule_To_v1beta2_IngressRule(in *IngressRule, out *v1beta2.IngressRule, s conversion.Scope) error { out.Description = in.Description out.Protocol = v1beta2.SecurityGroupProtocol(in.Protocol) diff --git a/api/v1beta2/awsmachine_types.go b/api/v1beta2/awsmachine_types.go index dad3bb0575..10d8ce0dcb 100644 --- a/api/v1beta2/awsmachine_types.go +++ b/api/v1beta2/awsmachine_types.go @@ -43,6 +43,17 @@ var ( SecretBackendSecretsManager = SecretBackend("secrets-manager") ) +// IgnitionStorageTypeOption defines the different storage types for Ignition. +type IgnitionStorageTypeOption string + +const ( + // IgnitionStorageTypeOptionClusterObjectStore means the chosen Ignition storage type is ClusterObjectStore. + IgnitionStorageTypeOptionClusterObjectStore = IgnitionStorageTypeOption("ClusterObjectStore") + + // IgnitionStorageTypeOptionUnencryptedUserData means the chosen Ignition storage type is UnencryptedUserData. + IgnitionStorageTypeOptionUnencryptedUserData = IgnitionStorageTypeOption("UnencryptedUserData") +) + // AWSMachineSpec defines the desired state of an Amazon EC2 instance. type AWSMachineSpec struct { // ProviderID is the unique identifier as specified by the cloud provider. @@ -206,6 +217,26 @@ type Ignition struct { // +kubebuilder:default="2.3" // +kubebuilder:validation:Enum="2.3";"3.0";"3.1";"3.2";"3.3";"3.4" Version string `json:"version,omitempty"` + + // StorageType defines how to store the boostrap user data for Ignition. + // This can be used to instruct Ignition from where to fetch the user data to bootstrap an instance. + // + // When omitted, the storage option will default to ClusterObjectStore. + // + // When set to "ClusterObjectStore", if the capability is available and a Cluster ObjectStore configuration + // is correctly provided in the Cluster object (under .spec.s3Bucket), + // an object store will be used to store bootstrap user data. + // + // When set to "UnencryptedUserData", EC2 Instance User Data will be used to store the machine bootstrap user data, unencrypted. + // This option is considered less secure than others as user data may contain sensitive informations (keys, certificates, etc.) + // and users with ec2:DescribeInstances permission or users running pods + // that can access the ec2 metadata service have access to this sensitive information. + // So this is only to be used at ones own risk, and only when other more secure options are not viable. + // + // +optional + // +kubebuilder:default="ClusterObjectStore" + // +kubebuilder:validation:Enum:="ClusterObjectStore";"UnencryptedUserData" + StorageType IgnitionStorageTypeOption `json:"storageType,omitempty"` } // AWSMachineStatus defines the observed state of AWSMachine. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml index 795f794a56..e356896c1b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml @@ -632,6 +632,28 @@ spec: description: Ignition defined options related to the bootstrapping systems where Ignition is used. properties: + storageType: + default: ClusterObjectStore + description: "StorageType defines how to store the boostrap user + data for Ignition. This can be used to instruct Ignition from + where to fetch the user data to bootstrap an instance. \n When + omitted, the storage option will default to ClusterObjectStore. + \n When set to \"ClusterObjectStore\", if the capability is + available and a Cluster ObjectStore configuration is correctly + provided in the Cluster object (under .spec.s3Bucket), an object + store will be used to store bootstrap user data. \n When set + to \"UnencryptedUserData\", EC2 Instance User Data will be used + to store the machine bootstrap user data, unencrypted. This + option is considered less secure than others as user data may + contain sensitive informations (keys, certificates, etc.) and + users with ec2:DescribeInstances permission or users running + pods that can access the ec2 metadata service have access to + this sensitive information. So this is only to be used at ones + own risk, and only when other more secure options are not viable." + enum: + - ClusterObjectStore + - UnencryptedUserData + type: string version: default: "2.3" description: Version defines which version of Ignition will be diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml index 23466b416c..00b85b4969 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml @@ -578,6 +578,30 @@ spec: description: Ignition defined options related to the bootstrapping systems where Ignition is used. properties: + storageType: + default: ClusterObjectStore + description: "StorageType defines how to store the boostrap + user data for Ignition. This can be used to instruct + Ignition from where to fetch the user data to bootstrap + an instance. \n When omitted, the storage option will + default to ClusterObjectStore. \n When set to \"ClusterObjectStore\", + if the capability is available and a Cluster ObjectStore + configuration is correctly provided in the Cluster object + (under .spec.s3Bucket), an object store will be used + to store bootstrap user data. \n When set to \"UnencryptedUserData\", + EC2 Instance User Data will be used to store the machine + bootstrap user data, unencrypted. This option is considered + less secure than others as user data may contain sensitive + informations (keys, certificates, etc.) and users with + ec2:DescribeInstances permission or users running pods + that can access the ec2 metadata service have access + to this sensitive information. So this is only to be + used at ones own risk, and only when other more secure + options are not viable." + enum: + - ClusterObjectStore + - UnencryptedUserData + type: string version: default: "2.3" description: Version defines which version of Ignition diff --git a/controllers/awsmachine_controller.go b/controllers/awsmachine_controller.go index 9c47adab1a..7ef74fe8c5 100644 --- a/controllers/awsmachine_controller.go +++ b/controllers/awsmachine_controller.go @@ -711,7 +711,21 @@ func (r *AWSMachineReconciler) resolveUserData(machineScope *scope.MachineScope, } if machineScope.UseIgnition(userDataFormat) { - userData, err = r.ignitionUserData(machineScope, objectStoreSvc, userData) + var ignitionStorageType infrav1.IgnitionStorageTypeOption + if machineScope.AWSMachine.Spec.Ignition == nil { + ignitionStorageType = infrav1.IgnitionStorageTypeOptionClusterObjectStore + } else { + ignitionStorageType = machineScope.AWSMachine.Spec.Ignition.StorageType + } + + switch ignitionStorageType { + case infrav1.IgnitionStorageTypeOptionClusterObjectStore: + userData, err = r.generateIgnitionWithRemoteStorage(machineScope, objectStoreSvc, userData) + case infrav1.IgnitionStorageTypeOptionUnencryptedUserData: + // No further modifications to userdata are needed for plain storage in UnencryptedUserData. + default: + return nil, "", errors.Errorf("unsupported ignition storageType %q", ignitionStorageType) + } } return userData, userDataFormat, err @@ -751,9 +765,12 @@ func (r *AWSMachineReconciler) cloudInitUserData(machineScope *scope.MachineScop return encryptedCloudInit, nil } -func (r *AWSMachineReconciler) ignitionUserData(scope *scope.MachineScope, objectStoreSvc services.ObjectStoreInterface, userData []byte) ([]byte, error) { +// generateIgnitionWithRemoteStorage uses a remote object storage (S3 bucket) and stores user data in it, +// then returns the config to instruct ignition on how to pull the user data from the bucket. +func (r *AWSMachineReconciler) generateIgnitionWithRemoteStorage(scope *scope.MachineScope, objectStoreSvc services.ObjectStoreInterface, userData []byte) ([]byte, error) { if objectStoreSvc == nil { - return nil, errors.New("object store service not available") + return nil, errors.New("using Ignition by default requires a cluster wide object storage configured at `AWSCluster.Spec.Ignition.S3Bucket`. " + + "You must configure one or instruct Ignition to use EC2 user data instead, by setting `AWSMachine.Spec.Ignition.StorageType` to `UnencryptedUserData`") } objectURL, err := objectStoreSvc.Create(scope, userData) @@ -852,7 +869,10 @@ func (r *AWSMachineReconciler) deleteIgnitionBootstrapDataFromS3(machineScope *s return err } - if !machineScope.UseIgnition(userDataFormat) { + // We only use an S3 bucket to store userdata if we use Ignition with StorageType ClusterObjectStore. + if !machineScope.UseIgnition(userDataFormat) || + (machineScope.AWSMachine.Spec.Ignition != nil && + machineScope.AWSMachine.Spec.Ignition.StorageType != infrav1.IgnitionStorageTypeOptionClusterObjectStore) { return nil } diff --git a/controllers/awsmachine_controller_unit_test.go b/controllers/awsmachine_controller_unit_test.go index 57af003cfe..dd444c5275 100644 --- a/controllers/awsmachine_controller_unit_test.go +++ b/controllers/awsmachine_controller_unit_test.go @@ -706,13 +706,14 @@ func TestAWSMachineReconciler(t *testing.T) { expectConditions(g, ms.AWSMachine, []conditionAssertion{{infrav1.ELBAttachedCondition, corev1.ConditionTrue, "", ""}}) expectConditions(g, ms.AWSMachine, []conditionAssertion{{infrav1.InstanceReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityWarning, infrav1.InstanceNotReadyReason}}) }) - t.Run("Should store userdata using AWS Secrets Manager", func(t *testing.T) { + t.Run("should store userdata for CloudInit using AWS Secrets Manager only when not skipped", func(t *testing.T) { g := NewWithT(t) awsMachine := getAWSMachine() setup(t, g, awsMachine) defer teardown(t, g) instanceCreate(t, g) + // Explicitly skip AWS Secrets Manager. ms.AWSMachine.Spec.CloudInit.InsecureSkipSecretsManager = true ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1) ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1) @@ -1120,7 +1121,7 @@ func TestAWSMachineReconciler(t *testing.T) { _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) }) - t.Run("should delete the secret from the S3 bucket", func(t *testing.T) { + t.Run("should delete the secret from the S3 bucket if StorageType ClusterObjectStore is set for Ignition", func(t *testing.T) { g := NewWithT(t) awsMachine := getAWSMachine() setup(t, g, awsMachine) @@ -1129,7 +1130,8 @@ func TestAWSMachineReconciler(t *testing.T) { ms.AWSMachine.Spec.CloudInit = infrav1.CloudInit{} ms.AWSMachine.Spec.Ignition = &infrav1.Ignition{ - Version: "2.3", + Version: "2.3", + StorageType: infrav1.IgnitionStorageTypeOptionClusterObjectStore, } buf := new(bytes.Buffer) @@ -1138,6 +1140,28 @@ func TestAWSMachineReconciler(t *testing.T) { objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + _, err := reconciler.reconcileDelete(ms, cs, cs, cs, cs) + g.Expect(err).To(BeNil()) + }) + t.Run("should not delete the secret from the S3 bucket if StorageType UnencryptedUserData is set for Ignition", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + setNodeRef(t, g) + + ms.AWSMachine.Spec.CloudInit = infrav1.CloudInit{} + ms.AWSMachine.Spec.Ignition = &infrav1.Ignition{ + Version: "2.3", + StorageType: infrav1.IgnitionStorageTypeOptionUnencryptedUserData, + } + + buf := new(bytes.Buffer) + klog.SetOutput(buf) + + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(0) + ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + _, err := reconciler.reconcileDelete(ms, cs, cs, cs, cs) g.Expect(err).To(BeNil()) }) @@ -1265,276 +1289,309 @@ func TestAWSMachineReconciler(t *testing.T) { }) }) - t.Run("Object storage lifecycle", func(t *testing.T) { - t.Run("creating EC2 instances", func(t *testing.T) { - var instance *infrav1.Instance - - getInstances := func(t *testing.T, g *WithT) { - t.Helper() - - ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() - } - - useIgnition := func(t *testing.T, g *WithT) { + t.Run("Object storage lifecycle for Ignition's userdata", func(t *testing.T) { + t.Run("when Ignition's StorageType is ClusterObjectStore", func(t *testing.T) { + useIgnitionWithClusterObjectStore := func(t *testing.T, g *WithT) { t.Helper() ms.Machine.Spec.Bootstrap.DataSecretName = ptr.To[string]("bootstrap-data-ignition") ms.AWSMachine.Spec.CloudInit.SecretCount = 0 ms.AWSMachine.Spec.CloudInit.SecretPrefix = "" + ms.AWSMachine.Spec.Ignition = &infrav1.Ignition{ + Version: "2.3", + StorageType: infrav1.IgnitionStorageTypeOptionClusterObjectStore, + } } - t.Run("should leverage AWS S3", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - getInstances(t, g) - useIgnition(t, g) + t.Run("creating EC2 instances", func(t *testing.T) { + var instance *infrav1.Instance - instance = &infrav1.Instance{ - ID: "myMachine", - State: infrav1.InstanceStatePending, + getInstances := func(t *testing.T, g *WithT) { + t.Helper() + + ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() } - fakeS3URL := "s3://foo" - objectStoreSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return(fakeS3URL, 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) + t.Run("should leverage a Cluster Object Store", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) + useIgnitionWithClusterObjectStore(t, g) - ms.AWSMachine.ObjectMeta.Labels = map[string]string{ - clusterv1.MachineControlPlaneLabel: "", - } + instance = &infrav1.Instance{ + ID: "myMachine", + State: infrav1.InstanceStatePending, + } + fakeS3URL := "s3://foo" - _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - g.Expect(err).To(BeNil()) - }) + objectStoreSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return(fakeS3URL, 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) - 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) + ms.AWSMachine.ObjectMeta.Labels = map[string]string{ + clusterv1.MachineControlPlaneLabel: "", + } - if cs.AWSCluster.Spec.S3Bucket == nil { - cs.AWSCluster.Spec.S3Bucket = &infrav1.S3Bucket{} - } - cs.AWSCluster.Spec.S3Bucket.PresignedURLDuration = &metav1.Duration{Duration: 1 * time.Hour} + _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + g.Expect(err).To(BeNil()) + }) - instance = &infrav1.Instance{ - ID: "myMachine", - State: infrav1.InstanceStatePending, - } + t.Run("should leverage a Cluster Object Store with presigned urls", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) + useIgnitionWithClusterObjectStore(t, g) - //nolint:gosec - 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" + if cs.AWSCluster.Spec.S3Bucket == nil { + cs.AWSCluster.Spec.S3Bucket = &infrav1.S3Bucket{} + } + cs.AWSCluster.Spec.S3Bucket.PresignedURLDuration = &metav1.Duration{Duration: 1 * time.Hour} - 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) + instance = &infrav1.Instance{ + ID: "myMachine", + State: infrav1.InstanceStatePending, + } - ms.AWSMachine.ObjectMeta.Labels = map[string]string{ - clusterv1.MachineControlPlaneLabel: "", - } + //nolint:gosec + 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" - _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - g.Expect(err).To(BeNil()) - }) - }) + 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) - t.Run("there's a node ref and a secret ARN", func(t *testing.T) { - var instance *infrav1.Instance - setNodeRef := func(t *testing.T, g *WithT) { - t.Helper() + ms.AWSMachine.ObjectMeta.Labels = map[string]string{ + clusterv1.MachineControlPlaneLabel: "", + } - instance = &infrav1.Instance{ - ID: "myMachine", - } + _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + g.Expect(err).To(BeNil()) + }) + }) - ms.Machine.Status.NodeRef = &corev1.ObjectReference{ - Kind: "Node", - Name: "myMachine", - APIVersion: "v1", - } + t.Run("there's a node ref and a secret ARN", func(t *testing.T) { + var instance *infrav1.Instance + setNodeRef := func(t *testing.T, g *WithT) { + t.Helper() - ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(instance, nil).AnyTimes() - } - useIgnition := func(t *testing.T, g *WithT) { - t.Helper() + instance = &infrav1.Instance{ + ID: "myMachine", + } - ms.Machine.Spec.Bootstrap.DataSecretName = ptr.To[string]("bootstrap-data-ignition") - ms.AWSMachine.Spec.CloudInit.SecretCount = 0 - ms.AWSMachine.Spec.CloudInit.SecretPrefix = "" - } + ms.Machine.Status.NodeRef = &corev1.ObjectReference{ + Kind: "Node", + Name: "myMachine", + APIVersion: "v1", + } - t.Run("should delete the object if the instance is running", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - setNodeRef(t, g) - useIgnition(t, g) + ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(instance, nil).AnyTimes() + } - instance.State = infrav1.InstanceStateRunning - ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1) - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1) - ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil) + t.Run("should delete the object if the instance is running", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + setNodeRef(t, g) + useIgnitionWithClusterObjectStore(t, g) - _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - }) + instance.State = infrav1.InstanceStateRunning + ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1) + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1) + ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil) - t.Run("should delete the object if the instance is terminated", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - setNodeRef(t, g) - useIgnition(t, g) + _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + }) - instance.State = infrav1.InstanceStateTerminated - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + t.Run("should delete the object if the instance is terminated", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + setNodeRef(t, g) + useIgnitionWithClusterObjectStore(t, g) - _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - }) + instance.State = infrav1.InstanceStateTerminated + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - t.Run("should delete the object if the instance is deleted", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - setNodeRef(t, g) - useIgnition(t, g) + _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + }) - instance.State = infrav1.InstanceStateRunning - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + t.Run("should delete the object if the instance is deleted", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + setNodeRef(t, g) + useIgnitionWithClusterObjectStore(t, g) - _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) - }) + instance.State = infrav1.InstanceStateRunning + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() - t.Run("should delete the object if the AWSMachine is in a failure condition", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - setNodeRef(t, g) - useIgnition(t, g) + _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + }) - // TODO: This seems to have no effect on the test result. - ms.AWSMachine.Status.FailureReason = capierrors.MachineStatusErrorPtr(capierrors.UpdateMachineError) + t.Run("should delete the object if the AWSMachine is in a failure condition", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + setNodeRef(t, g) + useIgnitionWithClusterObjectStore(t, g) - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + // TODO: This seems to have no effect on the test result. + ms.AWSMachine.Status.FailureReason = capierrors.MachineStatusErrorPtr(capierrors.UpdateMachineError) - _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + + _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + }) }) - }) - t.Run("there's only a secret ARN and no node ref", func(t *testing.T) { - var instance *infrav1.Instance + t.Run("there's only a secret ARN and no node ref", func(t *testing.T) { + var instance *infrav1.Instance - getInstances := func(t *testing.T, g *WithT) { - t.Helper() + getInstances := func(t *testing.T, g *WithT) { + t.Helper() - instance = &infrav1.Instance{ - ID: "myMachine", + instance = &infrav1.Instance{ + ID: "myMachine", + } + ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(instance, nil).AnyTimes() } - ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(instance, nil).AnyTimes() - } - useIgnition := func(t *testing.T, g *WithT) { - t.Helper() - - ms.Machine.Spec.Bootstrap.DataSecretName = ptr.To[string]("bootstrap-data-ignition") - ms.AWSMachine.Spec.CloudInit.SecretCount = 0 - ms.AWSMachine.Spec.CloudInit.SecretPrefix = "" - } + t.Run("should not delete the object if the instance is running", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) - t.Run("should not delete the object if the instance is running", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - getInstances(t, g) + instance.State = infrav1.InstanceStateRunning + 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) + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).MaxTimes(0) + _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + }) - instance.State = infrav1.InstanceStateRunning - 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) - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).MaxTimes(0) - _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - }) + t.Run("should delete the object if the instance is terminated", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) + useIgnitionWithClusterObjectStore(t, g) - t.Run("should delete the object if the instance is terminated", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - getInstances(t, g) - useIgnition(t, g) + instance.State = infrav1.InstanceStateTerminated + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + }) - instance.State = infrav1.InstanceStateTerminated - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - _, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - }) + t.Run("should delete the object if the AWSMachine is deleted", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) + useIgnitionWithClusterObjectStore(t, g) - t.Run("should delete the object if the AWSMachine is deleted", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - getInstances(t, g) - useIgnition(t, g) + instance.State = infrav1.InstanceStateRunning + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + }) - instance.State = infrav1.InstanceStateRunning - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() - _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + t.Run("should delete the object if the AWSMachine is in a failure condition", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) + useIgnitionWithClusterObjectStore(t, g) + + // TODO: This seems to have no effect on the test result. + ms.AWSMachine.Status.FailureReason = capierrors.MachineStatusErrorPtr(capierrors.UpdateMachineError) + objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) + ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() + _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + }) }) - t.Run("should delete the object if the AWSMachine is in a failure condition", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - getInstances(t, g) - useIgnition(t, g) + t.Run("there is an intermittent connection issue and no object could be created", func(t *testing.T) { + t.Run("should error if object could not be created", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + useIgnitionWithClusterObjectStore(t, g) - // TODO: This seems to have no effect on the test result. - ms.AWSMachine.Status.FailureReason = capierrors.MachineStatusErrorPtr(capierrors.UpdateMachineError) - objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - ec2Svc.EXPECT().TerminateInstance(gomock.Any()).Return(nil).AnyTimes() - _, _ = reconciler.reconcileDelete(ms, cs, cs, cs, cs) + ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() + objectStoreSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return("", errors.New("connection error")).Times(1) + _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) + g.Expect(err).ToNot(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("connection error")) + }) }) }) - t.Run("there is an intermittent connection issue and no object could be created", func(t *testing.T) { - useIgnition := func(t *testing.T, g *WithT) { + t.Run("when Ignition's StorageType is UnencryptedUserData", func(t *testing.T) { + useIgnitionAndUnencryptedUserData := func(t *testing.T, g *WithT) { t.Helper() ms.Machine.Spec.Bootstrap.DataSecretName = ptr.To[string]("bootstrap-data-ignition") ms.AWSMachine.Spec.CloudInit.SecretCount = 0 ms.AWSMachine.Spec.CloudInit.SecretPrefix = "" + ms.AWSMachine.Spec.Ignition = &infrav1.Ignition{ + Version: "2.3", + StorageType: infrav1.IgnitionStorageTypeOptionUnencryptedUserData, + } } + t.Run("creating EC2 instances", func(t *testing.T) { + var instance *infrav1.Instance - t.Run("should error if object could not be created", func(t *testing.T) { - g := NewWithT(t) - awsMachine := getAWSMachine() - setup(t, g, awsMachine) - defer teardown(t, g) - useIgnition(t, g) + getInstances := func(t *testing.T, g *WithT) { + t.Helper() + ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() + } + t.Run("should NOT leverage a Cluster Object Store", func(t *testing.T) { + g := NewWithT(t) + awsMachine := getAWSMachine() + setup(t, g, awsMachine) + defer teardown(t, g) + getInstances(t, g) + useIgnitionAndUnencryptedUserData(t, g) - ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() - objectStoreSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return("", errors.New("connection error")).Times(1) - _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs) - g.Expect(err).ToNot(BeNil()) - g.Expect(err.Error()).To(ContainSubstring("connection error")) + instance = &infrav1.Instance{ + ID: "myMachine", + State: infrav1.InstanceStatePending, + } + fakeS3URL := "s3://foo" + + // Expect no Cluster Object Store to be created. + objectStoreSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return(fakeS3URL, nil).Times(0) + + 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()) + }) }) }) }) diff --git a/docs/book/src/topics/ignition-support.md b/docs/book/src/topics/ignition-support.md index fe2c5c90db..0d19e20bdf 100644 --- a/docs/book/src/topics/ignition-support.md +++ b/docs/book/src/topics/ignition-support.md @@ -12,8 +12,8 @@ the underlying OS for workload clusters.