Use RBAC while connecting ovn-controllers to SB database

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

Related: #1922

Assisted-by: claude-opus-4.6

Signed-off-by: Slawek Kaplonski <skaplons@redhat.com>

Author: slawqo

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:

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 == "" {

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:

internal/controller/ovndbcluster_controller.go

@@ -725,6 +725,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() {
@@ -740,6 +741,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
@@ -749,6 +756,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

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
 	}

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()
 			}

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