Skip to content

Commit 5e0896b

Browse files
slawqoslawqo
committed
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>
1 parent c4e1cc2 commit 5e0896b

14 files changed

Lines changed: 215 additions & 7 deletions

‎api/bases/ovn.openstack.org_ovndbclusters.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,12 @@ spec:
428428
description: InternalDBAddress - DB IP address used by other Pods
429429
in the cluster
430430
type: string
431+
internalDbAddressRbacFullAccess:
432+
description: |-
433+
InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
434+
requires full access to the SB db, like e.g. Northd. This is used only
435+
when OVN RBAC for ovn-controllers is used (TLS enabled)
436+
type: string
431437
lastAppliedTopology:
432438
description: LastAppliedTopology - the last applied Topology
433439
properties:

‎api/test/helpers/crd.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ func (th *TestHelper) SimulateOVNNorthdReady(name types.NamespacedName) {
130130
gomega.Eventually(func(g gomega.Gomega) {
131131
service := th.GetOVNNorthd(name)
132132
service.Status.ObservedGeneration = service.Generation
133+
service.Status.ReadyCount = 1
133134
service.Status.Conditions.MarkTrue(condition.ReadyCondition, "Ready")
134135
g.Expect(th.K8sClient.Status().Update(th.Ctx, service)).To(gomega.Succeed())
135136
}, th.Timeout, th.Interval).Should(gomega.Succeed())

‎api/v1beta1/client.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,27 @@ func getItems(list client.ObjectList) []client.Object {
107107
return items
108108
}
109109

110+
// GetOVNNorthd - return OVNNorthd in the given namespace
111+
func GetOVNNorthd(
112+
ctx context.Context,
113+
h *helper.Helper,
114+
namespace string,
115+
) (*OVNNorthd, error) {
116+
ovnNorthdList := &OVNNorthdList{}
117+
listOpts := []client.ListOption{
118+
client.InNamespace(namespace),
119+
}
120+
err := h.GetClient().List(ctx, ovnNorthdList, listOpts...)
121+
if err != nil {
122+
return nil, err
123+
}
124+
if len(ovnNorthdList.Items) > 0 {
125+
return &ovnNorthdList.Items[0], nil
126+
}
127+
128+
return nil, nil
129+
}
130+
110131
// OVNCRNamespaceMapFunc // Generic function to watch any OVN CR
111132
func OVNCRNamespaceMapFunc(crs client.ObjectList, reader client.Reader) handler.MapFunc {
112133
return func(ctx context.Context, obj client.Object) []reconcile.Request {

‎api/v1beta1/ovndbcluster_types.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ type OVNDBClusterStatus struct {
168168
// InternalDBAddress - DB IP address used by other Pods in the cluster
169169
InternalDBAddress string `json:"internalDbAddress,omitempty"`
170170

171+
// InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
172+
// requires full access to the SB db, like e.g. Northd. This is used only
173+
// when OVN RBAC for ovn-controllers is used (TLS enabled)
174+
InternalDBAddressRbacFullAccess string `json:"internalDbAddressRbacFullAccess,omitempty"`
175+
171176
// NetworkAttachments status of the deployment pods
172177
NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`
173178

@@ -236,6 +241,23 @@ func (instance OVNDBCluster) GetInternalEndpoint() (string, error) {
236241
return instance.Status.InternalDBAddress, nil
237242
}
238243

244+
// GetInternalEndpointRbacFullAccess - return the DNS name that openshift coreDNS can resolve
245+
func (instance OVNDBCluster) GetInternalEndpointRbacFullAccess() (string, error) {
246+
if !instance.Spec.TLS.Enabled() {
247+
// if TLS is disabled, this is the same as internalDbAddress
248+
return instance.GetInternalEndpoint()
249+
}
250+
if instance.Spec.DBType != SBDBType {
251+
// if DBType is not SB, this is the same as internalDbAddress
252+
return instance.GetInternalEndpoint()
253+
}
254+
255+
if instance.Status.InternalDBAddressRbacFullAccess == "" {
256+
return "", fmt.Errorf("internal DBEndpoint not ready yet for %s", instance.Spec.DBType)
257+
}
258+
return instance.Status.InternalDBAddressRbacFullAccess, nil
259+
}
260+
239261
// GetExternalEndpoint - return the DNS that openstack dnsmasq can resolve
240262
func (instance OVNDBCluster) GetExternalEndpoint() (string, error) {
241263
if (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) && instance.Status.DBAddress == "" {

‎config/crd/bases/ovn.openstack.org_ovndbclusters.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,12 @@ spec:
428428
description: InternalDBAddress - DB IP address used by other Pods
429429
in the cluster
430430
type: string
431+
internalDbAddressRbacFullAccess:
432+
description: |-
433+
InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
434+
requires full access to the SB db, like e.g. Northd. This is used only
435+
when OVN RBAC for ovn-controllers is used (TLS enabled)
436+
type: string
431437
lastAppliedTopology:
432438
description: LastAppliedTopology - the last applied Topology
433439
properties:

‎internal/controller/ovncontroller_controller.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ func (r *OVNControllerReconciler) SetupWithManager(mgr ctrl.Manager) error {
267267
Owns(&rbacv1.Role{}).
268268
Owns(&rbacv1.RoleBinding{}).
269269
Watches(&ovnv1.OVNDBCluster{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))).
270+
Watches(&ovnv1.OVNNorthd{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))).
270271
Watches(
271272
&corev1.Secret{},
272273
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
@@ -641,6 +642,26 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
641642
return ctrl.Result{}, fmt.Errorf("waiting for Topology requirements: %w", err)
642643
}
643644

645+
// When TLS is enabled, RBAC is used for ovn-controller connections to the SB
646+
// database. The RBAC permission rules are populated by ovn-northd, so we must
647+
// wait for northd to be ready before deploying ovn-controller to avoid RBAC
648+
// permission errors during startup.
649+
if instance.Spec.TLS.Enabled() {
650+
northd, err := ovnv1.GetOVNNorthd(ctx, helper, instance.Namespace)
651+
if err != nil {
652+
return ctrl.Result{}, fmt.Errorf("failed to look up OVNNorthd: %w", err)
653+
}
654+
if northd == nil || northd.Status.ReadyCount == 0 {
655+
Log.Info("OVNNorthd is not ready yet, waiting before deploying OVNController to avoid RBAC errors")
656+
instance.Status.Conditions.Set(condition.FalseCondition(
657+
condition.DeploymentReadyCondition,
658+
condition.RequestedReason,
659+
condition.SeverityInfo,
660+
condition.DeploymentReadyRunningMessage))
661+
return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil
662+
}
663+
}
664+
644665
// Define a new DaemonSet object for OVNController
645666
dset := daemonset.NewDaemonSet(
646667
ovncontroller.CreateOVNDaemonSet(instance, inputHash, ovnServiceLabels, topology),

‎internal/controller/ovndbcluster_controller.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
738738
instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
739739
instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage)
740740
internalDbAddress := []string{}
741+
internalDbAddressRbacFullAccess := []string{}
741742
var svcPort int32
742743
scheme := "tcp"
743744
if instance.Spec.TLS.Enabled() {
@@ -753,6 +754,12 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
753754
// TODO: Watch operator.openshift.io resource once cluster domain is customizable
754755
clusterDomain := clusterdns.GetDNSClusterDomain()
755756
internalDbAddress = append(internalDbAddress, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, clusterDomain, svcPort))
757+
758+
// if TLS is enabled and DBType is SB, RBAC for ovn-controller is used, so additionally
759+
// set the internalDbAddressRbacFullAccess has to be set
760+
if instance.Spec.TLS.Enabled() && instance.Spec.DBType == ovnv1.SBDBType {
761+
internalDbAddressRbacFullAccess = append(internalDbAddressRbacFullAccess, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, clusterDomain, ovndbcluster.DbPortSBRBACFullAccess))
762+
}
756763
}
757764

758765
// Note setting this to the singular headless service address (e.g ssl:ovsdbserver-sb...) "works" but will not
@@ -762,6 +769,9 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
762769

763770
// Set DB Address
764771
instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",")
772+
if len(internalDbAddressRbacFullAccess) > 0 {
773+
instance.Status.InternalDBAddressRbacFullAccess = strings.Join(internalDbAddressRbacFullAccess, ",")
774+
}
765775
if instance.Spec.DBType == ovnv1.SBDBType && (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) {
766776
// This config map will populate the sb db address to edpm, can't use the nb
767777
// If there's no networkAttachments the configMap is not needed

‎internal/controller/ovnnorthd_controller.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,17 @@ func getInternalEndpoint(
642642
if err != nil {
643643
return "", err
644644
}
645-
internalEndpoint, err := cluster.GetInternalEndpoint()
645+
internalEndpoint := ""
646+
if instance.Spec.TLS.Enabled() && dbType == ovnv1.SBDBType {
647+
// When TLS is enabled, OVN is configured to use RBAC and in that case
648+
// the "regular" internal endpoint is provides limited access to the SB
649+
// for ovn-controllers. Northd howerver needs endpoint with full access
650+
// to the SB db
651+
internalEndpoint, err = cluster.GetInternalEndpointRbacFullAccess()
652+
} else {
653+
internalEndpoint, err = cluster.GetInternalEndpoint()
654+
}
655+
646656
if err != nil {
647657
return "", err
648658
}

‎test/functional/base_test.go‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,29 @@ func GetDefaultOVNDBClusterSpec() ovnv1.OVNDBClusterSpec {
8080
}
8181
}
8282

83-
func GetTLSOVNDBClusterSpec() ovnv1.OVNDBClusterSpec {
83+
func getTLSOVNDBClusterSpecWithTLSSecrets(caBundleSecretName, certSecretName string) ovnv1.OVNDBClusterSpec {
8484
spec := GetDefaultOVNDBClusterSpec()
8585
spec.TLS = tls.SimpleService{
8686
Ca: tls.Ca{
87-
CaBundleSecretName: CABundleSecretName,
87+
CaBundleSecretName: caBundleSecretName,
8888
},
8989
GenericService: tls.GenericService{
90-
SecretName: ptr.To(OvnDbCertSecretName),
90+
SecretName: ptr.To(certSecretName),
9191
},
9292
}
9393
return spec
9494
}
9595

96+
func GetTLSOVNDBClusterSpec() ovnv1.OVNDBClusterSpec {
97+
return getTLSOVNDBClusterSpecWithTLSSecrets(CABundleSecretName, OvnDbCertSecretName)
98+
}
99+
100+
// ovnDBClusterTestTLSSecrets names the K8s secrets used by OVNDBCluster TLS (nil means no TLS).
101+
type ovnDBClusterTestTLSSecrets struct {
102+
CaBundle string
103+
Cert string
104+
}
105+
96106
func CreateOVNDBCluster(namespace string, spec ovnv1.OVNDBClusterSpec) client.Object {
97107
name := ovn.CreateOVNDBCluster(nil, namespace, spec)
98108
return ovn.GetOVNDBCluster(name)
@@ -113,9 +123,32 @@ func ScaleDBCluster(name types.NamespacedName, replicas int32) {
113123

114124
// CreateOVNDBClusters Creates NB and SB OVNDBClusters
115125
func CreateOVNDBClusters(namespace string, nad map[string][]string, replicas int32) []types.NamespacedName {
126+
return createOVNDBClusters(namespace, nad, replicas, nil)
127+
}
128+
129+
// CreateTLSOVNDBClusters Creates NB and SB OVNDBClusters with TLS
130+
func CreateTLSOVNDBClusters(namespace string, nad map[string][]string, replicas int32) []types.NamespacedName {
131+
return createOVNDBClusters(namespace, nad, replicas, &ovnDBClusterTestTLSSecrets{
132+
CaBundle: CABundleSecretName,
133+
Cert: OvnDbCertSecretName,
134+
})
135+
}
136+
137+
// CreateTLSOVNDBClustersUsingSecrets Creates NB and SB OVNDBClusters with TLS using the given secret names.
138+
func CreateTLSOVNDBClustersUsingSecrets(namespace string, nad map[string][]string, replicas int32, caBundleSecret, certSecret string) []types.NamespacedName {
139+
return createOVNDBClusters(namespace, nad, replicas, &ovnDBClusterTestTLSSecrets{
140+
CaBundle: caBundleSecret,
141+
Cert: certSecret,
142+
})
143+
}
144+
145+
func createOVNDBClusters(namespace string, nad map[string][]string, replicas int32, tlsSecrets *ovnDBClusterTestTLSSecrets) []types.NamespacedName {
116146
dbs := []types.NamespacedName{}
117147
for _, db := range []string{ovnv1.NBDBType, ovnv1.SBDBType} {
118148
spec := GetDefaultOVNDBClusterSpec()
149+
if tlsSecrets != nil {
150+
spec = getTLSOVNDBClusterSpecWithTLSSecrets(tlsSecrets.CaBundle, tlsSecrets.Cert)
151+
}
119152
stringNad := ""
120153
// OVNDBCluster doesn't allow multiple NADs, hence map len
121154
// must be <= 1
@@ -157,7 +190,11 @@ func CreateOVNDBClusters(namespace string, nad map[string][]string, replicas int
157190
endpoint := ""
158191
// Check External endpoint when NAD is set
159192
if len(nad) == 0 {
160-
endpoint, _ = ovndbcluster.GetInternalEndpoint()
193+
if tlsSecrets != nil && db == ovnv1.SBDBType {
194+
endpoint, _ = ovndbcluster.GetInternalEndpointRbacFullAccess()
195+
} else {
196+
endpoint, _ = ovndbcluster.GetInternalEndpoint()
197+
}
161198
} else {
162199
endpoint, _ = ovndbcluster.GetExternalEndpoint()
163200
}

‎test/functional/ovncontroller_controller_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -912,6 +912,9 @@ var _ = Describe("OVNController controller", func() {
912912
BeforeEach(func() {
913913
dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
914914
DeferCleanup(DeleteOVNDBClusters, dbs)
915+
ovnNorthdName := ovn.CreateOVNNorthd(nil, namespace, GetDefaultOVNNorthdSpec())
916+
DeferCleanup(ovn.DeleteOVNNorthd, ovnNorthdName)
917+
ovn.SimulateOVNNorthdReady(ovnNorthdName)
915918
instance := CreateOVNController(namespace, GetTLSOVNControllerSpec())
916919
DeferCleanup(th.DeleteInstance, instance)
917920

0 commit comments

Comments
 (0)