From d28658444dd2d5892a97473f311118f865ce3b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20BUISSON?= Date: Thu, 25 Jul 2024 09:48:52 +0200 Subject: [PATCH] fix: validate values --- .../services/container/clusters/reconcile.go | 12 ++--- ...ster.x-k8s.io_gcpmanagedcontrolplanes.yaml | 4 +- .../v1beta1/gcpmanagedcontrolplane_types.go | 48 +++++++++++++++++-- .../v1beta1/gcpmanagedcontrolplane_webhook.go | 17 ++++++- exp/api/v1beta1/zz_generated.deepcopy.go | 4 +- 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/cloud/services/container/clusters/reconcile.go b/cloud/services/container/clusters/reconcile.go index 29b8b6f7e..ba314371a 100644 --- a/cloud/services/container/clusters/reconcile.go +++ b/cloud/services/container/clusters/reconcile.go @@ -297,10 +297,10 @@ func (s *Service) createCluster(ctx context.Context, log *logr.Logger) error { if !s.scope.IsAutopilotCluster() { cluster.NodePools = scope.ConvertToSdkNodePools(nodePools, machinePools, isRegional, cluster.GetName()) if s.scope.GCPManagedControlPlane.Spec.LoggingService != nil { - cluster.LoggingService = *s.scope.GCPManagedControlPlane.Spec.LoggingService + cluster.LoggingService = s.scope.GCPManagedControlPlane.Spec.LoggingService.String() } if s.scope.GCPManagedControlPlane.Spec.MonitoringService != nil { - cluster.MonitoringService = *s.scope.GCPManagedControlPlane.Spec.MonitoringService + cluster.MonitoringService = s.scope.GCPManagedControlPlane.Spec.MonitoringService.String() } } @@ -437,16 +437,16 @@ func (s *Service) checkDiffAndPrepareUpdate(existingCluster *containerpb.Cluster } // LoggingService - if existingCluster.GetLoggingService() != *s.scope.GCPManagedControlPlane.Spec.LoggingService { + if existingCluster.GetLoggingService() != s.scope.GCPManagedControlPlane.Spec.LoggingService.String() { needUpdate = true - clusterUpdate.DesiredLoggingService = *s.scope.GCPManagedControlPlane.Spec.LoggingService + clusterUpdate.DesiredLoggingService = s.scope.GCPManagedControlPlane.Spec.LoggingService.String() log.V(2).Info("LoggingService config update required", "current", existingCluster.GetLoggingService(), "desired", *s.scope.GCPManagedControlPlane.Spec.LoggingService) } // MonitoringService - if existingCluster.GetMonitoringService() != *s.scope.GCPManagedControlPlane.Spec.MonitoringService { + if existingCluster.GetMonitoringService() != s.scope.GCPManagedControlPlane.Spec.MonitoringService.String() { needUpdate = true - clusterUpdate.DesiredLoggingService = *s.scope.GCPManagedControlPlane.Spec.MonitoringService + clusterUpdate.DesiredLoggingService = s.scope.GCPManagedControlPlane.Spec.MonitoringService.String() log.V(2).Info("MonitoringService config update required", "current", existingCluster.GetMonitoringService(), "desired", *s.scope.GCPManagedControlPlane.Spec.MonitoringService) } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml index e401d6d69..01b052e98 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml @@ -164,7 +164,7 @@ spec: loggingService: description: |- LoggingService represents configuration of logging service feature of the GKE cluster. - Possible values: none, logging.googleapis.com/kubernetes (default for GKE 1.14+), logging.googleapis.com (default for GKE < 1.14). + Possible values: none, logging.googleapis.com/kubernetes (default). Value is ignored when enableAutopilot = true. type: string master_authorized_networks_config: @@ -198,7 +198,7 @@ spec: monitoringService: description: |- MonitoringService represents configuration of monitoring service feature of the GKE cluster. - Possible values: none, monitoring.googleapis.com/kubernetes (default for GKE 1.14+), monitoring.googleapis.com (default for GKE < 1.14). + Possible values: none, monitoring.googleapis.com/kubernetes (default). Value is ignored when enableAutopilot = true. type: string project: diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_types.go b/exp/api/v1beta1/gcpmanagedcontrolplane_types.go index ac9da7fde..ab9591f1b 100644 --- a/exp/api/v1beta1/gcpmanagedcontrolplane_types.go +++ b/exp/api/v1beta1/gcpmanagedcontrolplane_types.go @@ -17,7 +17,11 @@ limitations under the License. package v1beta1 import ( + "fmt" + "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/strings/slices" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -149,15 +153,15 @@ type GCPManagedControlPlaneSpec struct { // +optional MasterAuthorizedNetworksConfig *MasterAuthorizedNetworksConfig `json:"master_authorized_networks_config,omitempty"` // LoggingService represents configuration of logging service feature of the GKE cluster. - // Possible values: none, logging.googleapis.com/kubernetes (default for GKE 1.14+), logging.googleapis.com (default for GKE < 1.14). + // Possible values: none, logging.googleapis.com/kubernetes (default). // Value is ignored when enableAutopilot = true. // +optional - LoggingService *string `json:"loggingService,omitempty"` + LoggingService *LoggingService `json:"loggingService,omitempty"` // MonitoringService represents configuration of monitoring service feature of the GKE cluster. - // Possible values: none, monitoring.googleapis.com/kubernetes (default for GKE 1.14+), monitoring.googleapis.com (default for GKE < 1.14). + // Possible values: none, monitoring.googleapis.com/kubernetes (default). // Value is ignored when enableAutopilot = true. // +optional - MonitoringService *string `json:"monitoringService,omitempty"` + MonitoringService *MonitoringService `json:"monitoringService,omitempty"` } // GCPManagedControlPlaneStatus defines the observed state of GCPManagedControlPlane. @@ -243,6 +247,42 @@ type MasterAuthorizedNetworksConfigCidrBlock struct { CidrBlock string `json:"cidr_block,omitempty"` } +// LoggingService is GKE logging service configuration. +type LoggingService string + +// Validate validates LoggingService value. +func (l LoggingService) Validate() error { + validValues := []string{"none", "logging.googleapis.com/kubernetes"} + if !slices.Contains(validValues, l.String()) { + return fmt.Errorf("invalid value; expect one of : %s", strings.Join(validValues, ",")) + } + + return nil +} + +// String returns a string from LoggingService. +func (l LoggingService) String() string { + return string(l) +} + +// MonitoringService is GKE logging service configuration. +type MonitoringService string + +// Validate validates MonitoringService value. +func (m MonitoringService) Validate() error { + validValues := []string{"none", "monitoring.googleapis.com/kubernetes"} + if !slices.Contains(validValues, m.String()) { + return fmt.Errorf("invalid value; expect one of : %s", strings.Join(validValues, ",")) + } + + return nil +} + +// String returns a string from MonitoringService. +func (m MonitoringService) String() string { + return string(m) +} + // GetConditions returns the control planes conditions. func (r *GCPManagedControlPlane) GetConditions() clusterv1.Conditions { return r.Status.Conditions diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go b/exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go index 1dd09fc99..7835b4143 100644 --- a/exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go +++ b/exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/google/go-cmp/cmp" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/validation/field" @@ -150,6 +149,22 @@ func (r *GCPManagedControlPlane) ValidateUpdate(oldRaw runtime.Object) (admissio r.Spec.LoggingService, "can't be set when autopilot is enabled")) } + if r.Spec.LoggingService != nil { + err := r.Spec.LoggingService.Validate() + if err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "LoggingService"), + r.Spec.LoggingService, err.Error())) + } + } + + if r.Spec.MonitoringService != nil { + err := r.Spec.MonitoringService.Validate() + if err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "MonitoringService"), + r.Spec.MonitoringService, err.Error())) + } + } + if len(allErrs) == 0 { return nil, nil } diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go index cd517b0f7..521cd4510 100644 --- a/exp/api/v1beta1/zz_generated.deepcopy.go +++ b/exp/api/v1beta1/zz_generated.deepcopy.go @@ -310,12 +310,12 @@ func (in *GCPManagedControlPlaneSpec) DeepCopyInto(out *GCPManagedControlPlaneSp } if in.LoggingService != nil { in, out := &in.LoggingService, &out.LoggingService - *out = new(string) + *out = new(LoggingService) **out = **in } if in.MonitoringService != nil { in, out := &in.MonitoringService, &out.MonitoringService - *out = new(string) + *out = new(MonitoringService) **out = **in } }