From 4df5b91e75c4e0a52d6756130f2cdae58a014b9d Mon Sep 17 00:00:00 2001 From: Mulham Raee Date: Tue, 5 Mar 2024 18:06:43 +0100 Subject: [PATCH] add additionalSecurityGroups field to ROSAmachinePool --- ...ure.cluster.x-k8s.io_rosamachinepools.yaml | 6 +++ exp/api/v1beta2/rosamachinepool_types.go | 7 ++++ exp/api/v1beta2/rosamachinepool_webhook.go | 25 +++++++++++- exp/api/v1beta2/zz_generated.deepcopy.go | 5 +++ exp/controllers/rosamachinepool_controller.go | 39 ++++++++++++------- 5 files changed, 67 insertions(+), 15 deletions(-) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml index 0cc4441e88..e4a9c9fe14 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml @@ -47,6 +47,12 @@ spec: spec: description: RosaMachinePoolSpec defines the desired state of RosaMachinePool. properties: + additionalSecurityGroups: + description: AdditionalSecurityGroups is an optional set of security + groups to associate with all node instances of the machine pool. + items: + type: string + type: array autoRepair: default: false description: AutoRepair specifies whether health checks should be diff --git a/exp/api/v1beta2/rosamachinepool_types.go b/exp/api/v1beta2/rosamachinepool_types.go index 8335eaacec..67a9769950 100644 --- a/exp/api/v1beta2/rosamachinepool_types.go +++ b/exp/api/v1beta2/rosamachinepool_types.go @@ -79,6 +79,13 @@ type RosaMachinePoolSpec struct { // +optional TuningConfigs []string `json:"tuningConfigs,omitempty"` + // AdditionalSecurityGroups is an optional set of security groups to associate + // with all node instances of the machine pool. + // + // +immutable + // +optional + AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"` + // ProviderIDList contain a ProviderID for each machine instance that's currently managed by this machine pool. // +optional ProviderIDList []string `json:"providerIDList,omitempty"` diff --git a/exp/api/v1beta2/rosamachinepool_webhook.go b/exp/api/v1beta2/rosamachinepool_webhook.go index b0660c031f..6335d20a47 100644 --- a/exp/api/v1beta2/rosamachinepool_webhook.go +++ b/exp/api/v1beta2/rosamachinepool_webhook.go @@ -2,6 +2,8 @@ package v1beta2 import ( "github.com/blang/semver" + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" runtime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -44,12 +46,20 @@ func (r *ROSAMachinePool) ValidateCreate() (warnings admission.Warnings, err err // ValidateUpdate implements admission.Validator. func (r *ROSAMachinePool) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) { - var allErrs field.ErrorList + oldPool, ok := old.(*ROSAMachinePool) + if !ok { + return nil, apierrors.NewInvalid(GroupVersion.WithKind("ROSAMachinePool").GroupKind(), r.Name, field.ErrorList{ + field.InternalError(nil, errors.New("failed to convert old ROSAMachinePool to object")), + }) + } + var allErrs field.ErrorList if err := r.validateVersion(); err != nil { allErrs = append(allErrs, err) } + allErrs = append(allErrs, validateImmutable(oldPool.Spec.AdditionalSecurityGroups, r.Spec.AdditionalSecurityGroups, "additionalSecurityGroups")...) + if len(allErrs) == 0 { return nil, nil } @@ -78,6 +88,19 @@ func (r *ROSAMachinePool) validateVersion() *field.Error { return nil } +func validateImmutable(old, updated interface{}, name string) field.ErrorList { + var allErrs field.ErrorList + + if !cmp.Equal(old, updated) { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", name), updated, "field is immutable"), + ) + } + + return allErrs +} + // Default implements admission.Defaulter. func (r *ROSAMachinePool) Default() { } diff --git a/exp/api/v1beta2/zz_generated.deepcopy.go b/exp/api/v1beta2/zz_generated.deepcopy.go index 9a4a96b7ff..45f6a806ee 100644 --- a/exp/api/v1beta2/zz_generated.deepcopy.go +++ b/exp/api/v1beta2/zz_generated.deepcopy.go @@ -1105,6 +1105,11 @@ func (in *RosaMachinePoolSpec) DeepCopyInto(out *RosaMachinePoolSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.AdditionalSecurityGroups != nil { + in, out := &in.AdditionalSecurityGroups, &out.AdditionalSecurityGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.ProviderIDList != nil { in, out := &in.ProviderIDList, &out.ProviderIDList *out = make([]string, len(*in)) diff --git a/exp/controllers/rosamachinepool_controller.go b/exp/controllers/rosamachinepool_controller.go index 2dae9b6f9e..88bf270bdd 100644 --- a/exp/controllers/rosamachinepool_controller.go +++ b/exp/controllers/rosamachinepool_controller.go @@ -352,16 +352,18 @@ func (r *ROSAMachinePoolReconciler) updateNodePool(machinePoolScope *scope.RosaM currentSpec := nodePoolToRosaMachinePoolSpec(nodePool) currentSpec.ProviderIDList = desiredSpec.ProviderIDList // providerIDList is set by the controller and shouldn't be compared here. - currentSpec.Version = desiredSpec.Version // Version changed are reconciled separately and shouldn't be compared here. + currentSpec.Version = desiredSpec.Version // Version changes are reconciled separately and shouldn't be compared here. if cmp.Equal(desiredSpec, currentSpec) { // no changes detected. return nodePool, nil } - npBuilder := nodePoolBuilder(*desiredSpec, machinePoolScope.MachinePool.Spec) - npBuilder.Version(nil) // eunsure version is unset. + // zero-out fields that shouldn't be part of the update call. + desiredSpec.Version = "" + desiredSpec.AdditionalSecurityGroups = nil + npBuilder := nodePoolBuilder(*desiredSpec, machinePoolScope.MachinePool.Spec) nodePoolSpec, err := npBuilder.Build() if err != nil { return nil, fmt.Errorf("failed to build nodePool spec: %w", err) @@ -401,8 +403,11 @@ func validateMachinePoolSpec(machinePoolScope *scope.RosaMachinePoolScope) (*str func nodePoolBuilder(rosaMachinePoolSpec expinfrav1.RosaMachinePoolSpec, machinePoolSpec expclusterv1.MachinePoolSpec) *cmv1.NodePoolBuilder { npBuilder := cmv1.NewNodePool().ID(rosaMachinePoolSpec.NodePoolName). Labels(rosaMachinePoolSpec.Labels). - AutoRepair(rosaMachinePoolSpec.AutoRepair). - TuningConfigs(rosaMachinePoolSpec.TuningConfigs...) + AutoRepair(rosaMachinePoolSpec.AutoRepair) + + if rosaMachinePoolSpec.TuningConfigs != nil { + npBuilder = npBuilder.TuningConfigs(rosaMachinePoolSpec.TuningConfigs...) + } if len(rosaMachinePoolSpec.Taints) > 0 { taintBuilders := []*cmv1.TaintBuilder{} @@ -430,7 +435,12 @@ func nodePoolBuilder(rosaMachinePoolSpec expinfrav1.RosaMachinePoolSpec, machine npBuilder.Subnet(rosaMachinePoolSpec.Subnet) } - npBuilder.AWSNodePool(cmv1.NewAWSNodePool().InstanceType(rosaMachinePoolSpec.InstanceType)) + awsNodePool := cmv1.NewAWSNodePool().InstanceType(rosaMachinePoolSpec.InstanceType) + if rosaMachinePoolSpec.AdditionalSecurityGroups != nil { + awsNodePool = awsNodePool.AdditionalSecurityGroupIds(rosaMachinePoolSpec.AdditionalSecurityGroups...) + } + npBuilder.AWSNodePool(awsNodePool) + if rosaMachinePoolSpec.Version != "" { npBuilder.Version(cmv1.NewVersion().ID(ocm.CreateVersionID(rosaMachinePoolSpec.Version, ocm.DefaultChannelGroup))) } @@ -440,14 +450,15 @@ func nodePoolBuilder(rosaMachinePoolSpec expinfrav1.RosaMachinePoolSpec, machine func nodePoolToRosaMachinePoolSpec(nodePool *cmv1.NodePool) expinfrav1.RosaMachinePoolSpec { spec := expinfrav1.RosaMachinePoolSpec{ - NodePoolName: nodePool.ID(), - Version: rosa.RawVersionID(nodePool.Version()), - AvailabilityZone: nodePool.AvailabilityZone(), - Subnet: nodePool.Subnet(), - Labels: nodePool.Labels(), - AutoRepair: nodePool.AutoRepair(), - InstanceType: nodePool.AWSNodePool().InstanceType(), - TuningConfigs: nodePool.TuningConfigs(), + NodePoolName: nodePool.ID(), + Version: rosa.RawVersionID(nodePool.Version()), + AvailabilityZone: nodePool.AvailabilityZone(), + Subnet: nodePool.Subnet(), + Labels: nodePool.Labels(), + AutoRepair: nodePool.AutoRepair(), + InstanceType: nodePool.AWSNodePool().InstanceType(), + TuningConfigs: nodePool.TuningConfigs(), + AdditionalSecurityGroups: nodePool.AWSNodePool().AdditionalSecurityGroupIds(), } if nodePool.Autoscaling() != nil {