Skip to content

Commit d8cd997

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 40841a9 commit d8cd997

14 files changed

Lines changed: 213 additions & 7 deletions

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,12 @@ spec:
430430
description: InternalDBAddress - DB IP address used by other Pods
431431
in the cluster
432432
type: string
433+
internalDbAddressRbacFullAccess:
434+
description: |-
435+
InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
436+
requires full access to the SB db, like e.g. Northd. This is used only
437+
when OVN RBAC for ovn-controllers is used (TLS enabled)
438+
type: string
433439
lastAppliedTopology:
434440
description: LastAppliedTopology - the last applied Topology
435441
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
@@ -170,6 +170,11 @@ type OVNDBClusterStatus struct {
170170
// InternalDBAddress - DB IP address used by other Pods in the cluster
171171
InternalDBAddress string `json:"internalDbAddress,omitempty"`
172172

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

@@ -238,6 +243,23 @@ func (instance OVNDBCluster) GetInternalEndpoint() (string, error) {
238243
return instance.Status.InternalDBAddress, nil
239244
}
240245

246+
// GetInternalEndpointRbacFullAccess - return the DNS name that openshift coreDNS can resolve
247+
func (instance OVNDBCluster) GetInternalEndpointRbacFullAccess() (string, error) {
248+
if !instance.Spec.TLS.Enabled() {
249+
// if TLS is disabled, this is the same as internalDbAddress
250+
return instance.GetInternalEndpoint()
251+
}
252+
if instance.Spec.DBType != SBDBType {
253+
// if DBType is not SB, this is the same as internalDbAddress
254+
return instance.GetInternalEndpoint()
255+
}
256+
257+
if instance.Status.InternalDBAddressRbacFullAccess == "" {
258+
return "", fmt.Errorf("internal DBEndpoint not ready yet for %s", instance.Spec.DBType)
259+
}
260+
return instance.Status.InternalDBAddressRbacFullAccess, nil
261+
}
262+
241263
// GetExternalEndpoint - return the DNS that openstack dnsmasq can resolve
242264
func (instance OVNDBCluster) GetExternalEndpoint() (string, error) {
243265
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
@@ -430,6 +430,12 @@ spec:
430430
description: InternalDBAddress - DB IP address used by other Pods
431431
in the cluster
432432
type: string
433+
internalDbAddressRbacFullAccess:
434+
description: |-
435+
InternalDBAddressRbacFullAccess - DB IP address used by other Pods which
436+
requires full access to the SB db, like e.g. Northd. This is used only
437+
when OVN RBAC for ovn-controllers is used (TLS enabled)
438+
type: string
433439
lastAppliedTopology:
434440
description: LastAppliedTopology - the last applied Topology
435441
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: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
755755
instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
756756
instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage)
757757
internalDbAddress := []string{}
758+
internalDbAddressRbacFullAccess := []string{}
758759
var svcPort int32
759760
scheme := "tcp"
760761
if instance.Spec.TLS.Enabled() {
@@ -770,6 +771,12 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
770771
// TODO: Watch operator.openshift.io resource once cluster domain is customizable
771772
clusterDomain := clusterdns.GetDNSClusterDomain()
772773
internalDbAddress = append(internalDbAddress, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, clusterDomain, svcPort))
774+
775+
// if TLS is enabled and DBType is SB, RBAC for ovn-controller is used, so additionally
776+
// set the internalDbAddressRbacFullAccess has to be set
777+
if instance.Spec.TLS.Enabled() && instance.Spec.DBType == ovnv1.SBDBType {
778+
internalDbAddressRbacFullAccess = append(internalDbAddressRbacFullAccess, fmt.Sprintf("%s:%s.%s.svc.%s:%d", scheme, svc.Name, svc.Namespace, clusterDomain, ovndbcluster.DbPortSBRBACFullAccess))
779+
}
773780
}
774781

775782
// Note setting this to the singular headless service address (e.g ssl:ovsdbserver-sb...) "works" but will not
@@ -779,6 +786,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
779786

780787
// Set DB Address
781788
instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",")
789+
instance.Status.InternalDBAddressRbacFullAccess = strings.Join(internalDbAddressRbacFullAccess, ",")
782790
if instance.Spec.DBType == ovnv1.SBDBType && (instance.Spec.NetworkAttachment != "" || instance.Spec.Override.Service != nil) {
783791
// This config map will populate the sb db address to edpm, can't use the nb
784792
// 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)