From 239911450a8b0deeff02b0e216f259441bb2a731 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 1 Apr 2026 15:41:06 +0200 Subject: [PATCH 1/3] Generate OVN SB DB certificate This cert will be then used to sign "per chassis" certificates used by the ovn-controllers to connect to the DB with OVN RBAC enabled [1]. [1] https://docs.ovn.org/en/latest/tutorials/ovn-rbac.html Related: #OSPRH-1922 Assisted-by: claude-opus-4.6 Signed-off-by: Slawek Kaplonski --- cmd/main.go | 2 + config/rbac/role.yaml | 21 ++ go.mod | 5 +- go.sum | 18 +- internal/common/const.go | 9 + .../controller/ovndbcluster_controller.go | 18 +- internal/ovndbcluster/const.go | 10 + internal/ovndbcluster/pki.go | 109 ++++++++ internal/ovndbcluster/service.go | 7 + internal/ovndbcluster/statefulset.go | 18 ++ templates/ovndbcluster/bin/setup.sh | 43 ++- .../ovndbcluster/config/runtime-config.sh | 14 +- .../ovndbcluster_controller_test.go | 256 ++++++++++++++++++ test/functional/suite_test.go | 16 ++ test/kuttl/tests/ovn_tls/01-assert.yaml | 13 + .../kuttl/tests/ovn_tls_enable/02-assert.yaml | 10 + 16 files changed, 548 insertions(+), 21 deletions(-) create mode 100644 internal/ovndbcluster/pki.go diff --git a/cmd/main.go b/cmd/main.go index b17be762..7d6e6bb8 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -46,6 +46,7 @@ import ( // +kubebuilder:scaffold:imports + certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" @@ -62,6 +63,7 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(certmgrv1.AddToScheme(scheme)) utilruntime.Must(ovnv1.AddToScheme(scheme)) utilruntime.Must(networkv1.AddToScheme(scheme)) utilruntime.Must(infranetworkv1.AddToScheme(scheme)) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4e6e5293..9c4d58bc 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -65,6 +65,18 @@ rules: - patch - update - watch +- apiGroups: + - cert-manager.io + resources: + - certificates + - issuers + verbs: + - create + - get + - list + - patch + - update + - watch - apiGroups: - k8s.cni.cncf.io resources: @@ -158,6 +170,15 @@ rules: - securitycontextconstraints verbs: - use +- apiGroups: + - security.openshift.io + resourceNames: + - privileged + - restricted-v2 + resources: + - securitycontextconstraints + verbs: + - use - apiGroups: - security.openshift.io resourceNames: diff --git a/go.mod b/go.mod index 3631fde0..e1268bda 100644 --- a/go.mod +++ b/go.mod @@ -3,12 +3,14 @@ module github.com/openstack-k8s-operators/ovn-operator go 1.24.4 require ( + github.com/cert-manager/cert-manager v1.16.5 github.com/go-logr/logr v1.4.3 github.com/google/uuid v1.6.0 github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.7.7 github.com/onsi/ginkgo/v2 v2.28.1 github.com/onsi/gomega v1.39.1 github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260416122644-5476763a36b6 + github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.6.1-0.20251122131503-b76943960b6c github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260417092244-81c71b39e981 github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260417092244-81c71b39e981 github.com/openstack-k8s-operators/ovn-operator/api v0.0.0-20230418071801-b5843d9e05fb @@ -65,7 +67,7 @@ require ( github.com/prometheus/procfs v0.16.1 // indirect github.com/spf13/cobra v1.9.1 // indirect github.com/spf13/pflag v1.0.7 // indirect - github.com/stoewer/go-strcase v1.2.0 // indirect + github.com/stoewer/go-strcase v1.3.0 // indirect github.com/x448/float16 v0.8.4 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0 // indirect @@ -103,6 +105,7 @@ require ( k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250902184714-7fc278399c7f // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect + sigs.k8s.io/gateway-api v1.2.1 // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.7.0 // indirect diff --git a/go.sum b/go.sum index 8cd351a5..aa4e5f78 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,8 @@ github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= +github.com/cert-manager/cert-manager v1.16.5 h1:XIhKoS4zQV9RHXAkqQW0NLivvoxAnWzbPsy9BG6cPVc= +github.com/cert-manager/cert-manager v1.16.5/go.mod h1:0DwmIGjMOreiP7/6gAqnjaBRJ+yHCfZ5DP7NNqKV+tY= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -118,6 +120,8 @@ github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyU github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo= github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260416122644-5476763a36b6 h1:117Gu9HCSu2tAp579WnCJ9QtnslH2qnPB8UFvn8ZpqE= github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260416122644-5476763a36b6/go.mod h1:i7l8cihvFktd/LSuyvL2z6OcwauarQGoVhDMePL4VyI= +github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.6.1-0.20251122131503-b76943960b6c h1:cPnMoKY5QkPF3ynahGs2+epjYVBb59vqt+uO6RHGFHE= +github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.6.1-0.20251122131503-b76943960b6c/go.mod h1:3zDlaWh4PKwFAhYM6zcKe+bAnCggnSB94v4unP4snUM= github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260417092244-81c71b39e981 h1:v1viH0gmNb+AXMg/0GxDcj8VUTdjVLotfOIGrNyMxHk= github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260417092244-81c71b39e981/go.mod h1:I/VBXZLdjk8DUGsEbB+Ha72JBFYYntP7Pm2FpEto9K8= github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260417092244-81c71b39e981 h1:KAQ8T+Ri3JWgsyK1D6QybScMh6fpkYUUA+0ntnOiAl4= @@ -143,11 +147,15 @@ github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wx github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.7 h1:vN6T9TfwStFPFM5XzjsvmzZkLuaLX+HS+0SeFLRgU6M= github.com/spf13/pflag v1.0.7/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/stoewer/go-strcase v1.2.0 h1:Z2iHWqGXH00XYgqDmNgQbIBxf3wrNq0F3feEy0ainaU= -github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= +github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= +github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= @@ -254,9 +262,9 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSP gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.31.14 h1:xYn/S/WFJsksI7dk/5uBRd3Umm/D8W5g7sRnd4csotA= @@ -281,6 +289,8 @@ sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 h1:2770sDpzrjjsA sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= sigs.k8s.io/controller-runtime v0.19.7 h1:DLABZfMr20A+AwCZOHhcbcu+TqBXnJZaVBri9K3EO48= sigs.k8s.io/controller-runtime v0.19.7/go.mod h1:iRmWllt8IlaLjvTTDLhRBXIEtkCK6hwVBJJsYS9Ajf4= +sigs.k8s.io/gateway-api v1.2.1 h1:fZZ/+RyRb+Y5tGkwxFKuYuSRQHu9dZtbjenblleOLHM= +sigs.k8s.io/gateway-api v1.2.1/go.mod h1:EpNfEXNjiYfUJypf0eZ0P5iXA9ekSGWaS1WgPaM42X0= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 h1:gBQPwqORJ8d8/YNZWEjoZs7npUVDpVXUUOFfW6CgAqE= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg= sigs.k8s.io/randfill v0.0.0-20250304075658-069ef1bbf016/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY= diff --git a/internal/common/const.go b/internal/common/const.go index 1158bac4..afc09602 100644 --- a/internal/common/const.go +++ b/internal/common/const.go @@ -14,6 +14,15 @@ const ( // OVNMetricsKeyPath is the path to the metrics private key file OVNMetricsKeyPath string = "/etc/pki/tls/private/ovnmetrics.key" + // OVNRbacPkiCaMountPath is the mount path for the OVN RBAC PKI CA secret volume + OVNRbacPkiCaMountPath string = "/etc/pki/ovn-rbac-ca" + // OVNRbacPkiCaCertPath is the path where the OVN RBAC PKI CA certificate + // is mounted inside the container + OVNRbacPkiCaCertPath string = OVNRbacPkiCaMountPath + "/tls.crt" + // OVNRbacPkiCaKeyPath is the path where the OVN RBAC PKI CA private key + // is mounted inside the container + OVNRbacPkiCaKeyPath string = OVNRbacPkiCaMountPath + "/tls.key" + // MetricsPort is the port used for metrics MetricsPort int32 = 1981 ) diff --git a/internal/controller/ovndbcluster_controller.go b/internal/controller/ovndbcluster_controller.go index 69f6ce8b..f302702e 100644 --- a/internal/controller/ovndbcluster_controller.go +++ b/internal/controller/ovndbcluster_controller.go @@ -101,13 +101,15 @@ func (r *OVNDBClusterReconciler) GetLogger(ctx context.Context) logr.Logger { //+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list; //+kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch //+kubebuilder:rbac:groups=network.openstack.org,resources=dnsdata,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=cert-manager.io,resources=issuers,verbs=get;list;watch;create;update;patch +//+kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;patch // service account, role, rolebinding // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;update;patch // service account permissions that are needed to grant permission to the above -// +kubebuilder:rbac:groups="security.openshift.io",resourceNames=restricted-v2,resources=securitycontextconstraints,verbs=use +// +kubebuilder:rbac:groups="security.openshift.io",resourceNames=restricted-v2;privileged,resources=securitycontextconstraints,verbs=use // +kubebuilder:rbac:groups="",resources=pods,verbs=create;delete;get;list;patch;update;watch // +kubebuilder:rbac:groups=topology.openstack.org,resources=topologies,verbs=get;list;watch;update @@ -634,6 +636,19 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance * return ctrl.Result{}, fmt.Errorf("waiting for Topology requirements: %w", err) } + // When TLS is enabled on the SB database, ensure the OVN RBAC PKI CA + // cert-manager resources exist (self-signed issuer, CA certificate, CA issuer) + // BEFORE the StatefulSet is created. The StatefulSet mounts the CA secret, + // so the secret must exist before pods can start. + if instance.Spec.TLS.Enabled() && instance.Spec.DBType == ovnv1.SBDBType { + ctrlResult, err := ovndbcluster.EnsureRbacPkiCA(ctx, helper, instance.Namespace, serviceLabels) + if err != nil { + return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + } + stsSpec, err := ovndbcluster.StatefulSet(instance, inputHash, serviceLabels, serviceAnnotations, topology) if err != nil { @@ -1287,6 +1302,7 @@ func (r *OVNDBClusterReconciler) generateServiceConfigMaps( templateParameters["OVNDB_CERT_PATH"] = ovn_common.OVNDbCertPath templateParameters["OVNDB_KEY_PATH"] = ovn_common.OVNDbKeyPath templateParameters["OVNDB_CACERT_PATH"] = ovn_common.OVNDbCaCertPath + templateParameters["OVN_RBAC_CACERT_PATH"] = ovn_common.OVNRbacPkiCaCertPath templateParameters["OVN_METRICS_CERT_PATH"] = ovn_common.OVNMetricsCertPath templateParameters["OVN_METRICS_KEY_PATH"] = ovn_common.OVNMetricsKeyPath templateParameters["OVN_RUNDIR"] = "/etc/ovn" diff --git a/internal/ovndbcluster/const.go b/internal/ovndbcluster/const.go index 99aaaad7..85f5c834 100644 --- a/internal/ovndbcluster/const.go +++ b/internal/ovndbcluster/const.go @@ -9,6 +9,16 @@ 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 + + // OVNRbacPkiCaSecret is the name of the K8s Secret that stores the OVN RBAC + // PKI CA certificate and key, used for signing per-node ovn-controller + // certificates when RBAC is enabled (TLS enabled on SB). + OVNRbacPkiCaSecret = "ovn-rbac-pki-ca" //nolint:gosec ) diff --git a/internal/ovndbcluster/pki.go b/internal/ovndbcluster/pki.go new file mode 100644 index 00000000..ac7cf327 --- /dev/null +++ b/internal/ovndbcluster/pki.go @@ -0,0 +1,109 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ovndbcluster + +import ( + "context" + "fmt" + "time" + + certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmgrmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/openstack-k8s-operators/lib-common/modules/certmanager" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" +) + +const ( + // selfSignedIssuerName is the name of the self-signed issuer used to + // bootstrap the OVN RBAC PKI CA. + selfSignedIssuerName = "ovn-rbac-selfsigned-issuer" + // rbacCAIssuerName is the name of the CA issuer that uses the generated + // CA certificate to issue per-node ovn-controller certificates. + rbacCAIssuerName = "ovn-rbac-ca-issuer" +) + +// EnsureRbacPkiCA creates the cert-manager resources needed for OVN RBAC PKI: +// a self-signed Issuer, a CA Certificate, and a CA Issuer that can then be +// used to sign per-node ovn-controller certificates. This follows the same +// pattern as the openstack-operator's createRootCACertAndIssuer. +func EnsureRbacPkiCA( + ctx context.Context, + h *helper.Helper, + namespace string, + labels map[string]string, +) (ctrl.Result, error) { + // Step 1: Create self-signed issuer to bootstrap the CA + selfSignedIssuerReq := certmanager.SelfSignedIssuer( + selfSignedIssuerName, + namespace, + labels, + ) + selfSignedIssuer := certmanager.NewIssuer(selfSignedIssuerReq, time.Duration(5)*time.Second) + ctrlResult, err := selfSignedIssuer.CreateOrPatch(ctx, h) + if err != nil { + return ctrlResult, fmt.Errorf("failed to create self-signed issuer for OVN RBAC PKI: %w", err) + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + + // Step 2: Create CA certificate signed by the self-signed issuer + caCertReq := certmanager.Cert( + OVNRbacPkiCaSecret, + namespace, + labels, + certmgrv1.CertificateSpec{ + IsCA: true, + CommonName: "OVN RBAC CA", + SecretName: OVNRbacPkiCaSecret, + PrivateKey: &certmgrv1.CertificatePrivateKey{ + Algorithm: certmgrv1.RSAKeyAlgorithm, + Size: 2048, + }, + IssuerRef: certmgrmetav1.ObjectReference{ + Name: selfSignedIssuerReq.Name, + Kind: selfSignedIssuerReq.Kind, + Group: selfSignedIssuerReq.GroupVersionKind().Group, + }, + Duration: &metav1.Duration{ + Duration: time.Hour * 24 * 365 * 10, // 10 years + }, + }, + ) + caCert := certmanager.NewCertificate(caCertReq, time.Duration(5)*time.Second) + ctrlResult, _, err = caCert.CreateOrPatch(ctx, h, nil) + if err != nil { + return ctrlResult, fmt.Errorf("failed to create OVN RBAC PKI CA certificate: %w", err) + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + + // Step 3: Create CA issuer that uses the generated CA certificate + caIssuerReq := certmanager.CAIssuer( + rbacCAIssuerName, + namespace, + labels, + nil, // annotations + OVNRbacPkiCaSecret, + ) + caIssuer := certmanager.NewIssuer(caIssuerReq, time.Duration(5)*time.Second) + ctrlResult, err = caIssuer.CreateOrPatch(ctx, h) + if err != nil { + return ctrlResult, fmt.Errorf("failed to create OVN RBAC PKI CA issuer: %w", err) + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + + return ctrl.Result{}, nil +} diff --git a/internal/ovndbcluster/service.go b/internal/ovndbcluster/service.go index c52b73d7..a2988dde 100644 --- 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/internal/ovndbcluster/statefulset.go b/internal/ovndbcluster/statefulset.go index bd0c846f..342d84e0 100644 --- a/internal/ovndbcluster/statefulset.go +++ b/internal/ovndbcluster/statefulset.go @@ -133,6 +133,24 @@ func StatefulSet( } volumes = append(volumes, svc.CreateVolume(serviceName)) volumeMounts = append(volumeMounts, svc.CreateVolumeMounts(serviceName)...) + + // For SB database with RBAC, mount the RBAC PKI CA cert so it can be + // used in set-ssl to verify ovn-controller client certificates. + if instance.Spec.DBType == ovnv1.SBDBType { + volumes = append(volumes, corev1.Volume{ + Name: "ovn-rbac-pki-ca", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: OVNRbacPkiCaSecret, + }, + }, + }) + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "ovn-rbac-pki-ca", + MountPath: ovn_common.OVNRbacPkiCaMountPath, + ReadOnly: true, + }) + } } // NOTE(ihar) ovndb pods leave the raft cluster on delete; it's important diff --git a/templates/ovndbcluster/bin/setup.sh b/templates/ovndbcluster/bin/setup.sh index 364f053b..8cd3beaf 100755 --- a/templates/ovndbcluster/bin/setup.sh +++ b/templates/ovndbcluster/bin/setup.sh @@ -54,9 +54,17 @@ set "$@" --db-${DB_TYPE}-cluster-local-port=${RAFT_PORT} set "$@" --db-${DB_TYPE}-addr=${DB_ADDR} set "$@" --db-${DB_TYPE}-port=${DB_PORT} {{- if .TLS }} +SSL_CA_CERT={{.OVNDB_CACERT_PATH}} +if [[ "${DB_TYPE}" == "sb" ]]; then + # For SB, create a combined CA bundle containing both the infrastructure CA + # (for verifying raft peer certs) and the RBAC PKI CA (for verifying + # ovn-controller client certs). + SSL_CA_CERT="/tmp/ovn-sb-combined-ca.crt" + cat {{.OVNDB_CACERT_PATH}} {{.OVN_RBAC_CACERT_PATH}} > ${SSL_CA_CERT} +fi set "$@" --ovn-${DB_TYPE}-db-ssl-key={{.OVNDB_KEY_PATH}} set "$@" --ovn-${DB_TYPE}-db-ssl-cert={{.OVNDB_CERT_PATH}} -set "$@" --ovn-${DB_TYPE}-db-ssl-ca-cert={{.OVNDB_CACERT_PATH}} +set "$@" --ovn-${DB_TYPE}-db-ssl-ca-cert=${SSL_CA_CERT} set "$@" --db-${DB_TYPE}-cluster-local-proto=ssl set "$@" --db-${DB_TYPE}-cluster-remote-proto=ssl set "$@" --db-${DB_TYPE}-create-insecure-remote=no @@ -122,15 +130,26 @@ if [[ "$(hostname)" == "{{ .SERVICE_NAME }}-0" ]]; then export OVN_${DB_TYPE^^}_DAEMON=$(${CTLCMD} --pidfile --detach) {{- if .TLS }} - ${CTLCMD} set-ssl {{.OVNDB_KEY_PATH}} {{.OVNDB_CERT_PATH}} {{.OVNDB_CACERT_PATH}} + ${CTLCMD} set-ssl {{.OVNDB_KEY_PATH}} {{.OVNDB_CERT_PATH}} ${SSL_CA_CERT} + 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 }} - CURRENT_PROBE="$(${CTLCMD} get connection . inactivity_probe || echo [])" - if [ "$CURRENT_PROBE" = "[]" ]; then - CURRENT_PROBE=60000 - fi + # 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. # @@ -144,8 +163,14 @@ 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)" != "${CURRENT_PROBE}" ]; do - ${CTLCMD} --inactivity-probe="${CURRENT_PROBE}" set-connection ${DB_SCHEME}:${DB_PORT}:${DB_ADDR} + for connection_id in $(${CTLCMD} -f csv --no-headings --columns=_uuid list connection); do + CURRENT_PROBE="$(${CTLCMD} get connection $connection_id inactivity_probe || echo [])" + if [ "$CURRENT_PROBE" = "[]" ]; then + CURRENT_PROBE=60000 + fi + while [ "$(${CTLCMD} get connection $connection_id inactivity_probe)" != "${CURRENT_PROBE}" ]; do + ${CTLCMD} set connection $connection_id inactivity_probe=${CURRENT_PROBE} + done done ${CTLCMD} list connection diff --git a/templates/ovndbcluster/config/runtime-config.sh b/templates/ovndbcluster/config/runtime-config.sh index 3c8726f9..de633089 100755 --- a/templates/ovndbcluster/config/runtime-config.sh +++ b/templates/ovndbcluster/config/runtime-config.sh @@ -75,12 +75,14 @@ if echo "$CONFIG_FLAGS" | grep -q "INACTIVITY_PROBE"; then echo "Configuring inactivity probe to ${INACTIVITY_PROBE}ms on pod-0..." # Simple approach - connect directly to running database (OVN_RUNDIR already set) - if ovn-${DB_TYPE}ctl --no-leader-only set connection . inactivity_probe="$INACTIVITY_PROBE"; then - echo "✓ Successfully configured inactivity probe" - else - echo "✗ Failed to configure inactivity probe" - exit 1 - fi + for connection_id in $(ovn-${DB_TYPE}ctl --no-leader-only -f csv --no-headings --columns=_uuid list connection); do + if ovn-${DB_TYPE}ctl --no-leader-only set connection $connection_id inactivity_probe="$INACTIVITY_PROBE"; then + echo "✓ Successfully configured inactivity probe for connection ${connection_id}" + else + echo "✗ Failed to configure inactivity probe for connection ${connection_id}" + exit 1 + fi + done else echo "⚠ Skipping inactivity probe configuration (not pod-0)" fi diff --git a/test/functional/ovndbcluster_controller_test.go b/test/functional/ovndbcluster_controller_test.go index 96049987..2718b40b 100644 --- a/test/functional/ovndbcluster_controller_test.go +++ b/test/functional/ovndbcluster_controller_test.go @@ -27,15 +27,18 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/service" . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" + certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1" ovn_common "github.com/openstack-k8s-operators/ovn-operator/internal/common" + "github.com/openstack-k8s-operators/ovn-operator/internal/ovndbcluster" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -1338,6 +1341,34 @@ var _ = Describe("OVNDBCluster controller", func() { th.AssertVolumeMountExists("ovsdbserver-sb-tls-certs", "tls.crt", svcC.VolumeMounts) th.AssertVolumeMountExists("ovsdbserver-sb-tls-certs", "ca.crt", svcC.VolumeMounts) + // check RBAC PKI CA volume and mount for SB with TLS + th.AssertVolumeExists("ovn-rbac-pki-ca", ss.Spec.Template.Spec.Volumes) + th.AssertVolumeMountExists("ovn-rbac-pki-ca", "", svcC.VolumeMounts) + + // Verify the RBAC PKI CA volume references the correct secret + var rbacVolume *corev1.Volume + for i, v := range ss.Spec.Template.Spec.Volumes { + if v.Name == "ovn-rbac-pki-ca" { + rbacVolume = &ss.Spec.Template.Spec.Volumes[i] + break + } + } + Expect(rbacVolume).NotTo(BeNil()) + Expect(rbacVolume.VolumeSource.Secret).NotTo(BeNil()) + Expect(rbacVolume.VolumeSource.Secret.SecretName).To(Equal(ovndbcluster.OVNRbacPkiCaSecret)) + + // Verify the RBAC PKI CA volume mount path + var rbacMount *corev1.VolumeMount + for i, vm := range svcC.VolumeMounts { + if vm.Name == "ovn-rbac-pki-ca" { + rbacMount = &svcC.VolumeMounts[i] + break + } + } + Expect(rbacMount).NotTo(BeNil()) + Expect(rbacMount.MountPath).To(Equal(ovn_common.OVNRbacPkiCaMountPath)) + Expect(rbacMount.ReadOnly).To(BeTrue()) + // check DB url schema Eventually(func(g Gomega) { OVNDBCluster := GetOVNDBCluster(OVNDBClusterName) @@ -1363,6 +1394,10 @@ var _ = Describe("OVNDBCluster controller", func() { ContainSubstring("-cluster-remote-proto=ssl"), )) + // check SB setup script creates combined CA bundle for RBAC + Expect(th.GetConfigMap(scriptsCM).Data["setup.sh"]).Should( + ContainSubstring(ovn_common.OVNRbacPkiCaCertPath)) + th.ExpectCondition( OVNDBClusterName, ConditionGetterFunc(OVNDBClusterConditionGetter), @@ -1371,6 +1406,104 @@ var _ = Describe("OVNDBCluster controller", func() { ) }) + It("creates cert-manager resources for RBAC PKI CA on SB with TLS", func() { + DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{ + Name: CABundleSecretName, + Namespace: namespace, + })) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(types.NamespacedName{ + Name: OvnDbCertSecretName, + Namespace: namespace, + })) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods(statefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + th.ExpectCondition( + OVNDBClusterName, + ConditionGetterFunc(OVNDBClusterConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + + // Verify self-signed issuer is created + selfSignedIssuer := &certmgrv1.Issuer{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "ovn-rbac-selfsigned-issuer", + Namespace: namespace, + }, selfSignedIssuer)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + Expect(selfSignedIssuer.Spec.SelfSigned).NotTo(BeNil()) + + // Verify CA certificate is created + caCert := &certmgrv1.Certificate{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: ovndbcluster.OVNRbacPkiCaSecret, + Namespace: namespace, + }, caCert)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + Expect(caCert.Spec.IsCA).To(BeTrue()) + Expect(caCert.Spec.CommonName).To(Equal("OVN RBAC CA")) + Expect(caCert.Spec.SecretName).To(Equal(ovndbcluster.OVNRbacPkiCaSecret)) + Expect(caCert.Spec.IssuerRef.Name).To(Equal("ovn-rbac-selfsigned-issuer")) + + // Verify CA issuer is created + caIssuer := &certmgrv1.Issuer{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "ovn-rbac-ca-issuer", + Namespace: namespace, + }, caIssuer)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + Expect(caIssuer.Spec.CA).NotTo(BeNil()) + Expect(caIssuer.Spec.CA.SecretName).To(Equal(ovndbcluster.OVNRbacPkiCaSecret)) + }) + + It("creates services with RBAC full-access port for SB with TLS", func() { + DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{ + Name: CABundleSecretName, + Namespace: namespace, + })) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(types.NamespacedName{ + Name: OvnDbCertSecretName, + Namespace: namespace, + })) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-sb", + } + th.SimulateStatefulSetReplicaReadyWithPods(statefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + // Verify services have the RBAC full-access port (16642) + Eventually(func(g Gomega) { + serviceList := GetServicesListWithLabel(namespace, map[string]string{"type": "cluster"}) + g.Expect(serviceList.Items).ToNot(BeEmpty()) + for _, svc := range serviceList.Items { + var hasRbacPort bool + for _, port := range svc.Spec.Ports { + if port.Port == ovndbcluster.DbPortSBRBACFullAccess { + hasRbacPort = true + g.Expect(port.Name).To(ContainSubstring("rbac-full-access")) + break + } + } + g.Expect(hasRbacPort).To(BeTrue(), + fmt.Sprintf("Service %s should have RBAC full-access port %d", + svc.Name, ovndbcluster.DbPortSBRBACFullAccess)) + } + }, timeout, interval).Should(Succeed()) + }) + It("reconfigures the pods when CA bundle changes", func() { DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{ Name: CABundleSecretName, @@ -1465,6 +1598,129 @@ var _ = Describe("OVNDBCluster controller", func() { }) + When("OVNDBCluster NB is created with TLS", func() { + var OVNDBClusterName types.NamespacedName + BeforeEach(func() { + spec := GetTLSOVNDBClusterSpec() + spec.DBType = ovnv1.NBDBType + instance := CreateOVNDBCluster(namespace, spec) + OVNDBClusterName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()} + DeferCleanup(th.DeleteInstance, instance) + }) + + It("does not create RBAC PKI CA volume in StatefulSet for NB", func() { + DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{ + Name: CABundleSecretName, + Namespace: namespace, + })) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(types.NamespacedName{ + Name: OvnDbCertSecretName, + Namespace: namespace, + })) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-nb", + } + th.SimulateStatefulSetReplicaReadyWithPods(statefulSetName, map[string][]string{}) + + th.ExpectCondition( + OVNDBClusterName, + ConditionGetterFunc(OVNDBClusterConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + + ss := th.GetStatefulSet(statefulSetName) + + // Verify TLS volumes exist + th.AssertVolumeExists(CABundleSecretName, ss.Spec.Template.Spec.Volumes) + th.AssertVolumeExists("ovsdbserver-nb-tls-certs", ss.Spec.Template.Spec.Volumes) + + // Verify RBAC PKI CA volume does NOT exist for NB + for _, v := range ss.Spec.Template.Spec.Volumes { + Expect(v.Name).NotTo(Equal("ovn-rbac-pki-ca"), + "NB StatefulSet should not have ovn-rbac-pki-ca volume") + } + + svcC := ss.Spec.Template.Spec.Containers[0] + for _, vm := range svcC.VolumeMounts { + Expect(vm.Name).NotTo(Equal("ovn-rbac-pki-ca"), + "NB StatefulSet should not have ovn-rbac-pki-ca volume mount") + } + }) + + It("does not create cert-manager RBAC PKI CA resources for NB", func() { + DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{ + Name: CABundleSecretName, + Namespace: namespace, + })) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(types.NamespacedName{ + Name: OvnDbCertSecretName, + Namespace: namespace, + })) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-nb", + } + th.SimulateStatefulSetReplicaReadyWithPods(statefulSetName, map[string][]string{}) + + th.ExpectCondition( + OVNDBClusterName, + ConditionGetterFunc(OVNDBClusterConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + + // Verify cert-manager RBAC resources are NOT created for NB + issuerList := &certmgrv1.IssuerList{} + Expect(k8sClient.List(ctx, issuerList, client.InNamespace(namespace))).Should(Succeed()) + for _, issuer := range issuerList.Items { + Expect(issuer.Name).NotTo(Equal("ovn-rbac-selfsigned-issuer"), + "NB should not create RBAC self-signed issuer") + Expect(issuer.Name).NotTo(Equal("ovn-rbac-ca-issuer"), + "NB should not create RBAC CA issuer") + } + + certList := &certmgrv1.CertificateList{} + Expect(k8sClient.List(ctx, certList, client.InNamespace(namespace))).Should(Succeed()) + for _, cert := range certList.Items { + Expect(cert.Name).NotTo(Equal(ovndbcluster.OVNRbacPkiCaSecret), + "NB should not create RBAC PKI CA certificate") + } + }) + + It("does not add RBAC full-access port to services for NB", func() { + DeferCleanup(k8sClient.Delete, ctx, th.CreateCABundleSecret(types.NamespacedName{ + Name: CABundleSecretName, + Namespace: namespace, + })) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(types.NamespacedName{ + Name: OvnDbCertSecretName, + Namespace: namespace, + })) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: "ovsdbserver-nb", + } + th.SimulateStatefulSetReplicaReadyWithPods(statefulSetName, map[string][]string{}) + + Eventually(func(g Gomega) { + serviceList := GetServicesListWithLabel(namespace, map[string]string{"type": "cluster"}) + g.Expect(serviceList.Items).ToNot(BeEmpty()) + for _, svc := range serviceList.Items { + for _, port := range svc.Spec.Ports { + g.Expect(port.Port).NotTo(Equal(ovndbcluster.DbPortSBRBACFullAccess), + fmt.Sprintf("NB service %s should not have RBAC full-access port", svc.Name)) + } + } + }, timeout, interval).Should(Succeed()) + }) + + }) + When("OVNDB is created with topologyref", func() { var OVNDBClusterName types.NamespacedName var statefulSetName types.NamespacedName diff --git a/test/functional/suite_test.go b/test/functional/suite_test.go index 31127265..0ddf3d3f 100644 --- a/test/functional/suite_test.go +++ b/test/functional/suite_test.go @@ -42,6 +42,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1" appsv1 "k8s.io/api/apps/v1" @@ -78,6 +79,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) { @@ -105,6 +109,14 @@ var _ = BeforeSuite(func() { "github.com/openstack-k8s-operators/infra-operator/apis", "../../go.mod", "bases/topology.openstack.org_topologies.yaml") Expect(err).ShouldNot(HaveOccurred()) + certManagerCertsCRD, err := test.GetOpenShiftCRDDir( + "cert-manager/v1/certificates.cert-manager.io-crd.yaml", "../../go.mod") + Expect(err).ShouldNot(HaveOccurred()) + + certManagerIssuersCRD, err := test.GetOpenShiftCRDDir( + "cert-manager/v1/issuers.cert-manager.io-crd.yaml", "../../go.mod") + Expect(err).ShouldNot(HaveOccurred()) + By("bootstrapping test environment") testEnv = &envtest.Environment{ // Increase this to 60 or 120 seconds for the single-core run @@ -120,6 +132,8 @@ var _ = BeforeSuite(func() { networkv1CRD, infranetworkv1CRD, infratopologyv1CRD, + certManagerCertsCRD, + certManagerIssuersCRD, }, }, ErrorIfCRDPathMissing: true, @@ -159,6 +173,8 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) err = topologyv1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = certmgrv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:scheme logger = ctrl.Log.WithName("---Test---") diff --git a/test/kuttl/tests/ovn_tls/01-assert.yaml b/test/kuttl/tests/ovn_tls/01-assert.yaml index ed35ad7a..520e46b9 100644 --- a/test/kuttl/tests/ovn_tls/01-assert.yaml +++ b/test/kuttl/tests/ovn_tls/01-assert.yaml @@ -90,3 +90,16 @@ commands: if [[ -n "$matches" ]]; then exit 1 fi + # Verify cert-manager RBAC PKI CA resources are created for SB with TLS + - script: | + # Check self-signed issuer exists + oc get issuer -n $NAMESPACE ovn-rbac-selfsigned-issuer + - script: | + # Check CA certificate exists and is a CA + oc get certificate -n $NAMESPACE ovn-rbac-pki-ca -o jsonpath='{.spec.isCA}' | grep -q "true" + - script: | + # Check CA issuer exists and references the correct secret + oc get issuer -n $NAMESPACE ovn-rbac-ca-issuer -o jsonpath='{.spec.ca.secretName}' | grep -q "ovn-rbac-pki-ca" + # Verify SB service has RBAC full-access port (16642) + - script: | + oc get service -n $NAMESPACE ovsdbserver-sb-0 -o jsonpath='{.spec.ports[*].port}' | grep -q "16642" diff --git a/test/kuttl/tests/ovn_tls_enable/02-assert.yaml b/test/kuttl/tests/ovn_tls_enable/02-assert.yaml index 5c681280..ba17ce9e 100644 --- a/test/kuttl/tests/ovn_tls_enable/02-assert.yaml +++ b/test/kuttl/tests/ovn_tls_enable/02-assert.yaml @@ -37,3 +37,13 @@ commands: ../../common/scripts/check_cluster_status.sh nb 3 ssl - script: | ../../common/scripts/check_cluster_status.sh sb 3 ssl + # Verify cert-manager RBAC PKI CA resources are created for SB with TLS + - script: | + oc get issuer -n $NAMESPACE ovn-rbac-selfsigned-issuer + - script: | + oc get certificate -n $NAMESPACE ovn-rbac-pki-ca -o jsonpath='{.spec.isCA}' | grep -q "true" + - script: | + oc get issuer -n $NAMESPACE ovn-rbac-ca-issuer -o jsonpath='{.spec.ca.secretName}' | grep -q "ovn-rbac-pki-ca" + # Verify SB service has RBAC full-access port (16642) + - script: | + oc get service -n $NAMESPACE ovsdbserver-sb-0 -o jsonpath='{.spec.ports[*].port}' | grep -q "16642" From 40841a9ad3bbf28fd865e3f113620d4d187186f4 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 1 Apr 2026 15:50:00 +0200 Subject: [PATCH 2/3] Generate signed SSL Cert for ovn-controller node With this patch instead of using the same SSL certificate by each of the ovn-controller PODs in the environment, there is separate certificate generated, with unique CN name which match system-id set in that chassis and signed with certificate from SB. That way OVN RBAC can be used for the connections from ovn-controller PODs to the OVS SB database. Related: #1922 Assisted-by: claude-opus-4.6 Signed-off-by: Slawek Kaplonski --- internal/ovncontroller/configjob.go | 38 ++++++++++- internal/ovncontroller/const.go | 10 +++ internal/ovncontroller/daemonset.go | 20 +++++- templates/ovncontroller/bin/functions | 67 +++++++++++++++++++ .../ovncontroller/bin/init-ovsdb-server.sh | 11 ++- templates/ovncontroller/bin/init.sh | 12 ++++ .../ovncontroller_controller_test.go | 5 +- test/kuttl/tests/ovn_tls/01-assert.yaml | 24 +++++++ .../kuttl/tests/ovn_tls_enable/02-assert.yaml | 24 +++++++ 9 files changed, 202 insertions(+), 9 deletions(-) diff --git a/internal/ovncontroller/configjob.go b/internal/ovncontroller/configjob.go index 268eda5b..f287f0f3 100644 --- a/internal/ovncontroller/configjob.go +++ b/internal/ovncontroller/configjob.go @@ -19,6 +19,8 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/env" ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1" + ovn_common "github.com/openstack-k8s-operators/ovn-operator/internal/common" + ovndbcluster "github.com/openstack-k8s-operators/ovn-operator/internal/ovndbcluster" "sigs.k8s.io/controller-runtime/pkg/client" batchv1 "k8s.io/api/batch/v1" @@ -67,6 +69,38 @@ func ConfigJob( envVars["OVNLogLevel"] = env.SetValue(instance.Spec.OVNLogLevel) envVars["OVSLogLevel"] = env.SetValue(instance.Spec.OVSLogLevel) + // Prepare volumes and mounts for config job + volumes := GetOVNControllerVolumes(instance.Name, instance.Namespace, true) + volumeMounts := GetOVNControllerVolumeMounts(true) + + // When TLS is enabled, mount the RBAC PKI CA secret so the config job + // can generate and sign per-node ovn-controller certificates. + if instance.Spec.TLS.Enabled() { + volumes = append(volumes, corev1.Volume{ + Name: "ovn-rbac-pki-ca", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: ovndbcluster.OVNRbacPkiCaSecret, + }, + }, + }) + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "ovn-rbac-pki-ca", + MountPath: ovn_common.OVNRbacPkiCaMountPath, + ReadOnly: true, + }) + // Also mount etc-ovs to persist the generated certificates + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "etc-ovs", + MountPath: OVNControllerCertDir, + ReadOnly: false, + }) + + envVars["OVNControllerCertDir"] = env.SetValue(OVNControllerCertDir) + envVars["OVN_RBAC_CA_CERT"] = env.SetValue(ovn_common.OVNRbacPkiCaCertPath) + envVars["OVN_RBAC_CA_KEY"] = env.SetValue(ovn_common.OVNRbacPkiCaKeyPath) + } + for _, ovnPod := range ovnPods.Items { commands := []string{ "/usr/local/bin/container-scripts/init.sh", @@ -101,11 +135,11 @@ func ConfigJob( Privileged: &privileged, }, Env: env.MergeEnvs([]corev1.EnvVar{}, envVars), - VolumeMounts: GetOVNControllerVolumeMounts(true), + VolumeMounts: volumeMounts, Resources: instance.Spec.Resources, }, }, - Volumes: GetOVNControllerVolumes(instance.Name, instance.Namespace, true), + Volumes: volumes, NodeName: ovnPod.Spec.NodeName, // ^ NodeSelector not required }, diff --git a/internal/ovncontroller/const.go b/internal/ovncontroller/const.go index ef65079f..6267eb8e 100644 --- a/internal/ovncontroller/const.go +++ b/internal/ovncontroller/const.go @@ -1 +1,11 @@ package ovncontroller + +const ( + // OVNControllerCertDir is the directory where per-node ovn-controller + // RBAC certificates are stored (HostPath-backed) + OVNControllerCertDir string = "/etc/openvswitch" + // OVNControllerCertPath is the path to the per-node ovn-controller certificate + OVNControllerCertPath string = OVNControllerCertDir + "/ovn-controller-cert.pem" + // OVNControllerKeyPath is the path to the per-node ovn-controller private key + OVNControllerKeyPath string = OVNControllerCertDir + "/ovn-controller-privkey.pem" +) diff --git a/internal/ovncontroller/daemonset.go b/internal/ovncontroller/daemonset.go index d39c6518..9a8b34b8 100644 --- a/internal/ovncontroller/daemonset.go +++ b/internal/ovncontroller/daemonset.go @@ -59,9 +59,19 @@ func CreateOVNDaemonSet( mounts = append(mounts, instance.Spec.TLS.CreateVolumeMounts(nil)...) } + // When TLS is enabled, RBAC is used for the SB database connection. + // ovn-controller must use a per-node certificate (CN=node_name) generated + // by the config job and stored in /etc/openvswitch/ (HostPath). + // The CA cert for verifying the SB server remains the same. + mounts = append(mounts, corev1.VolumeMount{ + Name: "etc-ovs", + MountPath: OVNControllerCertDir, + ReadOnly: false, + }) + cmd = append(cmd, []string{ - fmt.Sprintf("--certificate=%s", ovn_common.OVNDbCertPath), - fmt.Sprintf("--private-key=%s", ovn_common.OVNDbKeyPath), + fmt.Sprintf("--certificate=%s", OVNControllerCertPath), + fmt.Sprintf("--private-key=%s", OVNControllerKeyPath), fmt.Sprintf("--ca-cert=%s", ovn_common.OVNDbCaCertPath), }...) } @@ -239,6 +249,10 @@ func CreateOVSDaemonSet( envVars := map[string]env.Setter{} envVars["CONFIG_HASH"] = env.SetValue(configHash) + initEnvVars := map[string]env.Setter{} + initEnvVars["CONFIG_HASH"] = env.SetValue(configHash) + initEnvVars["OVNHostName"] = env.DownwardAPI("spec.nodeName") + initContainers := []corev1.Container{ { Name: "ovsdb-server-init", @@ -252,7 +266,7 @@ func CreateOVSDaemonSet( RunAsUser: &runAsUser, Privileged: &privileged, }, - Env: env.MergeEnvs([]corev1.EnvVar{}, envVars), + Env: env.MergeEnvs([]corev1.EnvVar{}, initEnvVars), VolumeMounts: GetOVSDbVolumeMounts(), }, } diff --git a/templates/ovncontroller/bin/functions b/templates/ovncontroller/bin/functions index c13eb79a..a83b7ddb 100755 --- a/templates/ovncontroller/bin/functions +++ b/templates/ovncontroller/bin/functions @@ -22,6 +22,7 @@ OVNAvailabilityZones=${OVNAvailabilityZones:-""} EnableChassisAsGateway=${EnableChassisAsGateway:-true} PhysicalNetworks=${PhysicalNetworks:-""} OVNHostName=${OVNHostName:-""} +OVNControllerCertDir=${OVNControllerCertDir:-"/etc/openvswitch"} DB_FILE=/etc/openvswitch/conf.db ovs_dir=/var/lib/openvswitch @@ -152,3 +153,69 @@ function wait_for_db_creation { sleep 1 done } + +function hostname_to_uuid { + python3 -c "import uuid,sys; print(uuid.uuid5(uuid.NAMESPACE_DNS, sys.argv[1]))" "$1" +} + +# Generate per-node RBAC certificate signed by the OVN RBAC PKI CA. +# The certificate CN is set to the chassis system-id (UUID5 derived from +# hostname) which must match the OVN chassis system-id for RBAC ownership +# checks to work. +# Certificates are stored in /etc/openvswitch/ which is persisted via HostPath. +function generate_rbac_certificate { + local system_id="$1" + local cert_dir="${OVNControllerCertDir}" + local cert_file="${cert_dir}/ovn-controller-cert.pem" + local key_file="${cert_dir}/ovn-controller-privkey.pem" + local ca_cert="${OVN_RBAC_CA_CERT}" + local ca_key="${OVN_RBAC_CA_KEY}" + + if [ -z "${system_id}" ]; then + echo "ERROR: system_id is not provided, cannot generate RBAC certificate" + return 1 + fi + + # Check if CA cert and key are available + if [ ! -f "${ca_cert}" ] || [ ! -f "${ca_key}" ]; then + echo "OVN RBAC PKI CA not available, skipping certificate generation" + return 0 + fi + + # If certificate already exists and has the correct CN, skip regeneration + if [ -f "${cert_file}" ]; then + existing_cn=$(openssl x509 -in "${cert_file}" -noout -subject 2>/dev/null | sed -n 's/.*CN *= *\([^ ,]*\).*/\1/p') + if [ "${existing_cn}" = "${system_id}" ]; then + echo "RBAC certificate for ${system_id} already exists, skipping generation" + return 0 + fi + echo "RBAC certificate CN mismatch (${existing_cn} != ${system_id}), regenerating" + fi + + echo "Generating RBAC certificate for chassis ${system_id}" + + # Generate private key + openssl genrsa -out "${key_file}" 2048 + + # Generate CSR with CN=system_id (UUID5) + openssl req -new -key "${key_file}" \ + -out "${cert_dir}/ovn-controller.csr" \ + -subj "/CN=${system_id}" + + # Sign the certificate with the CA. + # Use -CAserial with a writable path since the CA secret mount is read-only. + openssl x509 -req \ + -in "${cert_dir}/ovn-controller.csr" \ + -CA "${ca_cert}" \ + -CAkey "${ca_key}" \ + -CAserial "${cert_dir}/ovn-rbac-ca.srl" \ + -CAcreateserial \ + -out "${cert_file}" \ + -days 3650 \ + -sha256 + + # Clean up CSR and serial file + rm -f "${cert_dir}/ovn-controller.csr" "${cert_dir}/ovn-rbac-ca.srl" + + echo "RBAC certificate generated successfully for ${system_id}" +} diff --git a/templates/ovncontroller/bin/init-ovsdb-server.sh b/templates/ovncontroller/bin/init-ovsdb-server.sh index 40ef21cb..a69316eb 100755 --- a/templates/ovncontroller/bin/init-ovsdb-server.sh +++ b/templates/ovncontroller/bin/init-ovsdb-server.sh @@ -29,8 +29,15 @@ if [ -f ${DB_FILE} ]; then ovsdb-tool compact ${DB_FILE} fi -# Initialize or upgrade database if needed -CTL_ARGS="--system-id=random --no-ovs-vswitchd" +# Initialize or upgrade database if needed. +# Use deterministic UUID5 (derived from hostname) as system-id for OVN RBAC. +# The system-id must match the certificate CN for RBAC ownership checks. +if [ -n "${OVNHostName}" ]; then + SYSTEM_ID=$(hostname_to_uuid "${OVNHostName}") + CTL_ARGS="--system-id=${SYSTEM_ID} --no-ovs-vswitchd" +else + CTL_ARGS="--system-id=random --no-ovs-vswitchd" +fi /usr/share/openvswitch/scripts/ovs-ctl start $CTL_ARGS /usr/share/openvswitch/scripts/ovs-ctl stop $CTL_ARGS diff --git a/templates/ovncontroller/bin/init.sh b/templates/ovncontroller/bin/init.sh index bec5945d..2fefb553 100755 --- a/templates/ovncontroller/bin/init.sh +++ b/templates/ovncontroller/bin/init.sh @@ -21,5 +21,17 @@ wait_for_ovsdb_server # From now on, we should exit immediatelly when any command exits with non-zero status set -ex +# Set deterministic UUID5 system-id derived from hostname for RBAC compatibility. +# The system-id must match the certificate CN for OVN RBAC ownership checks. +if [ -n "${OVNHostName}" ]; then + SYSTEM_ID=$(hostname_to_uuid "${OVNHostName}") + ovs-vsctl set open . external-ids:system-id=${SYSTEM_ID} +fi + configure_external_ids configure_physical_networks + +# Generate per-node RBAC certificate if the RBAC PKI CA is available +if [ -n "${OVN_RBAC_CA_CERT}" ] && [ -f "${OVN_RBAC_CA_CERT}" ]; then + generate_rbac_certificate "${SYSTEM_ID}" +fi diff --git a/test/functional/ovncontroller_controller_test.go b/test/functional/ovncontroller_controller_test.go index 5ff8c667..0ed97a68 100644 --- a/test/functional/ovncontroller_controller_test.go +++ b/test/functional/ovncontroller_controller_test.go @@ -31,6 +31,7 @@ import ( condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1" ovn_common "github.com/openstack-k8s-operators/ovn-operator/internal/common" + ovn_ovncontroller "github.com/openstack-k8s-operators/ovn-operator/internal/ovncontroller" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -1047,8 +1048,8 @@ var _ = Describe("OVNController controller", func() { // check cli args Expect(svcC.Command).To(And( - ContainElement(ContainSubstring(fmt.Sprintf("--private-key=%s", ovn_common.OVNDbKeyPath))), - ContainElement(ContainSubstring(fmt.Sprintf("--certificate=%s", ovn_common.OVNDbCertPath))), + ContainElement(ContainSubstring(fmt.Sprintf("--private-key=%s", ovn_ovncontroller.OVNControllerKeyPath))), + ContainElement(ContainSubstring(fmt.Sprintf("--certificate=%s", ovn_ovncontroller.OVNControllerCertPath))), ContainElement(ContainSubstring(fmt.Sprintf("--ca-cert=%s", ovn_common.OVNDbCaCertPath))), )) diff --git a/test/kuttl/tests/ovn_tls/01-assert.yaml b/test/kuttl/tests/ovn_tls/01-assert.yaml index 520e46b9..ea9afc75 100644 --- a/test/kuttl/tests/ovn_tls/01-assert.yaml +++ b/test/kuttl/tests/ovn_tls/01-assert.yaml @@ -103,3 +103,27 @@ commands: # Verify SB service has RBAC full-access port (16642) - script: | oc get service -n $NAMESPACE ovsdbserver-sb-0 -o jsonpath='{.spec.ports[*].port}' | grep -q "16642" + # Verify system-id is a deterministic UUID5 derived from node name (required for RBAC) + - script: | + controller_pod=$(oc get pod -n $NAMESPACE -l service=ovn-controller -o name|head -1) + system_id=$(oc rsh -n $NAMESPACE ${controller_pod} ovs-vsctl get open . external-ids:system-id | tr -d '"') + node_name=$(oc get -n $NAMESPACE ${controller_pod} -o jsonpath='{.spec.nodeName}') + expected_id=$(python3 -c "import uuid,sys; print(uuid.uuid5(uuid.NAMESPACE_DNS, sys.argv[1]))" "${node_name}") + if [ "${system_id}" != "${expected_id}" ]; then + echo "system-id (${system_id}) does not match expected UUID5 (${expected_id}) for node ${node_name}" + exit 1 + fi + # Verify per-node RBAC certificate exists and CN matches system-id (UUID5) + - script: | + controller_pod=$(oc get pod -n $NAMESPACE -l service=ovn-controller -o name|head -1) + node_name=$(oc get -n $NAMESPACE ${controller_pod} -o jsonpath='{.spec.nodeName}') + expected_id=$(python3 -c "import uuid,sys; print(uuid.uuid5(uuid.NAMESPACE_DNS, sys.argv[1]))" "${node_name}") + cert_cn=$(oc rsh -n $NAMESPACE ${controller_pod} openssl x509 -in /etc/openvswitch/ovn-controller-cert.pem -noout -subject 2>/dev/null | sed -n 's/.*CN *= *\([^ ,]*\).*/\1/p') + if [ -z "${cert_cn}" ]; then + echo "Per-node RBAC certificate not found at /etc/openvswitch/ovn-controller-cert.pem" + exit 1 + fi + if [ "${cert_cn}" != "${expected_id}" ]; then + echo "Certificate CN (${cert_cn}) does not match expected UUID5 (${expected_id}) for node ${node_name}" + exit 1 + fi diff --git a/test/kuttl/tests/ovn_tls_enable/02-assert.yaml b/test/kuttl/tests/ovn_tls_enable/02-assert.yaml index ba17ce9e..0e34d561 100644 --- a/test/kuttl/tests/ovn_tls_enable/02-assert.yaml +++ b/test/kuttl/tests/ovn_tls_enable/02-assert.yaml @@ -47,3 +47,27 @@ commands: # Verify SB service has RBAC full-access port (16642) - script: | oc get service -n $NAMESPACE ovsdbserver-sb-0 -o jsonpath='{.spec.ports[*].port}' | grep -q "16642" + # Verify system-id is a deterministic UUID5 derived from node name (required for RBAC) + - script: | + controller_pod=$(oc get pod -n $NAMESPACE -l service=ovn-controller -o name|head -1) + system_id=$(oc rsh -n $NAMESPACE ${controller_pod} ovs-vsctl get open . external-ids:system-id | tr -d '"') + node_name=$(oc get -n $NAMESPACE ${controller_pod} -o jsonpath='{.spec.nodeName}') + expected_id=$(python3 -c "import uuid,sys; print(uuid.uuid5(uuid.NAMESPACE_DNS, sys.argv[1]))" "${node_name}") + if [ "${system_id}" != "${expected_id}" ]; then + echo "system-id (${system_id}) does not match expected UUID5 (${expected_id}) for node ${node_name}" + exit 1 + fi + # Verify per-node RBAC certificate exists and CN matches system-id (UUID5) + - script: | + controller_pod=$(oc get pod -n $NAMESPACE -l service=ovn-controller -o name|head -1) + node_name=$(oc get -n $NAMESPACE ${controller_pod} -o jsonpath='{.spec.nodeName}') + expected_id=$(python3 -c "import uuid,sys; print(uuid.uuid5(uuid.NAMESPACE_DNS, sys.argv[1]))" "${node_name}") + cert_cn=$(oc rsh -n $NAMESPACE ${controller_pod} openssl x509 -in /etc/openvswitch/ovn-controller-cert.pem -noout -subject 2>/dev/null | sed -n 's/.*CN *= *\([^ ,]*\).*/\1/p') + if [ -z "${cert_cn}" ]; then + echo "Per-node RBAC certificate not found at /etc/openvswitch/ovn-controller-cert.pem" + exit 1 + fi + if [ "${cert_cn}" != "${expected_id}" ]; then + echo "Certificate CN (${cert_cn}) does not match expected UUID5 (${expected_id}) for node ${node_name}" + exit 1 + fi From d8cd9976f64fe51e4cfb20ce3863010e3e03c27c Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 1 Apr 2026 15:51:44 +0200 Subject: [PATCH 3/3] 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 --- .../ovn.openstack.org_ovndbclusters.yaml | 6 +++ api/test/helpers/crd.go | 1 + api/v1beta1/client.go | 21 +++++++++ api/v1beta1/ovndbcluster_types.go | 22 +++++++++ .../ovn.openstack.org_ovndbclusters.yaml | 6 +++ .../controller/ovncontroller_controller.go | 21 +++++++++ .../controller/ovndbcluster_controller.go | 8 ++++ internal/controller/ovnnorthd_controller.go | 12 ++++- test/functional/base_test.go | 45 +++++++++++++++++-- .../ovncontroller_controller_test.go | 3 ++ test/functional/ovnnorthd_controller_test.go | 17 ++++++- test/kuttl/tests/ovn_tls/01-assert.yaml | 23 ++++++++++ .../kuttl/tests/ovn_tls_enable/01-assert.yaml | 12 +++++ .../kuttl/tests/ovn_tls_enable/02-assert.yaml | 23 ++++++++++ 14 files changed, 213 insertions(+), 7 deletions(-) diff --git a/api/bases/ovn.openstack.org_ovndbclusters.yaml b/api/bases/ovn.openstack.org_ovndbclusters.yaml index 55764db1..2a69876d 100644 --- a/api/bases/ovn.openstack.org_ovndbclusters.yaml +++ b/api/bases/ovn.openstack.org_ovndbclusters.yaml @@ -430,6 +430,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/test/helpers/crd.go b/api/test/helpers/crd.go index bb9eb023..7b16e148 100644 --- a/api/test/helpers/crd.go +++ b/api/test/helpers/crd.go @@ -130,6 +130,7 @@ func (th *TestHelper) SimulateOVNNorthdReady(name types.NamespacedName) { gomega.Eventually(func(g gomega.Gomega) { service := th.GetOVNNorthd(name) service.Status.ObservedGeneration = service.Generation + service.Status.ReadyCount = 1 service.Status.Conditions.MarkTrue(condition.ReadyCondition, "Ready") g.Expect(th.K8sClient.Status().Update(th.Ctx, service)).To(gomega.Succeed()) }, th.Timeout, th.Interval).Should(gomega.Succeed()) diff --git a/api/v1beta1/client.go b/api/v1beta1/client.go index b42f2c64..61fb29b3 100644 --- a/api/v1beta1/client.go +++ b/api/v1beta1/client.go @@ -107,6 +107,27 @@ func getItems(list client.ObjectList) []client.Object { return items } +// GetOVNNorthd - return OVNNorthd in the given namespace +func GetOVNNorthd( + ctx context.Context, + h *helper.Helper, + namespace string, +) (*OVNNorthd, error) { + ovnNorthdList := &OVNNorthdList{} + listOpts := []client.ListOption{ + client.InNamespace(namespace), + } + err := h.GetClient().List(ctx, ovnNorthdList, listOpts...) + if err != nil { + return nil, err + } + if len(ovnNorthdList.Items) > 0 { + return &ovnNorthdList.Items[0], nil + } + + return nil, nil +} + // OVNCRNamespaceMapFunc // Generic function to watch any OVN CR func OVNCRNamespaceMapFunc(crs client.ObjectList, reader client.Reader) handler.MapFunc { return func(ctx context.Context, obj client.Object) []reconcile.Request { diff --git a/api/v1beta1/ovndbcluster_types.go b/api/v1beta1/ovndbcluster_types.go index 16a7e89d..8849b404 100644 --- a/api/v1beta1/ovndbcluster_types.go +++ b/api/v1beta1/ovndbcluster_types.go @@ -170,6 +170,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"` @@ -238,6 +243,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 index 55764db1..2a69876d 100644 --- a/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml +++ b/config/crd/bases/ovn.openstack.org_ovndbclusters.yaml @@ -430,6 +430,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/ovncontroller_controller.go b/internal/controller/ovncontroller_controller.go index 5eb75860..1fccd409 100644 --- a/internal/controller/ovncontroller_controller.go +++ b/internal/controller/ovncontroller_controller.go @@ -267,6 +267,7 @@ func (r *OVNControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&rbacv1.Role{}). Owns(&rbacv1.RoleBinding{}). Watches(&ovnv1.OVNDBCluster{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))). + Watches(&ovnv1.OVNNorthd{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))). Watches( &corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc), @@ -641,6 +642,26 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance return ctrl.Result{}, fmt.Errorf("waiting for Topology requirements: %w", err) } + // When TLS is enabled, RBAC is used for ovn-controller connections to the SB + // database. The RBAC permission rules are populated by ovn-northd, so we must + // wait for northd to be ready before deploying ovn-controller to avoid RBAC + // permission errors during startup. + if instance.Spec.TLS.Enabled() { + northd, err := ovnv1.GetOVNNorthd(ctx, helper, instance.Namespace) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to look up OVNNorthd: %w", err) + } + if northd == nil || northd.Status.ReadyCount == 0 { + Log.Info("OVNNorthd is not ready yet, waiting before deploying OVNController to avoid RBAC errors") + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DeploymentReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.DeploymentReadyRunningMessage)) + return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + } + } + // Define a new DaemonSet object for OVNController dset := daemonset.NewDaemonSet( ovncontroller.CreateOVNDaemonSet(instance, inputHash, ovnServiceLabels, topology), diff --git a/internal/controller/ovndbcluster_controller.go b/internal/controller/ovndbcluster_controller.go index f302702e..97c41b63 100644 --- a/internal/controller/ovndbcluster_controller.go +++ b/internal/controller/ovndbcluster_controller.go @@ -755,6 +755,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() { @@ -770,6 +771,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 @@ -779,6 +786,7 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance * // Set DB Address instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",") + 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 index 8651514c..c976c536 100644 --- 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/test/functional/base_test.go b/test/functional/base_test.go index 8e7b56b5..d0ab4529 100644 --- 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/ovncontroller_controller_test.go b/test/functional/ovncontroller_controller_test.go index 0ed97a68..0b5eb2e4 100644 --- a/test/functional/ovncontroller_controller_test.go +++ b/test/functional/ovncontroller_controller_test.go @@ -912,6 +912,9 @@ var _ = Describe("OVNController controller", func() { BeforeEach(func() { dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1) DeferCleanup(DeleteOVNDBClusters, dbs) + ovnNorthdName := ovn.CreateOVNNorthd(nil, namespace, GetDefaultOVNNorthdSpec()) + DeferCleanup(ovn.DeleteOVNNorthd, ovnNorthdName) + ovn.SimulateOVNNorthdReady(ovnNorthdName) instance := CreateOVNController(namespace, GetTLSOVNControllerSpec()) DeferCleanup(th.DeleteInstance, instance) diff --git a/test/functional/ovnnorthd_controller_test.go b/test/functional/ovnnorthd_controller_test.go index 5474bfc9..33d0a198 100644 --- 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/kuttl/tests/ovn_tls/01-assert.yaml b/test/kuttl/tests/ovn_tls/01-assert.yaml index ea9afc75..31c44235 100644 --- a/test/kuttl/tests/ovn_tls/01-assert.yaml +++ b/test/kuttl/tests/ovn_tls/01-assert.yaml @@ -127,3 +127,26 @@ commands: echo "Certificate CN (${cert_cn}) does not match expected UUID5 (${expected_id}) for node ${node_name}" exit 1 fi + # Verify internalDbAddressRbacFullAccess is set on SB OVNDBCluster with port 16642 + - script: | + template='{{.status.internalDbAddressRbacFullAccess}}{{"\n"}}' + dbUri=$(oc get -n $NAMESPACE OVNDBCluster ovndbcluster-sb-sample -o go-template="$template") + if [[ -z "$dbUri" ]]; then + echo "internalDbAddressRbacFullAccess is not set on SB OVNDBCluster" + exit 1 + fi + echo "$dbUri" | grep -q "ssl:" || { echo "internalDbAddressRbacFullAccess does not use ssl scheme"; exit 1; } + echo "$dbUri" | grep -q ":16642" || { echo "internalDbAddressRbacFullAccess does not use port 16642"; exit 1; } + # Verify internalDbAddressRbacFullAccess is NOT set on NB OVNDBCluster + - script: | + template='{{.status.internalDbAddressRbacFullAccess}}{{"\n"}}' + dbUri=$(oc get -n $NAMESPACE OVNDBCluster ovndbcluster-nb-sample -o go-template="$template") + if [[ -n "$dbUri" && "$dbUri" != "" ]]; then + echo "internalDbAddressRbacFullAccess should not be set on NB OVNDBCluster but got: ${dbUri}" + exit 1 + fi + # Verify northd uses port 16642 for SB and port 6641 for NB + - script: | + northd_args=$(oc get statefulset -n $NAMESPACE ovn-northd -o jsonpath='{.spec.template.spec.containers[0].args}') + echo "$northd_args" | grep -q -e "--ovnsb-db=.*:16642" || { echo "northd --ovnsb-db does not use port 16642"; exit 1; } + echo "$northd_args" | grep -q -e "--ovnnb-db=.*:6641" || { echo "northd --ovnnb-db does not use port 6641"; exit 1; } diff --git a/test/kuttl/tests/ovn_tls_enable/01-assert.yaml b/test/kuttl/tests/ovn_tls_enable/01-assert.yaml index 581f425e..756a5f1d 100644 --- a/test/kuttl/tests/ovn_tls_enable/01-assert.yaml +++ b/test/kuttl/tests/ovn_tls_enable/01-assert.yaml @@ -37,3 +37,15 @@ commands: ../../common/scripts/check_cluster_status.sh nb 3 - script: | ../../common/scripts/check_cluster_status.sh sb 3 + # Verify internalDbAddressRbacFullAccess is NOT set on SB OVNDBCluster (no TLS = no RBAC) + - script: | + template='{{.status.internalDbAddressRbacFullAccess}}{{"\n"}}' + dbUri=$(oc get -n $NAMESPACE OVNDBCluster ovndbcluster-sb-sample -o go-template="$template") + if [[ -n "$dbUri" && "$dbUri" != "" ]]; then + echo "internalDbAddressRbacFullAccess should not be set without TLS but got: ${dbUri}" + exit 1 + fi + # Verify northd uses regular port 6642 for SB (no RBAC without TLS) + - script: | + northd_args=$(oc get statefulset -n $NAMESPACE ovn-northd -o jsonpath='{.spec.template.spec.containers[0].args}') + echo "$northd_args" | grep -q -e "--ovnsb-db=.*:6642" || { echo "northd --ovnsb-db does not use port 6642"; exit 1; } diff --git a/test/kuttl/tests/ovn_tls_enable/02-assert.yaml b/test/kuttl/tests/ovn_tls_enable/02-assert.yaml index 0e34d561..ebe03e5f 100644 --- a/test/kuttl/tests/ovn_tls_enable/02-assert.yaml +++ b/test/kuttl/tests/ovn_tls_enable/02-assert.yaml @@ -71,3 +71,26 @@ commands: echo "Certificate CN (${cert_cn}) does not match expected UUID5 (${expected_id}) for node ${node_name}" exit 1 fi + # Verify internalDbAddressRbacFullAccess is set on SB OVNDBCluster with port 16642 + - script: | + template='{{.status.internalDbAddressRbacFullAccess}}{{"\n"}}' + dbUri=$(oc get -n $NAMESPACE OVNDBCluster ovndbcluster-sb-sample -o go-template="$template") + if [[ -z "$dbUri" ]]; then + echo "internalDbAddressRbacFullAccess is not set on SB OVNDBCluster" + exit 1 + fi + echo "$dbUri" | grep -q "ssl:" || { echo "internalDbAddressRbacFullAccess does not use ssl scheme"; exit 1; } + echo "$dbUri" | grep -q ":16642" || { echo "internalDbAddressRbacFullAccess does not use port 16642"; exit 1; } + # Verify internalDbAddressRbacFullAccess is NOT set on NB OVNDBCluster + - script: | + template='{{.status.internalDbAddressRbacFullAccess}}{{"\n"}}' + dbUri=$(oc get -n $NAMESPACE OVNDBCluster ovndbcluster-nb-sample -o go-template="$template") + if [[ -n "$dbUri" && "$dbUri" != "" ]]; then + echo "internalDbAddressRbacFullAccess should not be set on NB OVNDBCluster but got: ${dbUri}" + exit 1 + fi + # Verify northd uses port 16642 for SB and port 6641 for NB + - script: | + northd_args=$(oc get statefulset -n $NAMESPACE ovn-northd -o jsonpath='{.spec.template.spec.containers[0].args}') + echo "$northd_args" | grep -q -e "--ovnsb-db=.*:16642" || { echo "northd --ovnsb-db does not use port 16642"; exit 1; } + echo "$northd_args" | grep -q -e "--ovnnb-db=.*:6641" || { echo "northd --ovnnb-db does not use port 6641"; exit 1; }