NETOBSERV-971 Fix couple of CRD issues on zero-values (#319)

* NETOBSERV-971 Fix couple of CRD issues on zero-values

CRD cleanup / sanitizing

Find & fix a bunch of CRD issues where go zero-value was conflicting
with CRD default. The typical workaround is to use pointers or, for
lists and maps, remove "omitempty".

Add a test to help catch these issues

Found issues:
- portNamings.enable (this was the original issue)
- portNamings.portNames (could not set empty list)
- ebpf.interfaces and excludeInterfaces (e.g. could not set empty exclusion
  list)
- processor.metrics.ignoreTags and disableAlerts (could not set empty
  ignoreTags)
- processor.enableKubeProbles, dropUnusedFields (could not set false)
- processor.kafkaConsumerReplicas (could not set 0)
- loki.maxRetries (could not set 0)
- loki.staticLabels (could not set empty list)
- plugin.register (could not set false)
- plugin.replicas (could not set 0)
- plugin.quickFilters (could not set empry list)

* Use pointer package

api/v1beta1/flowcollector_types.go

@@ -182,13 +182,14 @@ type FlowCollectorEBPF struct {
 	// If an entry is enclosed by slashes (such as `/br-/`), it will match as regular expression,
 	// otherwise it will be matched as a case-sensitive string.
 	//+optional
-	Interfaces []string `json:"interfaces,omitempty"`
+	Interfaces []string `json:"interfaces"`
 
 	// excludeInterfaces contains the interface names that will be excluded from flow tracing.
 	// If an entry is enclosed by slashes (such as `/br-/`), it will match as regular expression,
 	// otherwise it will be matched as a case-sensitive string.
 	//+kubebuilder:default=lo;
-	ExcludeInterfaces []string `json:"excludeInterfaces,omitempty"`
+	//+optional
+	ExcludeInterfaces []string `json:"excludeInterfaces"`
 
 	//+kubebuilder:validation:Enum=trace;debug;info;warn;error;fatal;panic
 	//+kubebuilder:default:=info
@@ -292,14 +293,15 @@ type FLPMetrics struct {
 	// ignoreTags is a list of tags to specify which metrics to ignore. Each metric is associated with a list of tags. More details in https://github.com/netobserv/network-observability-operator/tree/main/controllers/flowlogspipeline/metrics_definitions .
 	// Available tags are: egress, ingress, flows, bytes, packets, namespaces, nodes, workloads
 	//+kubebuilder:default:={"egress","packets"}
-	IgnoreTags []string `json:"ignoreTags,omitempty"`
+	// +optional
+	IgnoreTags []string `json:"ignoreTags"`
 
 	// disableAlerts is a list of alerts that should be disabled.
 	// Possible values are:
 	// `NetObservNoFlows`, which is triggered when no flows are being observed for a certain period.
 	// `NetObservLokiError`, which is triggered when flows are being dropped due to Loki errors.
 	// +optional
-	DisableAlerts []FLPAlert `json:"disableAlerts,omitempty"`
+	DisableAlerts []FLPAlert `json:"disableAlerts"`
 }
 
 const (
@@ -354,17 +356,17 @@ type FlowCollectorFLP struct {
 
 	//+kubebuilder:default:=true
 	// enableKubeProbes is a flag to enable or disable Kubernetes liveness and readiness probes
-	EnableKubeProbes bool `json:"enableKubeProbes,omitempty"`
+	EnableKubeProbes *bool `json:"enableKubeProbes,omitempty"`
 
 	//+kubebuilder:default:=true
 	// dropUnusedFields allows, when set to true, to drop fields that are known to be unused by OVS, in order to save storage space.
-	DropUnusedFields bool `json:"dropUnusedFields,omitempty"`
+	DropUnusedFields *bool `json:"dropUnusedFields,omitempty"`
 
 	//+kubebuilder:validation:Minimum=0
 	//+kubebuilder:default:=3
 	// kafkaConsumerReplicas defines the number of replicas (pods) to start for flowlogs-pipeline-transformer, which consumes Kafka messages.
 	// This setting is ignored when Kafka is disabled.
-	KafkaConsumerReplicas int32 `json:"kafkaConsumerReplicas,omitempty"`
+	KafkaConsumerReplicas *int32 `json:"kafkaConsumerReplicas,omitempty"`
 
 	// kafkaConsumerAutoscaler spec of a horizontal pod autoscaler to set up for flowlogs-pipeline-transformer, which consumes Kafka messages.
 	// This setting is ignored when Kafka is disabled.
@@ -504,11 +506,12 @@ type FlowCollectorLoki struct {
 	//+kubebuilder:validation:Minimum=0
 	//+kubebuilder:default:=2
 	// maxRetries is the maximum number of retries for client connections.
-	MaxRetries int32 `json:"maxRetries,omitempty"`
+	MaxRetries *int32 `json:"maxRetries,omitempty"`
 
 	//+kubebuilder:default:={"app":"netobserv-flowcollector"}
+	// +optional
 	// staticLabels is a map of common labels to set on each flow.
-	StaticLabels map[string]string `json:"staticLabels,omitempty"`
+	StaticLabels map[string]string `json:"staticLabels"`
 
 	// tls client configuration for loki URL.
 	// +optional
@@ -527,12 +530,12 @@ type FlowCollectorConsolePlugin struct {
 	// register allows, when set to true, to automatically register the provided console plugin with the OpenShift Console operator.
 	// When set to false, you can still register it manually by editing console.operator.openshift.io/cluster.
 	// E.g: oc patch console.operator.openshift.io cluster --type='json' -p '[{"op": "add", "path": "/spec/plugins/-", "value": "netobserv-plugin"}]'
-	Register bool `json:"register"`
+	Register *bool `json:"register,omitempty"`
 
 	//+kubebuilder:validation:Minimum=0
 	//+kubebuilder:default:=1
 	// replicas defines the number of replicas (pods) to start.
-	Replicas int32 `json:"replicas,omitempty"`
+	Replicas *int32 `json:"replicas,omitempty"`
 
 	//+kubebuilder:validation:Minimum=1
 	//+kubebuilder:validation:Maximum=65535
@@ -565,20 +568,21 @@ type FlowCollectorConsolePlugin struct {
 	PortNaming ConsolePluginPortConfig `json:"portNaming,omitempty"`
 
 	//+kubebuilder:default:={{name:"Applications",filter:{"src_namespace!":"openshift-,netobserv","dst_namespace!":"openshift-,netobserv"},default:true},{name:"Infrastructure",filter:{"src_namespace":"openshift-,netobserv","dst_namespace":"openshift-,netobserv"}},{name:"Pods network",filter:{"src_kind":"Pod","dst_kind":"Pod"},default:true},{name:"Services network",filter:{"dst_kind":"Service"}}}
+	// +optional
 	// quickFilters configures quick filter presets for the Console plugin
-	QuickFilters []QuickFilter `json:"quickFilters,omitempty"`
+	QuickFilters []QuickFilter `json:"quickFilters"`
 }
 
 // Configuration of the port to service name translation feature of the console plugin
 type ConsolePluginPortConfig struct {
 	//+kubebuilder:default:=true
 	// enable the console plugin port-to-service name translation
-	Enable bool `json:"enable,omitempty"`
+	Enable *bool `json:"enable,omitempty"`
 
 	// portNames defines additional port names to use in the console.
 	// Example: portNames: {"3100": "loki"}
 	// +optional
-	PortNames map[string]string `json:"portNames,omitempty" yaml:"portNames,omitempty"`
+	PortNames map[string]string `json:"portNames" yaml:"portNames"`
 }
 
 // QuickFilter defines preset configuration for Console's quick filters

bundle/manifests/flows.netobserv.io_flowcollectors.yaml

@@ -3096,8 +3096,6 @@ spec:
                           to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
                         type: object
                     type: object
-                required:
-                - register
                 type: object
               deploymentModel:
                 default: DIRECT

config/crd/bases/flows.netobserv.io_flowcollectors.yaml

@@ -3083,8 +3083,6 @@ spec:
                           to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
                         type: object
                     type: object
-                required:
-                - register
                 type: object
               deploymentModel:
                 default: DIRECT

controllers/consoleplugin/consoleplugin_objects.go

@@ -131,7 +131,7 @@ func (b *builder) deployment(cmDigest string) *appsv1.Deployment {
 			Labels:    b.labels,
 		},
 		Spec: appsv1.DeploymentSpec{
-			Replicas: &b.desired.ConsolePlugin.Replicas,
+			Replicas: b.desired.ConsolePlugin.Replicas,
 			Selector: &metav1.LabelSelector{
 				MatchLabels: b.selector,
 			},

controllers/consoleplugin/consoleplugin_reconciler.go

@@ -115,18 +115,19 @@ func (r *CPReconciler) Reconcile(ctx context.Context, desired *flowslatest.FlowC
 
 func (r *CPReconciler) checkAutoPatch(ctx context.Context, desired *flowslatest.FlowCollector) error {
 	console := operatorsv1.Console{}
+	reg := helper.PtrBool(desired.Spec.ConsolePlugin.Register)
 	if err := r.Client.Get(ctx, types.NamespacedName{Name: "cluster"}, &console); err != nil {
 		// Console operator CR not found => warn but continue execution
-		if desired.Spec.ConsolePlugin.Register {
+		if reg {
 			log.FromContext(ctx).Error(err, "Could not get the Console Operator resource for plugin registration. Please register manually.")
 		}
 		return nil
 	}
 	registered := helper.ContainsString(console.Spec.Plugins, constants.PluginName)
-	if desired.Spec.ConsolePlugin.Register && !registered {
+	if reg && !registered {
 		console.Spec.Plugins = append(console.Spec.Plugins, constants.PluginName)
 		return r.Client.Update(ctx, &console)
-	} else if !desired.Spec.ConsolePlugin.Register && registered {
+	} else if !reg && registered {
 		console.Spec.Plugins = helper.RemoveAllStrings(console.Spec.Plugins, constants.PluginName)
 		return r.Client.Update(ctx, &console)
 	}
@@ -200,7 +201,7 @@ func (r *CPReconciler) reconcileDeployment(ctx context.Context, builder builder,
 		if err := r.CreateOwned(ctx, newDepl); err != nil {
 			return err
 		}
-	} else if helper.DeploymentChanged(r.owned.deployment, newDepl, constants.PluginName, helper.HPADisabled(&desired.ConsolePlugin.Autoscaler), desired.ConsolePlugin.Replicas, &report) {
+	} else if helper.DeploymentChanged(r.owned.deployment, newDepl, constants.PluginName, helper.HPADisabled(&desired.ConsolePlugin.Autoscaler), helper.PtrInt32(desired.ConsolePlugin.Replicas), &report) {
 		if err := r.UpdateOwned(ctx, r.owned.deployment, newDepl); err != nil {
 			return err
 		}

controllers/consoleplugin/consoleplugin_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	promConfig "github.com/prometheus/common/config"
 	"github.com/stretchr/testify/assert"
 	ascv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
@@ -13,8 +14,6 @@ import (
 	flowslatest "github.com/netobserv/network-observability-operator/api/v1beta1"
 	"github.com/netobserv/network-observability-operator/controllers/constants"
 	"github.com/netobserv/network-observability-operator/pkg/helper"
-
-	promConfig "github.com/prometheus/common/config"
 )
 
 const testImage = "quay.io/netobserv/network-observability-console-plugin:dev"

controllers/flowcollector_controller_console_test.go

@@ -77,7 +77,7 @@ func flowCollectorConsolePluginSpecs() {
 					ConsolePlugin: flowslatest.FlowCollectorConsolePlugin{
 						Port:            9001,
 						ImagePullPolicy: "Never",
-						Register:        true,
+						Register:        pointer.Bool(true),
 						Autoscaler: flowslatest.FlowCollectorHPA{
 							Status:      flowslatest.HPAStatusEnabled,
 							MinReplicas: pointer.Int32(1),
@@ -94,7 +94,7 @@ func flowCollectorConsolePluginSpecs() {
 							}},
 						},
 						PortNaming: flowslatest.ConsolePluginPortConfig{
-							Enable: true,
+							Enable: pointer.Bool(true),
 							PortNames: map[string]string{
 								"3100": "loki",
 							},
@@ -151,7 +151,7 @@ func flowCollectorConsolePluginSpecs() {
 					return err
 				}
 				fc.Spec.ConsolePlugin.Port = 9099
-				fc.Spec.ConsolePlugin.Replicas = 2
+				fc.Spec.ConsolePlugin.Replicas = pointer.Int32(2)
 				fc.Spec.ConsolePlugin.Autoscaler.Status = flowslatest.HPAStatusDisabled
 				return k8sClient.Update(ctx, &fc)
 			}).Should(Succeed())
@@ -241,7 +241,7 @@ func flowCollectorConsolePluginSpecs() {
 		It("Should be unregistered", func() {
 			By("Update CR to unregister")
 			UpdateCR(crKey, func(fc *flowslatest.FlowCollector) {
-				fc.Spec.ConsolePlugin.Register = false
+				fc.Spec.ConsolePlugin.Register = pointer.Bool(false)
 			})
 
 			By("Expecting the Console CR to not have plugin registered")