NETOBSERV-1532: add TLS support to ebpf agent metrics config (#602)

* NETOBSERV-1532: add TLS support to ebpf agent metrics config Signed-off-by: Mohamed Mahmoud <[email protected]> * update APIs to mark TLS Provided mode as Unsupported Signed-off-by: Mohamed Mahmoud <[email protected]> * NETOBSERV-1585: fix issues exposing metrics upstream/downstream - Fix using TLS with user workload monitoring - Do not set openshift-monitoring annotation on netobserv namespace upstream - Fix tagging DOWNSTREAM DEPLOYMENT on netobserv-privileged namespace * Use helper function for agent servicemonitor TLS --------- Signed-off-by: Mohamed Mahmoud <[email protected]> Co-authored-by: Joel Takvorian <[email protected]>
netobserv · Mar 28, 2024 · fd49392 · fd49392
1 parent caab987
commit fd49392
Show file tree

Hide file tree

Showing 15 changed files with 176 additions and 97 deletions.
diff --git a/apis/flowcollector/v1beta1/flowcollector_types.go b/apis/flowcollector/v1beta1/flowcollector_types.go
@@ -301,7 +301,7 @@ type ServerTLSConfigType string
 type ServerTLS struct {
 	// Select the type of TLS configuration:<br>
 	// - `DISABLED` (default) to not configure TLS for the endpoint.
-	// - `PROVIDED` to manually provide cert file and a key file.
+	// - `PROVIDED` to manually provide cert file and a key file. [Unsupported (*)].
 	// - `AUTO` to use OpenShift auto generated certificate using annotations.
 	// +unionDiscriminator
 	// +kubebuilder:validation:Enum:="DISABLED";"PROVIDED";"AUTO"

diff --git a/apis/flowcollector/v1beta2/flowcollector_types.go b/apis/flowcollector/v1beta2/flowcollector_types.go
@@ -308,7 +308,7 @@ const (
 type ServerTLS struct {
 	// Select the type of TLS configuration:<br>
 	// - `Disabled` (default) to not configure TLS for the endpoint.
-	// - `Provided` to manually provide cert file and a key file.
+	// - `Provided` to manually provide cert file and a key file. [Unsupported (*)].
 	// - `Auto` to use OpenShift auto generated certificate using annotations.
 	// +unionDiscriminator
 	// +kubebuilder:validation:Enum:="Disabled";"Provided";"Auto"

diff --git a/bundle/manifests/flows.netobserv.io_flowcollectors.yaml b/bundle/manifests/flows.netobserv.io_flowcollectors.yaml
@@ -286,8 +286,9 @@ spec:
                                     description: Select the type of TLS configuration:<br>
                                       - `DISABLED` (default) to not configure TLS
                                       for the endpoint. - `PROVIDED` to manually provide
-                                      cert file and a key file. - `AUTO` to use OpenShift
-                                      auto generated certificate using annotations.
+                                      cert file and a key file. [Unsupported (*)].
+                                      - `AUTO` to use OpenShift auto generated certificate
+                                      using annotations.
                                     enum:
                                     - DISABLED
                                     - PROVIDED
@@ -2182,8 +2183,9 @@ spec:
                                 description: Select the type of TLS configuration:<br>
                                   - `DISABLED` (default) to not configure TLS for
                                   the endpoint. - `PROVIDED` to manually provide cert
-                                  file and a key file. - `AUTO` to use OpenShift auto
-                                  generated certificate using annotations.
+                                  file and a key file. [Unsupported (*)]. - `AUTO`
+                                  to use OpenShift auto generated certificate using
+                                  annotations.
                                 enum:
                                 - DISABLED
                                 - PROVIDED
@@ -3029,8 +3031,9 @@ spec:
                                     description: Select the type of TLS configuration:<br>
                                       - `Disabled` (default) to not configure TLS
                                       for the endpoint. - `Provided` to manually provide
-                                      cert file and a key file. - `Auto` to use OpenShift
-                                      auto generated certificate using annotations.
+                                      cert file and a key file. [Unsupported (*)].
+                                      - `Auto` to use OpenShift auto generated certificate
+                                      using annotations.
                                     enum:
                                     - Disabled
                                     - Provided
@@ -6028,8 +6031,9 @@ spec:
                                 description: Select the type of TLS configuration:<br>
                                   - `Disabled` (default) to not configure TLS for
                                   the endpoint. - `Provided` to manually provide cert
-                                  file and a key file. - `Auto` to use OpenShift auto
-                                  generated certificate using annotations.
+                                  file and a key file. [Unsupported (*)]. - `Auto`
+                                  to use OpenShift auto generated certificate using
+                                  annotations.
                                 enum:
                                 - Disabled
                                 - Provided

diff --git a/config/crd/bases/flows.netobserv.io_flowcollectors.yaml b/config/crd/bases/flows.netobserv.io_flowcollectors.yaml
@@ -186,7 +186,7 @@ spec:
                                       type: object
                                     type:
                                       default: DISABLED
-                                      description: Select the type of TLS configuration:<br> - `DISABLED` (default) to not configure TLS for the endpoint. - `PROVIDED` to manually provide cert file and a key file. - `AUTO` to use OpenShift auto generated certificate using annotations.
+                                      description: Select the type of TLS configuration:<br> - `DISABLED` (default) to not configure TLS for the endpoint. - `PROVIDED` to manually provide cert file and a key file. [Unsupported (*)]. - `AUTO` to use OpenShift auto generated certificate using annotations.
                                       enum:
                                         - DISABLED
                                         - PROVIDED
@@ -1695,7 +1695,7 @@ spec:
                                   type: object
                                 type:
                                   default: DISABLED
-                                  description: Select the type of TLS configuration:<br> - `DISABLED` (default) to not configure TLS for the endpoint. - `PROVIDED` to manually provide cert file and a key file. - `AUTO` to use OpenShift auto generated certificate using annotations.
+                                  description: Select the type of TLS configuration:<br> - `DISABLED` (default) to not configure TLS for the endpoint. - `PROVIDED` to manually provide cert file and a key file. [Unsupported (*)]. - `AUTO` to use OpenShift auto generated certificate using annotations.
                                   enum:
                                     - DISABLED
                                     - PROVIDED
@@ -2397,7 +2397,7 @@ spec:
                                       type: object
                                     type:
                                       default: Disabled
-                                      description: Select the type of TLS configuration:<br> - `Disabled` (default) to not configure TLS for the endpoint. - `Provided` to manually provide cert file and a key file. - `Auto` to use OpenShift auto generated certificate using annotations.
+                                      description: Select the type of TLS configuration:<br> - `Disabled` (default) to not configure TLS for the endpoint. - `Provided` to manually provide cert file and a key file. [Unsupported (*)]. - `Auto` to use OpenShift auto generated certificate using annotations.
                                       enum:
                                         - Disabled
                                         - Provided
@@ -4903,7 +4903,7 @@ spec:
                                   type: object
                                 type:
                                   default: Disabled
-                                  description: Select the type of TLS configuration:<br> - `Disabled` (default) to not configure TLS for the endpoint. - `Provided` to manually provide cert file and a key file. - `Auto` to use OpenShift auto generated certificate using annotations.
+                                  description: Select the type of TLS configuration:<br> - `Disabled` (default) to not configure TLS for the endpoint. - `Provided` to manually provide cert file and a key file. [Unsupported (*)]. - `Auto` to use OpenShift auto generated certificate using annotations.
                                   enum:
                                     - Disabled
                                     - Provided

diff --git a/config/openshift/namespace.yaml b/config/openshift/namespace.yaml
@@ -1,6 +1,4 @@
 apiVersion: v1
 kind: Namespace
 metadata:
-  labels:
-    openshift.io/cluster-monitoring: "true"
   name: openshift-netobserv-operator
diff --git a/controllers/ebpf/agent-metrics-test.go b/controllers/ebpf/agent-metrics-test.go
@@ -36,8 +36,16 @@ func TestPromServiceMonitoring(t *testing.T) {
 	// Create a new instance of your controller
 	controller := &AgentController{}
 
+	// Create a sample FlowCollectorEBPF object for testing
+	target := &flowslatest.FlowCollectorEBPF{
+		Metrics: flowslatest.EBPFMetrics{
+			Server: flowslatest.MetricsServerConfig{
+				Port: 8080, // Sample port for testing
+			},
+		},
+	}
 	// Call the promServiceMonitoring function
-	monitor := controller.promServiceMonitoring()
+	monitor := controller.promServiceMonitoring(target)
 
 	// Assert that the returned monitor is not nil
 	assert.NotNil(t, monitor)

diff --git a/controllers/ebpf/agent-metrics.go b/controllers/ebpf/agent-metrics.go
@@ -2,6 +2,7 @@ package ebpf
 
 import (
 	"context"
+	"fmt"
 
 	flowslatest "github.com/netobserv/network-observability-operator/apis/flowcollector/v1beta2"
 	"github.com/netobserv/network-observability-operator/controllers/constants"
@@ -30,7 +31,7 @@ func (c *AgentController) reconcileMetricsService(ctx context.Context, target *f
 		return err
 	}
 	if c.AvailableAPIs.HasSvcMonitor() {
-		serviceMonitor := c.promServiceMonitoring()
+		serviceMonitor := c.promServiceMonitoring(target)
 		if err := reconcilers.GenericReconcile(ctx, c.Managed, &c.Client, c.serviceMonitor,
 			serviceMonitor, &report, helper.ServiceMonitorChanged); err != nil {
 			return err
@@ -60,11 +61,18 @@ func (c *AgentController) promService(target *flowslatest.FlowCollectorEBPF) *co
 			}},
 		},
 	}
+	if target.Metrics.Server.TLS.Type == flowslatest.ServerTLSAuto {
+		svc.ObjectMeta.Annotations = map[string]string{
+			constants.OpenShiftCertificateAnnotation: constants.EBPFAgentMetricsSvcName,
+		}
+	}
 	return &svc
 }
 
-func (c *AgentController) promServiceMonitoring() *monitoringv1.ServiceMonitor {
-	agentServiceMonitorObject := monitoringv1.ServiceMonitor{
+func (c *AgentController) promServiceMonitoring(target *flowslatest.FlowCollectorEBPF) *monitoringv1.ServiceMonitor {
+	serverName := fmt.Sprintf("%s.%s.svc", constants.EBPFAgentMetricsSvcName, c.PrivilegedNamespace())
+	scheme, smTLS := helper.GetServiceMonitorTLSConfig(&target.Metrics.Server.TLS, serverName, c.IsDownstream)
+	return &monitoringv1.ServiceMonitor{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      constants.EBPFAgentMetricsSvcMonitoringName,
 			Namespace: c.PrivilegedNamespace(),
@@ -75,9 +83,10 @@ func (c *AgentController) promServiceMonitoring() *monitoringv1.ServiceMonitor {
 		Spec: monitoringv1.ServiceMonitorSpec{
 			Endpoints: []monitoringv1.Endpoint{
 				{
-					Port:     "metrics",
-					Interval: "30s",
-					Scheme:   "http",
+					Port:      "metrics",
+					Interval:  "30s",
+					Scheme:    scheme,
+					TLSConfig: smTLS,
 				},
 			},
 			NamespaceSelector: monitoringv1.NamespaceSelector{
@@ -92,6 +101,4 @@ func (c *AgentController) promServiceMonitoring() *monitoringv1.ServiceMonitor {
 			},
 		},
 	}
-
-	return &agentServiceMonitorObject
 }
diff --git a/controllers/ebpf/agent_controller.go b/controllers/ebpf/agent_controller.go
@@ -58,6 +58,8 @@ const (
 	envEnableMetrics              = "METRICS_ENABLE"
 	envMetricsPort                = "METRICS_SERVER_PORT"
 	envMetricPrefix               = "METRICS_PREFIX"
+	envMetricsTLSCertPath         = "METRICS_TLS_CERT_PATH"
+	envMetricsTLSKeyPath          = "METRICS_TLS_KEY_PATH"
 	envListSeparator              = ","
 )
 
@@ -138,16 +140,17 @@ func (c *AgentController) Reconcile(ctx context.Context, target *flowslatest.Flo
 	if err := c.permissions.Reconcile(ctx, &target.Spec.Agent.EBPF); err != nil {
 		return fmt.Errorf("reconciling permissions: %w", err)
 	}
-	desired, err := c.desired(ctx, target, rlog)
-	if err != nil {
-		return err
-	}
 
 	err = c.reconcileMetricsService(ctx, &target.Spec.Agent.EBPF)
 	if err != nil {
 		return fmt.Errorf("reconciling prometheus service: %w", err)
 	}
 
+	desired, err := c.desired(ctx, target, rlog)
+	if err != nil {
+		return err
+	}
+
 	switch helper.DaemonSetChanged(current, desired) {
 	case helper.ActionCreate:
 		rlog.Info("action: create agent")
@@ -199,6 +202,37 @@ func (c *AgentController) desired(ctx context.Context, coll *flowslatest.FlowCol
 	if err != nil {
 		return nil, err
 	}
+
+	if coll.Spec.Agent.EBPF.Metrics.Server.TLS.Type != flowslatest.ServerTLSDisabled {
+		var promTLS *flowslatest.CertificateReference
+		switch coll.Spec.Agent.EBPF.Metrics.Server.TLS.Type {
+		case flowslatest.ServerTLSProvided:
+			promTLS = coll.Spec.Agent.EBPF.Metrics.Server.TLS.Provided
+			if promTLS == nil {
+				rlog.Info("EBPF agent metric tls configuration set to provided but none is provided")
+			}
+		case flowslatest.ServerTLSAuto:
+			promTLS = &flowslatest.CertificateReference{
+				Type:     "secret",
+				Name:     constants.EBPFAgentMetricsSvcName,
+				CertFile: "tls.crt",
+				CertKey:  "tls.key",
+			}
+		case flowslatest.ServerTLSDisabled:
+			// show never happens added for linting purposes
+		}
+		cert, key := c.volumes.AddCertificate(promTLS, "prom-certs")
+		if cert != "" && key != "" {
+			env = append(env, corev1.EnvVar{Name: envMetricsTLSKeyPath,
+				Value: key,
+			})
+			env = append(env, corev1.EnvVar{
+				Name:  envMetricsTLSCertPath,
+				Value: cert,
+			})
+		}
+	}
+
 	volumeMounts := c.volumes.GetMounts()
 	volumes := c.volumes.GetVolumes()
 

diff --git a/controllers/ebpf/internal/permissions/permissions.go b/controllers/ebpf/internal/permissions/permissions.go
@@ -66,35 +66,40 @@ func (c *Reconciler) reconcileNamespace(ctx context.Context) error {
 	}
 	desired := &v1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: ns,
-			Labels: map[string]string{
-				"app":                                constants.OperatorName,
-				"pod-security.kubernetes.io/enforce": "privileged",
-				"pod-security.kubernetes.io/audit":   "privileged",
-			},
+			Name:   ns,
+			Labels: namespaceLabels(true, c.IsDownstream),
 		},
 	}
-	if actual == nil && desired != nil {
+	if actual == nil {
 		rlog.Info("creating namespace")
 		return c.CreateOwned(ctx, desired)
 	}
-	if actual != nil && desired != nil {
-		// We noticed that audit labels are automatically removed
-		// in some configurations of K8s, so to avoid an infinite update loop, we just ignore
-		// it (if the user removes it manually, it's at their own risk)
-		if !helper.IsSubSet(actual.ObjectMeta.Labels,
-			map[string]string{
-				"app":                                constants.OperatorName,
-				"pod-security.kubernetes.io/enforce": "privileged",
-			}) {
-			rlog.Info("updating namespace")
-			return c.UpdateIfOwned(ctx, actual, desired)
-		}
+	// We noticed that audit labels are automatically removed
+	// in some configurations of K8s, so to avoid an infinite update loop, we just ignore
+	// it (if the user removes it manually, it's at their own risk)
+	if !helper.IsSubSet(actual.ObjectMeta.Labels, namespaceLabels(false, c.IsDownstream)) {
+		rlog.Info("updating namespace")
+		return c.UpdateIfOwned(ctx, actual, desired)
 	}
+
 	rlog.Info("namespace is already reconciled. Doing nothing")
 	return nil
 }
 
+func namespaceLabels(includeAudit, isDownstream bool) map[string]string {
+	l := map[string]string{
+		"app":                                constants.OperatorName,
+		"pod-security.kubernetes.io/enforce": "privileged",
+	}
+	if includeAudit {
+		l["pod-security.kubernetes.io/audit"] = "privileged"
+	}
+	if isDownstream {
+		l["openshift.io/cluster-monitoring"] = "true"
+	}
+	return l
+}
+
 func (c *Reconciler) reconcileServiceAccount(ctx context.Context) error {
 	rlog := log.FromContext(ctx, "serviceAccount", constants.EBPFServiceAccount)
 

diff --git a/controllers/flowcollector_controller.go b/controllers/flowcollector_controller.go
@@ -190,5 +190,6 @@ func (r *FlowCollectorReconciler) newCommonInfo(clh *helper.Client, ns, prevNs s
 		AvailableAPIs:     &r.mgr.AvailableAPIs,
 		Watcher:           r.watcher,
 		Loki:              loki,
+		IsDownstream:      r.mgr.Config.DownstreamDeployment,
 	}
 }