Use RBAC while connecting ovn-controllers to SB database

slawqo · slawqo · commit 445a848ae48c · 2026-03-20T15:22:30.000+01:00
This patch configures RBAC to access OVN SB databases so that ovn-controllers now have limited access to this DB and will only be able to modify its own data. On the other hand Northd requires "full access" to the SB DB, and to achieve that there is another DB listener created on port 16642 for to be used by northd. More info about OVN RBAC can be found in its documentation at [1]. [1] https://docs.ovn.org/en/latest/tutorials/ovn-rbac.html Closes: #OSPRH-1921 Closes: #OSPRH-1922
diff --git a/api/bases/ovn.openstack.org_ovndbclusters.yaml b/api/bases/ovn.openstack.org_ovndbclusters.yaml
@@ -428,6 +428,12 @@ spec:
                 description: InternalDBAddress - DB IP address used by other Pods
                   in the cluster
                 type: string
+              internalDbAddressRbacFullAccess:
+                description: |-
+                  InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
+                  requires full access to the SB db, like e.g. Northd. This is used only
+                  when OVN RBAC for ovn-controllers is used (TLS enabled)
+                type: string
               lastAppliedTopology:
                 description: LastAppliedTopology - the last applied Topology
                 properties:
diff --git a/api/v1beta1/ovndbcluster_types.go b/api/v1beta1/ovndbcluster_types.go
@@ -168,6 +168,11 @@ type OVNDBClusterStatus struct {
 	// InternalDBAddress - DB IP address used by other Pods in the cluster
 	InternalDBAddress string `json:"internalDbAddress,omitempty"`
 
+	// InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
+	// requires full access to the SB db, like e.g. Northd. This is used only
+	// when OVN RBAC for ovn-controllers is used (TLS enabled)
+	InternalDBAddressRbacFullAccess string `json:"internalDbAddressRbacFullAccess,omitempty"`
+
 	// NetworkAttachments status of the deployment pods
 	NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`
 
@@ -236,6 +241,23 @@ func (instance OVNDBCluster) GetInternalEndpoint() (string, error) {
 	return instance.Status.InternalDBAddress, nil
 }
 
+// GetInternalEndpointRbacFullAccess - return the DNS name that openshift coreDNS can resolve
+func (instance OVNDBCluster) GetInternalEndpointRbacFullAccess() (string, error) {
+	if !instance.Spec.TLS.Enabled() {
+		// if TLS is disabled, this is the same as internalDbAddress
+		return instance.GetInternalEndpoint()
+	}
+	if instance.Spec.DBType != SBDBType {
+		// if DBType is not SB, this is the same as internalDbAddress
+		return instance.GetInternalEndpoint()
+	}
+
+	if instance.Status.InternalDBAddressRbacFullAccess == "" {
+		return "", fmt.Errorf("internal DBEndpoint not ready yet for %s", instance.Spec.DBType)
+	}
+	return instance.Status.InternalDBAddressRbacFullAccess, nil
+}
+
 // GetExternalEndpoint - return the DNS that openstack dnsmasq can resolve
 func (instance OVNDBCluster) GetExternalEndpoint() (string, error) {
 	if (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) && instance.Status.DBAddress == "" {
diff --git a/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml b/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml
@@ -428,6 +428,12 @@ spec:
                 description: InternalDBAddress - DB IP address used by other Pods
                   in the cluster
                 type: string
+              internalDbAddressRbacFullAccess:
+                description: |-
+                  InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
+                  requires full access to the SB db, like e.g. Northd. This is used only
+                  when OVN RBAC for ovn-controllers is used (TLS enabled)
+                type: string
               lastAppliedTopology:
                 description: LastAppliedTopology - the last applied Topology
                 properties:
diff --git a/internal/controller/ovndbcluster_controller.go b/internal/controller/ovndbcluster_controller.go
@@ -719,6 +719,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
 		instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage)
 		internalDbAddress := []string{}
+		internalDbAddressRbacFullAccess := []string{}
 		var svcPort int32
 		scheme := "tcp"
 		if instance.Spec.TLS.Enabled() {
@@ -734,6 +735,12 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 			// TODO: Watch operator.openshift.io resource once cluster domain is customizable
 			clusterDomain := clusterdns.GetDNSClusterDomain()
 			internalDbAddress = append(internalDbAddress, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, clusterDomain, svcPort))
+
+			// if TLS is enabled and DBType is SB, RBAC for ovn-controller is used, so additionally
+			// set the internalDbAddressRbacFullAccess has to be set
+			if instance.Spec.TLS.Enabled() && instance.Spec.DBType == ovnv1.SBDBType {
+				internalDbAddressRbacFullAccess = append(internalDbAddressRbacFullAccess, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, clusterDomain, ovndbcluster.DbPortSBRBACFullAccess))
+			}
 		}
 
 		// Note setting this to the singular headless service address (e.g ssl:ovsdbserver-sb...) "works" but will not
@@ -743,6 +750,9 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 
 		// Set DB Address
 		instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",")
+		if len(internalDbAddressRbacFullAccess) > 0 {
+			instance.Status.InternalDBAddressRbacFullAccess = strings.Join(internalDbAddressRbacFullAccess, ",")
+		}
 		if instance.Spec.DBType == ovnv1.SBDBType && (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) {
 			// This config map will populate the sb db address to edpm, can't use the nb
 			// If there's no networkAttachments the configMap is not needed
diff --git a/internal/controller/ovnnorthd_controller.go b/internal/controller/ovnnorthd_controller.go
@@ -642,7 +642,17 @@ func getInternalEndpoint(
 	if err != nil {
 		return "", err
 	}
-	internalEndpoint, err := cluster.GetInternalEndpoint()
+	internalEndpoint := ""
+	if instance.Spec.TLS.Enabled() && dbType == ovnv1.SBDBType {
+		// When TLS is enabled, OVN is configured to use RBAC and in that case
+		// the "regular" internal endpoint is provides limited access to the SB
+		// for ovn-controllers. Northd howerver needs endpoint with full access
+		// to the SB db
+		internalEndpoint, err = cluster.GetInternalEndpointRbacFullAccess()
+	} else {
+		internalEndpoint, err = cluster.GetInternalEndpoint()
+	}
+
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/ovndbcluster/const.go b/internal/ovndbcluster/const.go
@@ -9,6 +9,11 @@ const (
 
 	// DbPortSB is the port number for the OVN Southbound database
 	DbPortSB int32 = 6642
+	// DbPortSBRBACFullAccess is the port number for the OVN Southbound database
+	// which provides connection with full access to the SB db.
+	// In case when DbPortSB provides RBAC listener for ovn-controller,
+	// this port is used to provide listener with full write access used by Northd.
+	DbPortSBRBACFullAccess int32 = 16642
 	// RaftPortSB is the port number for the OVN Southbound database Raft protocol
 	RaftPortSB int32 = 6644
 )
diff --git a/internal/ovndbcluster/service.go b/internal/ovndbcluster/service.go
@@ -37,6 +37,13 @@ func Service(
 			Protocol: corev1.ProtocolTCP,
 		},
 	}
+	if instance.Spec.TLS.Enabled() && instance.Spec.DBType == ovnv1.SBDBType {
+		ports = append(ports, corev1.ServicePort{
+			Name:     dbPortName + "-rbac-full-access",
+			Port:     DbPortSBRBACFullAccess,
+			Protocol: corev1.ProtocolTCP,
+		})
+	}
 
 	// Add metrics port if metrics are enabled and exporter image is specified
 	if instance.Spec.ExporterImage != "" && (instance.Spec.MetricsEnabled == nil || *instance.Spec.MetricsEnabled) {
diff --git a/templates/ovndbcluster/bin/setup.sh b/templates/ovndbcluster/bin/setup.sh
@@ -120,10 +120,24 @@ if [[ "$(hostname)" == "{{ .SERVICE_NAME }}-0" ]]; then
 
 {{- if .TLS }}
     ${CTLCMD} set-ssl {{.OVNDB_KEY_PATH}} {{.OVNDB_CERT_PATH}} {{.OVNDB_CACERT_PATH}}
+    if [[ "${DB_TYPE}" == "sb" ]]; then
+        # Use RBAC for the connections of the ovn-controller to SB
+        ${CTLCMD} set-connection role=ovn-controller ${DB_SCHEME}:${DB_PORT}:${DB_ADDR}
+        # In this case, Northd needs to have full access to the DB so there need
+        # to be another connection defined for it
+        # TODO(slaweq): port has to be also set in Northd
+        ${CTLCMD} -- --id=@conn_uuid create Connection target="${DB_SCHEME}\:16642" -- add SB_Global . connections @conn_uuid
+    else
+        # No RBAC for connecting to the Northbound DB so only one connection
+        # defined is fine
+        ${CTLCMD} set-connection ${DB_SCHEME}:${DB_PORT}:${DB_ADDR}
+    fi
+
 {{- else }}
     ${CTLCMD} del-ssl
-{{- end }}
+    # If TLS is disabled, RBAC can't be used, so one connection defined is enough
     ${CTLCMD} set-connection ${DB_SCHEME}:${DB_PORT}:${DB_ADDR}
+{{- end }}
 
     # OVN does not support setting inactivity-probe through --remote cli arg so
     # we have to set it after database is up.
@@ -138,8 +152,10 @@ if [[ "$(hostname)" == "{{ .SERVICE_NAME }}-0" ]]; then
     # TODO: Consider migrating inactivity probe setting  to config files when
     # we update to ovs 3.3. See --config-file in ovsdb-server(1) for more
     # details.
-    while [ "$(${CTLCMD} get connection . inactivity_probe)" != "{{ .OVN_INACTIVITY_PROBE }}" ]; do
-        ${CTLCMD} --inactivity-probe={{ .OVN_INACTIVITY_PROBE }} set-connection ${DB_SCHEME}:${DB_PORT}:${DB_ADDR}
+    for connection_id in $(${CTLCMD} -f csv --no-headings --columns=_uuid list connection); do
+        while [ "$(${CTLCMD} get connection $connection_id inactivity_probe)" != "{{ .OVN_INACTIVITY_PROBE }}" ]; do
+            ${CTLCMD} set connection $connection_id inactivity_probe={{ .OVN_INACTIVITY_PROBE }}
+        done
     done
     ${CTLCMD} list connection
 
diff --git a/test/functional/base_test.go b/test/functional/base_test.go
@@ -80,19 +80,29 @@ func GetDefaultOVNDBClusterSpec() ovnv1.OVNDBClusterSpec {
 	}
 }
 
-func GetTLSOVNDBClusterSpec() ovnv1.OVNDBClusterSpec {
+func getTLSOVNDBClusterSpecWithTLSSecrets(caBundleSecretName, certSecretName string) ovnv1.OVNDBClusterSpec {
 	spec := GetDefaultOVNDBClusterSpec()
 	spec.TLS = tls.SimpleService{
 		Ca: tls.Ca{
-			CaBundleSecretName: CABundleSecretName,
+			CaBundleSecretName: caBundleSecretName,
 		},
 		GenericService: tls.GenericService{
-			SecretName: ptr.To(OvnDbCertSecretName),
+			SecretName: ptr.To(certSecretName),
 		},
 	}
 	return spec
 }
 
+func GetTLSOVNDBClusterSpec() ovnv1.OVNDBClusterSpec {
+	return getTLSOVNDBClusterSpecWithTLSSecrets(CABundleSecretName, OvnDbCertSecretName)
+}
+
+// ovnDBClusterTestTLSSecrets names the K8s secrets used by OVNDBCluster TLS (nil means no TLS).
+type ovnDBClusterTestTLSSecrets struct {
+	CaBundle string
+	Cert     string
+}
+
 func CreateOVNDBCluster(namespace string, spec ovnv1.OVNDBClusterSpec) client.Object {
 	name := ovn.CreateOVNDBCluster(nil, namespace, spec)
 	return ovn.GetOVNDBCluster(name)
@@ -113,9 +123,32 @@ func ScaleDBCluster(name types.NamespacedName, replicas int32) {
 
 // CreateOVNDBClusters Creates NB and SB OVNDBClusters
 func CreateOVNDBClusters(namespace string, nad map[string][]string, replicas int32) []types.NamespacedName {
+	return createOVNDBClusters(namespace, nad, replicas, nil)
+}
+
+// CreateTLSOVNDBClusters Creates NB and SB OVNDBClusters with TLS
+func CreateTLSOVNDBClusters(namespace string, nad map[string][]string, replicas int32) []types.NamespacedName {
+	return createOVNDBClusters(namespace, nad, replicas, &ovnDBClusterTestTLSSecrets{
+		CaBundle: CABundleSecretName,
+		Cert:     OvnDbCertSecretName,
+	})
+}
+
+// CreateTLSOVNDBClustersUsingSecrets Creates NB and SB OVNDBClusters with TLS using the given secret names.
+func CreateTLSOVNDBClustersUsingSecrets(namespace string, nad map[string][]string, replicas int32, caBundleSecret, certSecret string) []types.NamespacedName {
+	return createOVNDBClusters(namespace, nad, replicas, &ovnDBClusterTestTLSSecrets{
+		CaBundle: caBundleSecret,
+		Cert:     certSecret,
+	})
+}
+
+func createOVNDBClusters(namespace string, nad map[string][]string, replicas int32, tlsSecrets *ovnDBClusterTestTLSSecrets) []types.NamespacedName {
 	dbs := []types.NamespacedName{}
 	for _, db := range []string{ovnv1.NBDBType, ovnv1.SBDBType} {
 		spec := GetDefaultOVNDBClusterSpec()
+		if tlsSecrets != nil {
+			spec = getTLSOVNDBClusterSpecWithTLSSecrets(tlsSecrets.CaBundle, tlsSecrets.Cert)
+		}
 		stringNad := ""
 		// OVNDBCluster doesn't allow multiple NADs, hence map len
 		// must be <= 1
@@ -157,7 +190,11 @@ func CreateOVNDBClusters(namespace string, nad map[string][]string, replicas int
 			endpoint := ""
 			// Check External endpoint when NAD is set
 			if len(nad) == 0 {
-				endpoint, _ = ovndbcluster.GetInternalEndpoint()
+				if tlsSecrets != nil && db == ovnv1.SBDBType {
+					endpoint, _ = ovndbcluster.GetInternalEndpointRbacFullAccess()
+				} else {
+					endpoint, _ = ovndbcluster.GetInternalEndpoint()
+				}
 			} else {
 				endpoint, _ = ovndbcluster.GetExternalEndpoint()
 			}
diff --git a/test/functional/ovnnorthd_controller_test.go b/test/functional/ovnnorthd_controller_test.go
@@ -263,7 +263,18 @@ var _ = Describe("OVNNorthd controller", func() {
 		var ovnNorthdName types.NamespacedName
 
 		BeforeEach(func() {
-			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
+			// OVNDBCluster TLS needs these secrets before its StatefulSet exists; use names
+			// distinct from northd's CABundleSecretName / OvnDbCertSecretName so missing-secret specs stay valid.
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{
+				Name:      OvnDBClusterFuncTLSCaBundleSecretName,
+				Namespace: namespace,
+			}))
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(types.NamespacedName{
+				Name:      OvnDBClusterFuncTLSCertSecretName,
+				Namespace: namespace,
+			}))
+			dbs := CreateTLSOVNDBClustersUsingSecrets(namespace, map[string][]string{}, 1,
+				OvnDBClusterFuncTLSCaBundleSecretName, OvnDBClusterFuncTLSCertSecretName)
 			DeferCleanup(DeleteOVNDBClusters, dbs)
 			spec := GetTLSOVNNorthdSpec()
 			ovnNorthdName = ovn.CreateOVNNorthd(nil, namespace, spec)
@@ -309,7 +320,7 @@ var _ = Describe("OVNNorthd controller", func() {
 			)
 		})
 
-		It("creates a StatefulSet with TLS certs attached", func() {
+		It("creates a StatefulSet with TLS certs attached and DB endpoints set", func() {
 			DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{
 				Name:      CABundleSecretName,
 				Namespace: namespace,
@@ -345,6 +356,8 @@ var _ = Describe("OVNNorthd controller", func() {
 				ContainElement(ContainSubstring("--private-key=")),
 				ContainElement(ContainSubstring("--certificate=")),
 				ContainElement(ContainSubstring("--ca-cert=")),
+				ContainElement(ContainSubstring("--ovnnb-db=ssl:ovsdbserver-nb-0."+namespace+".svc.cluster.local:6641")),
+				ContainElement(ContainSubstring("--ovnsb-db=ssl:ovsdbserver-sb-0."+namespace+".svc.cluster.local:16642")),
 			))
 
 			// Verify metrics container exists and has correct configuration
diff --git a/test/functional/suite_test.go b/test/functional/suite_test.go
@@ -78,6 +78,9 @@ var (
 const (
 	CABundleSecretName  = "combined-ca-bundle" //nolint:gosec // G101: Not actual credentials, just secret name constants
 	OvnDbCertSecretName = "ovndb-tls-cert"     //nolint:gosec // G101: Not actual credentials, just secret name constants
+	// OVNDBCluster TLS secrets for OVNNorthd TLS functional tests (distinct from northd's CABundleSecretName / OvnDbCertSecretName).
+	OvnDBClusterFuncTLSCaBundleSecretName = "ovndbcluster-func-tls-ca-bundle" //nolint:gosec // G101: test fixture name only
+	OvnDBClusterFuncTLSCertSecretName     = "ovndbcluster-func-tls-cert"      //nolint:gosec // G101: test fixture name only
 )
 
 func TestAPIs(t *testing.T) {
