feat(logging): enhance L4LBConfig and LoggingConfig with improved validation rules for optionalFields

Author: 08volt

cmd/glbc/main.go

@@ -192,6 +192,7 @@ func main() {
 	var l4LBConfigClient l4lbconfigclient.Interface
 	if flags.F.ManageL4LBLogging && (flags.F.RunL4Controller || flags.F.RunL4NetLBController) {
 		l4LBConfigCRDMeta := l4lbconfig.CRDMeta()
+		klog.V(0).Info("Ensuring L4LBConfig CRD exists")
 		if _, err := crdHandler.EnsureCRD(l4LBConfigCRDMeta, true); err != nil {
 			klog.Fatalf("Failed to ensure L4LBConfig CRD: %v", err)
 		}

pkg/apis/l4lbconfig/v1/types.go

@@ -37,6 +37,8 @@ type L4LBConfig struct {
 // +k8s:openapi-gen=true
 type L4LBConfigSpec struct {
 	// Logging defines the telemetry and flow logging configuration for the L4 Load Balancer.
+	// +k8s:validation:cel[0]:rule="(self.optionalMode == 'CUSTOM' && has(self.optionalFields) && size(self.optionalFields) > 0) || (self.optionalMode != 'CUSTOM' && (!has(self.optionalFields) || size(self.optionalFields) == 0))"
+	// +k8s:validation:cel[0]:message="optionalFields can only be set when optionalMode is 'CUSTOM', and must be set when optionalMode is 'CUSTOM'"
 	// +optional
 	Logging *LoggingConfig `json:"logging,omitempty"`
 }
@@ -57,7 +59,6 @@ type L4LBConfigList struct {
 }
 
 // LoggingConfig defines the parameters for LB logging.
-// +kubebuilder:validation:XValidation:rule="has(self.optionalFields) && size(self.optionalFields) > 0 ? self.optionalMode == 'CUSTOM' : true",message="optionalFields can only be set when optionalMode is 'CUSTOM'"
 // +k8s:openapi-gen=true
 type LoggingConfig struct {
 	// Enabled allows toggling of Cloud Logging.
@@ -66,14 +67,15 @@ type LoggingConfig struct {
 
 	// SampleRate is the percentage of flows to log, from 0 to 1000000.
 	// 1000000 means 100% of packets are logged.
-	// +kubebuilder:validation:Minimum=0
-	// +kubebuilder:validation:Maximum=1000000
+	// +k8s:validation:cel[0]:rule="self >= 0 && self <= 1000000"
+	// +k8s:validation:cel[0]:message="sampleRate must be between 0 and 1_000_000"
 	// +optional
 	SampleRate *int32 `json:"sampleRate,omitempty"`
 
 	// OptionalMode defines which metadata fields to include in the logs.
 	// Options: INCLUDE_ALL_OPTIONAL, EXCLUDE_ALL_OPTIONAL, CUSTOM.
-	// +kubebuilder:validation:Enum=INCLUDE_ALL_OPTIONAL;EXCLUDE_ALL_OPTIONAL;CUSTOM
+	// +k8s:validation:cel[0]:rule="self in ['INCLUDE_ALL_OPTIONAL', 'EXCLUDE_ALL_OPTIONAL', 'CUSTOM']"
+	// +k8s:validation:cel[0]:message="optionalMode must be one of 'INCLUDE_ALL_OPTIONAL', 'EXCLUDE_ALL_OPTIONAL', 'CUSTOM'"
 	// +optional
 	OptionalMode string `json:"optionalMode,omitempty"`
 

pkg/apis/l4lbconfig/v1/zz_generated.openapi.go

@@ -28,14 +28,14 @@ import (
 	"k8s.io/ingress-gce/pkg/crd"
 	"k8s.io/ingress-gce/pkg/flags"
 	"k8s.io/ingress-gce/pkg/l4annotations"
-	common "k8s.io/kube-openapi/pkg/common"
 )
 
 var (
-	ErrL4LBConfigDoesNotExist = errors.New("no L4LBConfig for service port exists.")
-	ErrL4LBConfigFailedToGet  = errors.New("client had error getting L4LBConfig for service.")
-	ErrL4LBConfigInvalidMode  = errors.New("invalid OptionalMode in L4LBConfig for service.")
-	allowedOptionalModes      = []interface{}{"INCLUDE_ALL_OPTIONAL", "EXCLUDE_ALL_OPTIONAL", "CUSTOM"}
+	ErrL4LBConfigDoesNotExist      = errors.New("no L4LBConfig for service port exists.")
+	ErrL4LBConfigFailedToGet       = errors.New("client had error getting L4LBConfig for service.")
+	ErrL4LBConfigInvalidMode       = errors.New("invalid OptionalMode in L4LBConfig for service.")
+	ErrL4LBConfigInvalidSampleRate = errors.New("invalid SampleRate in L4LBConfig for service.")
+	allowedOptionalModes           = []interface{}{"INCLUDE_ALL_OPTIONAL", "EXCLUDE_ALL_OPTIONAL", "CUSTOM"}
 )
 
 const (
@@ -62,37 +62,12 @@ func CRDMeta() *crd.CRDMeta {
 		"l4lbconfig",
 		"l4lbconfigs",
 		[]*crd.Version{
-			crd.NewVersion("v1", "k8s.io/ingress-gce/pkg/apis/l4lbconfig/v1.L4LBConfig", getOpenAPIDefinitions, false),
+			crd.NewVersion("v1", "k8s.io/ingress-gce/pkg/apis/l4lbconfig/v1.L4LBConfig", l4lbconfigv1.GetOpenAPIDefinitions, false),
 		},
 	)
 	return meta
 }
 
-func getOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {
-	defs := l4lbconfigv1.GetOpenAPIDefinitions(ref)
-
-	if def, ok := defs["k8s.io/ingress-gce/pkg/apis/l4lbconfig/v1.LoggingConfig"]; ok {
-		// Patch SampleRate
-		if prop, ok := def.Schema.SchemaProps.Properties["sampleRate"]; ok {
-			min := minSampleRate
-			max := maxSampleRate
-			prop.SchemaProps.Minimum = &min
-			prop.SchemaProps.Maximum = &max
-			def.Schema.SchemaProps.Properties["sampleRate"] = prop
-		}
-
-		// Patch OptionalMode (Enum)
-		if prop, ok := def.Schema.SchemaProps.Properties["optionalMode"]; ok {
-			prop.SchemaProps.Enum = allowedOptionalModes
-			def.Schema.SchemaProps.Properties["optionalMode"] = prop
-		}
-
-		defs["k8s.io/ingress-gce/pkg/apis/l4lbconfig/v1.LoggingConfig"] = def
-	}
-
-	return defs
-}
-
 // GetL4LBConfigForService returns the corresponding L4LBConfig for
 // the given Service if specified.
 func GetL4LBConfigForService(l4lbConfigLister cache.Store, svc *corev1.Service) (*l4lbconfigv1.L4LBConfig, error) {
@@ -157,13 +132,17 @@ func DetermineL4LoggingConfig(
 		resolvedConfig.OptionalMode = "EXCLUDE_ALL_OPTIONAL"
 	}
 
+	// Double-check OptionalMode validity before proceeding with OptionalFields checks
+	// Validation already occurs at the API level, but this serves as a safeguard against any unexpected values.
 	switch resolvedConfig.OptionalMode {
 	case "EXCLUDE_ALL_OPTIONAL", "INCLUDE_ALL_OPTIONAL", "CUSTOM":
 		// Valid
 	default:
 		return nil, NewConditionLoggingInvalid(ErrL4LBConfigInvalidMode), ErrL4LBConfigInvalidMode
 	}
 
+	// Double-check consistency between OptionalMode and OptionalFields
+	// Validation already occurs at the API level, but this serves as a safeguard against any unexpected states.
 	if resolvedConfig.OptionalMode != "CUSTOM" && len(resolvedConfig.OptionalFields) > 0 {
 		return nil, NewConditionLoggingInvalid(ErrL4LBConfigInvalidMode), ErrL4LBConfigInvalidMode
 	}
@@ -178,6 +157,12 @@ func DetermineL4LoggingConfig(
 		resolvedConfig.SampleRate = defaultSampleRate
 	}
 
+	// Doube-check SampleRate validity before returning the resolved config
+	// Validation already occurs at the API level, but this serves as a safeguard against any unexpected values.
+	if resolvedConfig.SampleRate < 0 || resolvedConfig.SampleRate > 1 {
+		return nil, NewConditionLoggingInvalid(ErrL4LBConfigInvalidSampleRate), ErrL4LBConfigInvalidSampleRate
+	}
+
 	return resolvedConfig, NewConditionLoggingReconciled(), nil
 }