diff --git a/api/v1beta2/awsmachine_webhook.go b/api/v1beta2/awsmachine_webhook.go index 9c271c6939..36d6c28ed0 100644 --- a/api/v1beta2/awsmachine_webhook.go +++ b/api/v1beta2/awsmachine_webhook.go @@ -460,6 +460,11 @@ func (*awsMachineWebhook) Default(_ context.Context, obj runtime.Object) error { r.Spec.Ignition.Version = DefaultIgnitionVersion } + if r.Spec.InstanceMetadataOptions == nil { + r.Spec.InstanceMetadataOptions = &InstanceMetadataOptions{} + } + r.Spec.InstanceMetadataOptions.SetDefaults() + return nil } diff --git a/api/v1beta2/awsmachine_webhook_test.go b/api/v1beta2/awsmachine_webhook_test.go index d233cde8d5..122dfd88ee 100644 --- a/api/v1beta2/awsmachine_webhook_test.go +++ b/api/v1beta2/awsmachine_webhook_test.go @@ -38,6 +38,11 @@ func TestMachineDefault(t *testing.T) { err := (&awsMachineWebhook{}).Default(context.Background(), machine) g.Expect(err).NotTo(HaveOccurred()) g.Expect(machine.Spec.CloudInit.SecureSecretsBackend).To(Equal(SecretBackendSecretsManager)) + g.Expect(machine.Spec.InstanceMetadataOptions).NotTo(BeNil()) + g.Expect(machine.Spec.InstanceMetadataOptions.HTTPEndpoint).To(Equal(InstanceMetadataEndpointStateEnabled)) + g.Expect(machine.Spec.InstanceMetadataOptions.HTTPPutResponseHopLimit).To(Equal(int64(1))) + g.Expect(machine.Spec.InstanceMetadataOptions.HTTPTokens).To(Equal(HTTPTokensStateOptional)) + g.Expect(machine.Spec.InstanceMetadataOptions.InstanceMetadataTags).To(Equal(InstanceMetadataEndpointStateDisabled)) } func TestAWSMachineCreate(t *testing.T) { diff --git a/controllers/awsmachine_controller.go b/controllers/awsmachine_controller.go index ab76af1cf9..b1a3c3d713 100644 --- a/controllers/awsmachine_controller.go +++ b/controllers/awsmachine_controller.go @@ -197,8 +197,6 @@ func (r *AWSMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } - infrav1.SetDefaults_AWSMachineSpec(&awsMachine.Spec) - if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, awsMachine); err != nil || isPaused || conditionChanged { return ctrl.Result{}, err } @@ -723,12 +721,6 @@ func (r *AWSMachineReconciler) reconcileOperationalState(ec2svc services.EC2Inte } v1beta1conditions.MarkTrue(machineScope.AWSMachine, infrav1.SecurityGroupsReadyCondition) - err = r.ensureInstanceMetadataOptions(ec2svc, instance, machineScope.AWSMachine) - if err != nil { - machineScope.Error(err, "failed to ensure instance metadata options") - return err - } - return nil } @@ -1322,11 +1314,3 @@ func (r *AWSMachineReconciler) ensureStorageTags(ec2svc services.EC2Interface, i } } } - -func (r *AWSMachineReconciler) ensureInstanceMetadataOptions(ec2svc services.EC2Interface, instance *infrav1.Instance, machine *infrav1.AWSMachine) error { - if cmp.Equal(machine.Spec.InstanceMetadataOptions, instance.InstanceMetadataOptions) { - return nil - } - - return ec2svc.ModifyInstanceMetadataOptions(instance.ID, machine.Spec.InstanceMetadataOptions) -} diff --git a/controllers/awsmachine_controller_test.go b/controllers/awsmachine_controller_test.go index ad65f4a5cb..3cc28536ca 100644 --- a/controllers/awsmachine_controller_test.go +++ b/controllers/awsmachine_controller_test.go @@ -585,6 +585,12 @@ func getAWSMachine() *infrav1.AWSMachine { }, InstanceType: "test", Subnet: &infrav1.AWSResourceReference{ID: aws.String("subnet-1")}, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, }, } } diff --git a/controllers/awsmachine_controller_unit_test.go b/controllers/awsmachine_controller_unit_test.go index 072c2a5e65..ad7dd79636 100644 --- a/controllers/awsmachine_controller_unit_test.go +++ b/controllers/awsmachine_controller_unit_test.go @@ -88,6 +88,12 @@ func TestAWSMachineReconciler(t *testing.T) { } klog.SetOutput(GinkgoWriter) + // Ensure InstanceMetadataOptions defaults are set (webhook sets these normally, but not in unit tests) + if awsMachine.Spec.InstanceMetadataOptions == nil { + awsMachine.Spec.InstanceMetadataOptions = &infrav1.InstanceMetadataOptions{} + awsMachine.Spec.InstanceMetadataOptions.SetDefaults() + } + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "bootstrap-data", @@ -361,6 +367,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", VolumeIDs: []string{"volume-1", "volume-2"}, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } instance.State = infrav1.InstanceStatePending @@ -766,6 +778,12 @@ func TestAWSMachineReconciler(t *testing.T) { ID: "myMachine", VolumeIDs: []string{"volume-1", "volume-2"}, AvailabilityZone: "us-east-1", + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } instance.State = infrav1.InstanceStatePending } @@ -1022,6 +1040,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", State: infrav1.InstanceStatePending, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() @@ -1059,6 +1083,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", State: infrav1.InstanceStatePending, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(nil, nil).AnyTimes() @@ -1083,6 +1113,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } ms.Machine.Status.NodeRef = clusterv1.MachineNodeReference{ @@ -1217,6 +1253,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } ms.AWSMachine.Spec.CloudInit = infrav1.CloudInit{ @@ -1314,6 +1356,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } instance.State = infrav1.InstanceStatePending secretSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return(secretPrefix, int32(1), nil).Times(1) @@ -1366,6 +1414,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", State: infrav1.InstanceStatePending, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } fakeS3URL := "s3://foo" @@ -1399,6 +1453,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", State: infrav1.InstanceStatePending, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } //nolint:gosec @@ -1426,6 +1486,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } ms.Machine.Status.NodeRef = clusterv1.MachineNodeReference{ @@ -1507,6 +1573,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } ec2Svc.EXPECT().GetRunningInstanceByTags(gomock.Any()).Return(instance, nil).AnyTimes() } @@ -1616,6 +1688,12 @@ func TestAWSMachineReconciler(t *testing.T) { instance = &infrav1.Instance{ ID: "myMachine", State: infrav1.InstanceStatePending, + InstanceMetadataOptions: &infrav1.InstanceMetadataOptions{ + HTTPEndpoint: infrav1.InstanceMetadataEndpointStateEnabled, + HTTPPutResponseHopLimit: 1, + HTTPTokens: infrav1.HTTPTokensStateOptional, + InstanceMetadataTags: infrav1.InstanceMetadataEndpointStateDisabled, + }, } fakeS3URL := "s3://foo" diff --git a/pkg/cloud/services/ec2/instances.go b/pkg/cloud/services/ec2/instances.go index 95ad690860..d904d441c4 100644 --- a/pkg/cloud/services/ec2/instances.go +++ b/pkg/cloud/services/ec2/instances.go @@ -1122,24 +1122,6 @@ func (s *Service) checkRootVolume(rootVolume *infrav1.Volume, imageID string) (* return rootDeviceName, nil } -// ModifyInstanceMetadataOptions modifies the metadata options of the given EC2 instance. -func (s *Service) ModifyInstanceMetadataOptions(instanceID string, options *infrav1.InstanceMetadataOptions) error { - input := &ec2.ModifyInstanceMetadataOptionsInput{ - HttpEndpoint: types.InstanceMetadataEndpointState(string(options.HTTPEndpoint)), - HttpPutResponseHopLimit: utils.ToInt32Pointer(&options.HTTPPutResponseHopLimit), - HttpTokens: types.HttpTokensState(string(options.HTTPTokens)), - InstanceMetadataTags: types.InstanceMetadataTagsState(string(options.InstanceMetadataTags)), - InstanceId: aws.String(instanceID), - } - - s.scope.Info("Updating instance metadata options", "instance id", instanceID, "options", input) - if _, err := s.EC2Client.ModifyInstanceMetadataOptions(context.TODO(), input); err != nil { - return err - } - - return nil -} - // GetDHCPOptionSetDomainName returns the domain DNS name for the VPC from the DHCP Options. func (s *Service) GetDHCPOptionSetDomainName(ec2client common.EC2API, vpcID *string) *string { log := s.scope.GetLogger() diff --git a/pkg/cloud/services/interfaces.go b/pkg/cloud/services/interfaces.go index 5b4eccb30e..e3ceec2839 100644 --- a/pkg/cloud/services/interfaces.go +++ b/pkg/cloud/services/interfaces.go @@ -73,7 +73,6 @@ type EC2Interface interface { GetInstanceSecurityGroups(instanceID string) (map[string][]string, error) UpdateInstanceSecurityGroups(id string, securityGroups []string) error UpdateResourceTags(resourceID *string, create, remove map[string]string) error - ModifyInstanceMetadataOptions(instanceID string, options *infrav1.InstanceMetadataOptions) error TerminateInstanceAndWait(instanceID string) error DetachSecurityGroupsFromNetworkInterface(groups []string, interfaceID string) error diff --git a/pkg/cloud/services/mock_services/ec2_interface_mock.go b/pkg/cloud/services/mock_services/ec2_interface_mock.go index 6b923ac502..ea1efbf9aa 100644 --- a/pkg/cloud/services/mock_services/ec2_interface_mock.go +++ b/pkg/cloud/services/mock_services/ec2_interface_mock.go @@ -324,20 +324,6 @@ func (mr *MockEC2InterfaceMockRecorder) LaunchTemplateNeedsUpdate(arg0, arg1, ar return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LaunchTemplateNeedsUpdate", reflect.TypeOf((*MockEC2Interface)(nil).LaunchTemplateNeedsUpdate), arg0, arg1, arg2) } -// ModifyInstanceMetadataOptions mocks base method. -func (m *MockEC2Interface) ModifyInstanceMetadataOptions(arg0 string, arg1 *v1beta2.InstanceMetadataOptions) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ModifyInstanceMetadataOptions", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 -} - -// ModifyInstanceMetadataOptions indicates an expected call of ModifyInstanceMetadataOptions. -func (mr *MockEC2InterfaceMockRecorder) ModifyInstanceMetadataOptions(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ModifyInstanceMetadataOptions", reflect.TypeOf((*MockEC2Interface)(nil).ModifyInstanceMetadataOptions), arg0, arg1) -} - // PruneLaunchTemplateVersions mocks base method. func (m *MockEC2Interface) PruneLaunchTemplateVersions(arg0 string) (*types.LaunchTemplateVersion, error) { m.ctrl.T.Helper()