From 873083b3eac17271557be4e1fb89a2ef3803a199 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 24 Apr 2026 12:05:02 +0200 Subject: [PATCH 1/2] Generate OVN RBAC PKI This patch adds generation of the rootca-ovn-rbac issuer which is used by the ovn-operator to sign per-node ovn-controller RBAC certificates. This CA is intentionally not added to the combined CA bundle as it is only used between the SB database and ovn-controller nodes. When TLS is enabled, the reconciler passes the RBAC CA cert secret name to the SB DB cluster and the RBAC issuer name to OVNController so the ovn-operator can create cert-manager Certificate resources and verify client connections. Related: #OSPRH-1921 Related: #OSPRH-1922 Signed-off-by: Slawek Kaplonski --- ....openstack.org_openstackcontrolplanes.yaml | 4 + .../v1beta1/openstackcontrolplane_types.go | 7 + bindata/crds/crds.yaml | 4 + .../ovn.openstack.org_ovncontrollers.yaml | 7 + .../crds/ovn.openstack.org_ovndbclusters.yaml | 6 + bindata/rbac/ovn-operator-rbac.yaml | 20 +++ ....openstack.org_openstackcontrolplanes.yaml | 4 + internal/openstack/ca.go | 35 +++++ internal/openstack/common.go | 4 + internal/openstack/ovn.go | 12 ++ test/functional/ctlplane/base_test.go | 14 ++ .../openstackoperator_controller_test.go | 145 +++++++++++++++++- 12 files changed, 261 insertions(+), 1 deletion(-) diff --git a/api/bases/core.openstack.org_openstackcontrolplanes.yaml b/api/bases/core.openstack.org_openstackcontrolplanes.yaml index 81a216d318..5e5da2e62b 100644 --- a/api/bases/core.openstack.org_openstackcontrolplanes.yaml +++ b/api/bases/core.openstack.org_openstackcontrolplanes.yaml @@ -13083,6 +13083,8 @@ spec: ovsLogLevel: default: info type: string + rbacIssuerName: + type: string resources: properties: claims: @@ -13217,6 +13219,8 @@ spec: default: 60000 format: int32 type: integer + rbacCACertSecretName: + type: string replicas: default: 1 format: int32 diff --git a/api/core/v1beta1/openstackcontrolplane_types.go b/api/core/v1beta1/openstackcontrolplane_types.go index 1c4721e5a7..1c7ecd2964 100644 --- a/api/core/v1beta1/openstackcontrolplane_types.go +++ b/api/core/v1beta1/openstackcontrolplane_types.go @@ -61,6 +61,8 @@ const ( OvnDbCaName = tls.DefaultCAPrefix + "ovn" // LibvirtCaName - LibvirtCaName = tls.DefaultCAPrefix + "libvirt" + // OvnRbacCaName - + OvnRbacCaName = tls.DefaultCAPrefix + "ovn-rbac" // GlanceName - Default Glance name GlanceName = "glance" @@ -1244,6 +1246,11 @@ func (instance OpenStackControlPlane) GetOvnIssuer() string { return OvnDbCaName } +// GetOvnRbacIssuer - returns the OVN RBAC CA issuer name +func (instance OpenStackControlPlane) GetOvnRbacIssuer() string { + return OvnRbacCaName +} + // GetLibvirtIssuer - returns the libvirt CA issuer name or custom if configured func (instance OpenStackControlPlane) GetLibvirtIssuer() string { // use custom issuer if set diff --git a/bindata/crds/crds.yaml b/bindata/crds/crds.yaml index 38aed21500..615e112e07 100644 --- a/bindata/crds/crds.yaml +++ b/bindata/crds/crds.yaml @@ -13617,6 +13617,8 @@ spec: ovsLogLevel: default: info type: string + rbacIssuerName: + type: string resources: properties: claims: @@ -13751,6 +13753,8 @@ spec: default: 60000 format: int32 type: integer + rbacCACertSecretName: + type: string replicas: default: 1 format: int32 diff --git a/bindata/crds/ovn.openstack.org_ovncontrollers.yaml b/bindata/crds/ovn.openstack.org_ovncontrollers.yaml index 9a34138036..e5c3bdb70c 100644 --- a/bindata/crds/ovn.openstack.org_ovncontrollers.yaml +++ b/bindata/crds/ovn.openstack.org_ovncontrollers.yaml @@ -164,6 +164,13 @@ spec: description: OVSLogLevel - Set log level off, emer, err, warn, info, or dbg. Default is info. type: string + rbacIssuerName: + description: |- + RbacIssuerName - The name of the cert-manager Issuer used to sign + per-node ovn-controller RBAC certificates. When set, the controller + creates cert-manager Certificate resources for each node instead of + signing certificates locally with the CA key. + type: string resources: description: |- Resources - Compute Resources required by this service (Limits/Requests). diff --git a/bindata/crds/ovn.openstack.org_ovndbclusters.yaml b/bindata/crds/ovn.openstack.org_ovndbclusters.yaml index ad2d4748dc..a9bbfd9cdb 100644 --- a/bindata/crds/ovn.openstack.org_ovndbclusters.yaml +++ b/bindata/crds/ovn.openstack.org_ovndbclusters.yaml @@ -266,6 +266,12 @@ spec: Active probe interval from standby to active ovsdb-server remote format: int32 type: integer + rbacCACertSecretName: + description: |- + RbacCACertSecretName - The name of the K8s Secret containing the RBAC + PKI CA certificate (tls.crt). Used by the SB database to verify + ovn-controller client certificates when RBAC is enabled. + type: string replicas: default: 1 description: Replicas of OVN DBCluster to run diff --git a/bindata/rbac/ovn-operator-rbac.yaml b/bindata/rbac/ovn-operator-rbac.yaml index 2e133574fd..1195004732 100644 --- a/bindata/rbac/ovn-operator-rbac.yaml +++ b/bindata/rbac/ovn-operator-rbac.yaml @@ -127,6 +127,26 @@ rules: - patch - update - watch +- apiGroups: + - cert-manager.io + resources: + - certificates + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - cert-manager.io + resources: + - issuers + verbs: + - get + - list + - watch - apiGroups: - k8s.cni.cncf.io resources: diff --git a/config/crd/bases/core.openstack.org_openstackcontrolplanes.yaml b/config/crd/bases/core.openstack.org_openstackcontrolplanes.yaml index 81a216d318..5e5da2e62b 100644 --- a/config/crd/bases/core.openstack.org_openstackcontrolplanes.yaml +++ b/config/crd/bases/core.openstack.org_openstackcontrolplanes.yaml @@ -13083,6 +13083,8 @@ spec: ovsLogLevel: default: info type: string + rbacIssuerName: + type: string resources: properties: claims: @@ -13217,6 +13219,8 @@ spec: default: 60000 format: int32 type: integer + rbacCACertSecretName: + type: string replicas: default: 1 format: int32 diff --git a/internal/openstack/ca.go b/internal/openstack/ca.go index 8791622455..878aa9be04 100644 --- a/internal/openstack/ca.go +++ b/internal/openstack/ca.go @@ -408,6 +408,41 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h } } + // create CA for OVN RBAC (used to sign per-node ovn-controller certificates) + // This CA is NOT added to the combined CA bundle — it is only used between + // the SB database (to verify ovn-controller client certs) and the + // ovn-controller nodes (whose certs are signed by this CA via cert-manager). + issuerLabels = map[string]string{rootCAIssuerOvnRbacLabel: ""} + issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Ovn.Cert) + err = removeIssuerLabel( + ctx, + helper, + corev1.OvnRbacCaName, + instance.Namespace, + issuerLabels, + ) + if err != nil { + return ctrl.Result{}, err + } + + ctrlResult, err = ensureRootCA( + ctx, + instance, + helper, + issuerReq, + corev1.OvnRbacCaName, + issuerLabels, + issuerAnnotations, + bundle, + caOnlyBundle, + instance.Spec.TLS.PodLevel.Ovn.Ca, + ) + if err != nil { + return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + // create/update combined CA secret if instance.Spec.TLS.CaBundleSecretName != "" { caSecret, _, err := secret.GetSecret(ctx, helper, instance.Spec.TLS.CaBundleSecretName, instance.Namespace) diff --git a/internal/openstack/common.go b/internal/openstack/common.go index 3b9e914bee..5e436fdc34 100644 --- a/internal/openstack/common.go +++ b/internal/openstack/common.go @@ -70,6 +70,10 @@ const ( // caCertSelector selector passed to cert-manager to set on the ca cert secret caCertSelector = "ca-cert" + // rootCAIssuerOvnRbacLabel labels the OVN RBAC CA issuer. + // TODO: upstream this to lib-common certmanager module alongside the other RootCAIssuer*Label constants. + rootCAIssuerOvnRbacLabel = "osp-rootca-issuer-ovn-rbac" + // ReconcileTriggerAnnotation - Generic annotation to trigger reconciliation and webhook. // Value is typically a timestamp to ensure annotation changes trigger updates // Used by controller to trigger UPDATE webhook when needed (e.g., for service name caching, field migrations) diff --git a/internal/openstack/ovn.go b/internal/openstack/ovn.go index e18f8c6f6c..b20e2140a2 100644 --- a/internal/openstack/ovn.go +++ b/internal/openstack/ovn.go @@ -208,6 +208,12 @@ func ReconcileOVNDbClusters(ctx context.Context, instance *corev1beta1.OpenStack dbcluster.MetricsTLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName } + // Pass the RBAC CA cert secret name to SB DB clusters so they can + // build a combined CA bundle for verifying ovn-controller client certs + if instance.Spec.TLS.PodLevel.Enabled && dbcluster.DBType == ovnv1.SBDBType { + dbcluster.RbacCACertSecretName = corev1beta1.OvnRbacCaName + } + if dbcluster.NodeSelector == nil { dbcluster.NodeSelector = &instance.Spec.NodeSelector } @@ -493,6 +499,12 @@ func ReconcileOVNController(ctx context.Context, instance *corev1beta1.OpenStack ovnControllerSpec.MetricsTLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName } + // Pass the RBAC CA issuer name so the OVNController can create per-node + // cert-manager Certificate resources for OVN RBAC + if instance.Spec.TLS.PodLevel.Enabled { + ovnControllerSpec.RbacIssuerName = instance.GetOvnRbacIssuer() + } + if ovnControllerSpec.NodeSelector == nil { ovnControllerSpec.NodeSelector = &instance.Spec.NodeSelector } diff --git a/test/functional/ctlplane/base_test.go b/test/functional/ctlplane/base_test.go index 6806c3bfc2..2a313d70d0 100644 --- a/test/functional/ctlplane/base_test.go +++ b/test/functional/ctlplane/base_test.go @@ -86,6 +86,7 @@ type Names struct { RootCAPublicName types.NamespacedName RootCAInternalName types.NamespacedName RootCAOvnName types.NamespacedName + RootCAOvnRbacName types.NamespacedName RootCALibvirtName types.NamespacedName SelfSignedIssuerName types.NamespacedName CustomIssuerName types.NamespacedName @@ -97,7 +98,9 @@ type Names struct { OVNControllerName types.NamespacedName OVNControllerCertName types.NamespacedName OVNDbServerNBName types.NamespacedName + OVNDbServerNBCertName types.NamespacedName OVNDbServerSBName types.NamespacedName + OVNDbServerSBCertName types.NamespacedName OVNMetricsCertName types.NamespacedName NeutronOVNCertName types.NamespacedName OpenStackTopology []types.NamespacedName @@ -131,6 +134,9 @@ func CreateNames(openstackControlplaneName types.NamespacedName) Names { RootCAOvnName: types.NamespacedName{ Namespace: openstackControlplaneName.Namespace, Name: "rootca-ovn"}, + RootCAOvnRbacName: types.NamespacedName{ + Namespace: openstackControlplaneName.Namespace, + Name: "rootca-ovn-rbac"}, RootCALibvirtName: types.NamespacedName{ Namespace: openstackControlplaneName.Namespace, Name: "rootca-libvirt"}, @@ -275,10 +281,18 @@ func CreateNames(openstackControlplaneName types.NamespacedName) Names { Namespace: openstackControlplaneName.Namespace, Name: "ovndbcluster-nb", }, + OVNDbServerNBCertName: types.NamespacedName{ + Namespace: openstackControlplaneName.Namespace, + Name: "cert-ovndbcluster-nb-ovndbs", + }, OVNDbServerSBName: types.NamespacedName{ Namespace: openstackControlplaneName.Namespace, Name: "ovndbcluster-sb", }, + OVNDbServerSBCertName: types.NamespacedName{ + Namespace: openstackControlplaneName.Namespace, + Name: "cert-ovndbcluster-sb-ovndbs", + }, OVNControllerName: types.NamespacedName{ Namespace: openstackControlplaneName.Namespace, Name: "ovncontroller", diff --git a/test/functional/ctlplane/openstackoperator_controller_test.go b/test/functional/ctlplane/openstackoperator_controller_test.go index d65e71ea14..482426f90b 100644 --- a/test/functional/ctlplane/openstackoperator_controller_test.go +++ b/test/functional/ctlplane/openstackoperator_controller_test.go @@ -74,6 +74,7 @@ var _ = Describe("OpenStackOperator controller", func() { DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAInternalName)) DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAOvnName)) DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCALibvirtName)) + DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAOvnRbacName)) // create cert secrets for galera instances DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.DBCertName)) DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.DBCell1CertName)) @@ -442,6 +443,12 @@ var _ = Describe("OpenStackOperator controller", func() { g.Expect(issuer.Annotations).Should(HaveKeyWithValue(certmanager.CertDurationAnnotation, "43800h0m0s")) g.Expect(issuer.Annotations).Should(Not(HaveKey(certmanager.CertRenewBeforeAnnotation))) }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + issuer := crtmgr.GetIssuer(names.RootCAOvnRbacName) + g.Expect(issuer).Should(Not(BeNil())) + g.Expect(issuer.Annotations).Should(HaveKeyWithValue(certmanager.CertDurationAnnotation, "43800h0m0s")) + g.Expect(issuer.Annotations).Should(Not(HaveKey(certmanager.CertRenewBeforeAnnotation))) + }, timeout, interval).Should(Succeed()) }) }) When("TLS - A TLSe OpenStackControlplane instance is created with customized internal ca duration", func() { @@ -497,6 +504,12 @@ var _ = Describe("OpenStackOperator controller", func() { g.Expect(issuer.Annotations).Should(HaveKeyWithValue(certmanager.CertDurationAnnotation, "43800h0m0s")) g.Expect(issuer.Annotations).Should(Not(HaveKey(certmanager.CertRenewBeforeAnnotation))) }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + issuer := crtmgr.GetIssuer(names.RootCAOvnRbacName) + g.Expect(issuer).Should(Not(BeNil())) + g.Expect(issuer.Annotations).Should(HaveKeyWithValue(certmanager.CertDurationAnnotation, "43800h0m0s")) + g.Expect(issuer.Annotations).Should(Not(HaveKey(certmanager.CertRenewBeforeAnnotation))) + }, timeout, interval).Should(Succeed()) }) }) When("TLS - A TLSe OpenStackControlplane instance is created with customized internal cert duration", func() { @@ -552,6 +565,12 @@ var _ = Describe("OpenStackOperator controller", func() { g.Expect(issuer.Annotations).Should(HaveKeyWithValue(certmanager.CertDurationAnnotation, "43800h0m0s")) g.Expect(issuer.Annotations).Should(Not(HaveKey(certmanager.CertRenewBeforeAnnotation))) }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + issuer := crtmgr.GetIssuer(names.RootCAOvnRbacName) + g.Expect(issuer).Should(Not(BeNil())) + g.Expect(issuer.Annotations).Should(HaveKeyWithValue(certmanager.CertDurationAnnotation, "43800h0m0s")) + g.Expect(issuer.Annotations).Should(Not(HaveKey(certmanager.CertRenewBeforeAnnotation))) + }, timeout, interval).Should(Succeed()) }) }) When("TLS - A TLSe OpenStackControlplane instance is created with an internal custom issuer", func() { @@ -839,6 +858,19 @@ var _ = Describe("OpenStackOperator controller", func() { g.Expect(issuer).Should(Not(BeNil())) g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCALibvirtName.Name)) }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + // ca cert + cert := crtmgr.GetCert(names.RootCAOvnRbacName) + g.Expect(cert).Should(Not(BeNil())) + g.Expect(cert.Spec.CommonName).Should(Equal(names.RootCAOvnRbacName.Name)) + g.Expect(cert.Spec.IsCA).Should(BeTrue()) + g.Expect(cert.Spec.IssuerRef.Name).Should(Equal(names.SelfSignedIssuerName.Name)) + g.Expect(cert.Spec.SecretName).Should(Equal(names.RootCAOvnRbacName.Name)) + // issuer + issuer := crtmgr.GetIssuer(names.RootCAOvnRbacName) + g.Expect(issuer).Should(Not(BeNil())) + g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAOvnRbacName.Name)) + }, timeout, interval).Should(Succeed()) }) It("should create full ca bundle", func() { @@ -850,6 +882,8 @@ var _ = Describe("OpenStackOperator controller", func() { crtmgr.GetIssuer(names.RootCAOvnName) crtmgr.GetCert(names.RootCALibvirtName) crtmgr.GetIssuer(names.RootCALibvirtName) + crtmgr.GetCert(names.RootCAOvnRbacName) + crtmgr.GetIssuer(names.RootCAOvnRbacName) Eventually(func(g Gomega) { th.GetSecret(names.RootCAPublicName) @@ -1014,7 +1048,7 @@ var _ = Describe("OpenStackOperator controller", func() { //Expect(OSCtlplane.Spec.Placement.APIOverride.Route.Annotations).Should(HaveKeyWithValue("api.placement.openstack.org/timeout", "60s")) }) - It("should create selfsigned issuer and public, internal, libvirt and ovn CA and issuer", func() { + It("should create selfsigned issuer and public, internal, libvirt, ovn and ovn-rbac CA and issuer", func() { OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) Expect(OSCtlplane.Spec.TLS.Ingress.Enabled).Should(BeTrue()) @@ -1082,6 +1116,20 @@ var _ = Describe("OpenStackOperator controller", func() { g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCALibvirtName.Name)) g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerLibvirtLabel)) }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + // ca cert + cert := crtmgr.GetCert(names.RootCAOvnRbacName) + g.Expect(cert).Should(Not(BeNil())) + g.Expect(cert.Spec.CommonName).Should(Equal(names.RootCAOvnRbacName.Name)) + g.Expect(cert.Spec.IsCA).Should(BeTrue()) + g.Expect(cert.Spec.IssuerRef.Name).Should(Equal(names.SelfSignedIssuerName.Name)) + g.Expect(cert.Spec.SecretName).Should(Equal(names.RootCAOvnRbacName.Name)) + // issuer + issuer := crtmgr.GetIssuer(names.RootCAOvnRbacName) + g.Expect(issuer).Should(Not(BeNil())) + g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAOvnRbacName.Name)) + g.Expect(issuer.Labels).Should(HaveKey("osp-rootca-issuer-ovn-rbac")) + }, timeout, interval).Should(Succeed()) th.ExpectCondition( names.OpenStackControlplaneName, @@ -1107,6 +1155,8 @@ var _ = Describe("OpenStackOperator controller", func() { crtmgr.GetIssuer(names.RootCAOvnName) crtmgr.GetCert(names.RootCALibvirtName) crtmgr.GetIssuer(names.RootCALibvirtName) + crtmgr.GetCert(names.RootCAOvnRbacName) + crtmgr.GetIssuer(names.RootCAOvnRbacName) Eventually(func(g Gomega) { th.GetSecret(names.RootCAPublicName) @@ -1357,6 +1407,19 @@ var _ = Describe("OpenStackOperator controller", func() { g.Expect(issuer).Should(Not(BeNil())) g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCALibvirtName.Name)) }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + // ca cert + cert := crtmgr.GetCert(names.RootCAOvnRbacName) + g.Expect(cert).Should(Not(BeNil())) + g.Expect(cert.Spec.CommonName).Should(Equal(names.RootCAOvnRbacName.Name)) + g.Expect(cert.Spec.IsCA).Should(BeTrue()) + g.Expect(cert.Spec.IssuerRef.Name).Should(Equal(names.SelfSignedIssuerName.Name)) + g.Expect(cert.Spec.SecretName).Should(Equal(names.RootCAOvnRbacName.Name)) + // issuer + issuer := crtmgr.GetIssuer(names.RootCAOvnRbacName) + g.Expect(issuer).Should(Not(BeNil())) + g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAOvnRbacName.Name)) + }, timeout, interval).Should(Succeed()) th.ExpectCondition( names.OpenStackControlplaneName, @@ -1833,6 +1896,23 @@ var _ = Describe("OpenStackOperator controller", func() { }, timeout, interval).Should(Succeed()) }) + It("should not set RBAC fields when TLS pod-level is disabled", func() { + Eventually(func(g Gomega) { + ovnDbServerSB := ovn.GetOVNDBCluster(names.OVNDbServerSBName) + g.Expect(ovnDbServerSB.Spec.RbacCACertSecretName).Should(BeEmpty()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + ovnDbServerNB := ovn.GetOVNDBCluster(names.OVNDbServerNBName) + g.Expect(ovnDbServerNB.Spec.RbacCACertSecretName).Should(BeEmpty()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + ovnController := ovn.GetOVNController(names.OVNControllerName) + g.Expect(ovnController.Spec.RbacIssuerName).Should(BeEmpty()) + }, timeout, interval).Should(Succeed()) + }) + It("should remove ovn-controller if nicMappings are removed", func() { // Update spec Eventually(func(g Gomega) { @@ -1905,6 +1985,68 @@ var _ = Describe("OpenStackOperator controller", func() { }) }) + When("A OVN OpenStackControlplane instance with TLS pod-level enabled is created", func() { + BeforeEach(func() { + // create cert secrets for rabbitmq instances + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQCertName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQCell1CertName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQNotificationsCertName)) + // create cert secrets for memcached instance + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.MemcachedCertName)) + // create cert secrets for ovn instance + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.OVNNorthdCertName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.OVNControllerCertName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.OVNMetricsCertName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.NeutronOVNCertName)) + // create cert secrets for ovn db clusters (needed for TLS pod-level) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.OVNDbServerNBCertName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.OVNDbServerSBCertName)) + + spec := GetDefaultOpenStackControlPlaneSpec() + spec["ovn"] = map[string]interface{}{ + "enabled": true, + "template": map[string]interface{}{ + "ovnDBCluster": map[string]interface{}{ + "ovndbcluster-nb": map[string]interface{}{ + "dbType": "NB", + }, + "ovndbcluster-sb": map[string]interface{}{ + "dbType": "SB", + }, + }, + "ovnController": map[string]interface{}{ + "nicMappings": map[string]interface{}{ + "datacentre": "ospbr", + }, + }, + }, + } + DeferCleanup( + th.DeleteInstance, + CreateOpenStackControlPlane(names.OpenStackControlplaneName, spec), + ) + }) + + It("should set RbacCACertSecretName on SB OVNDBCluster only", func() { + Eventually(func(g Gomega) { + ovnDbServerSB := ovn.GetOVNDBCluster(names.OVNDbServerSBName) + g.Expect(ovnDbServerSB.Spec.RbacCACertSecretName).Should(Equal(corev1.OvnRbacCaName)) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + ovnDbServerNB := ovn.GetOVNDBCluster(names.OVNDbServerNBName) + g.Expect(ovnDbServerNB.Spec.RbacCACertSecretName).Should(BeEmpty()) + }, timeout, interval).Should(Succeed()) + }) + + It("should set RbacIssuerName on OVNController", func() { + Eventually(func(g Gomega) { + ovnController := ovn.GetOVNController(names.OVNControllerName) + g.Expect(ovnController.Spec.RbacIssuerName).Should(Equal(corev1.OvnRbacCaName)) + }, timeout, interval).Should(Succeed()) + }) + }) + When("A OpenStackControlplane instance is created", func() { BeforeEach(func() { // NOTE(bogdando): DBs certs need to be created here as well, but those are already existing somehow @@ -4027,6 +4169,7 @@ var _ = Describe("OpenStackOperator controller nova cell deletion", func() { DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAInternalName)) DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAOvnName)) DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCALibvirtName)) + DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAOvnRbacName)) }) When("openstack galera and rabbitmq deletion by cell", func() { From 0bed68f2f389d7f07f633877d87b06ac8eb3ffe7 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 30 Apr 2026 11:22:36 +0200 Subject: [PATCH 2/2] Generate SSL cert for ovn and neutron services running on EDPM nodes This patch adds generations of the individual SSL certificate for each EDPM node. Those certificates are signed with the cert from the OVN SB DB and each of them have CN field set to `uuid5(hostname)` so that the same uuid can be later set as `system-id` on the EDPM node. This is mandatory to make OVN with RBAC working fine. Generated certificates are stored in secret and mounted in the ansibleee POD which provisions ovn-controller service. From there edpm-ansible role can copy it to the EDPM nodes individually. Related: #OSPRH-1921 Related: #OSPRH-1923 Related: #OSPRH-1924 Related: #OSPRH-1925 Signed-off-by: Slawek Kaplonski --- ...nstack.org_openstackdataplaneservices.yaml | 15 ++- .../openstackdataplaneservice_types.go | 16 ++- ...nstack.org_openstackdataplaneservices.yaml | 15 ++- ...tackdataplaneservice_neutron_metadata.yaml | 6 + ...openstackdataplaneservice_neutron_ovn.yaml | 6 + ...v1beta1_openstackdataplaneservice_ovn.yaml | 6 + ...enstackdataplaneservice_ovn_bgp_agent.yaml | 6 + internal/dataplane/cert.go | 19 ++- internal/dataplane/cert_test.go | 110 ++++++++++++++++++ ...enstackdataplaneservice_controller_test.go | 60 ++++++++++ 10 files changed, 246 insertions(+), 13 deletions(-) create mode 100644 internal/dataplane/cert_test.go diff --git a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml index 917bae442f..38ef4a2943 100644 --- a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml +++ b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml @@ -151,13 +151,22 @@ spec: OpenstackDataPlaneServiceCert defines the property of a TLS cert issued for a dataplane service properties: + commonName: + description: |- + CommonName overrides how the certificate Common Name is derived. + When set to "system-id", the CN is a UUID5 derived from the node's + ctlplane FQDN, matching the OVN chassis system-id convention. + When empty, CN defaults to the short hostname. + enum: + - system-id + type: string contents: description: |- Contents of the certificate - This is a list of strings for properties that are needed in the cert + This is a list of strings for properties that are needed in the cert. + May be empty for client-only certificates that require no SANs. items: type: string - minItems: 1 type: array edpmRoleServiceName: description: |- @@ -241,8 +250,6 @@ spec: pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$ type: string type: array - required: - - contents type: object description: TLSCerts tls certs to be generated type: object diff --git a/api/dataplane/v1beta1/openstackdataplaneservice_types.go b/api/dataplane/v1beta1/openstackdataplaneservice_types.go index d613f4fab6..792454ddcb 100644 --- a/api/dataplane/v1beta1/openstackdataplaneservice_types.go +++ b/api/dataplane/v1beta1/openstackdataplaneservice_types.go @@ -28,10 +28,10 @@ import ( // a dataplane service type OpenstackDataPlaneServiceCert struct { // Contents of the certificate - // This is a list of strings for properties that are needed in the cert - // +kubebuilder:validation:Required - // +kubebuilder:validation:MinItems:=1 - Contents []string `json:"contents"` + // This is a list of strings for properties that are needed in the cert. + // May be empty for client-only certificates that require no SANs. + // +kubebuilder:validation:Optional + Contents []string `json:"contents,omitempty"` // Networks to include in SNI for the cert // +kubebuilder:validation:Optional @@ -46,6 +46,14 @@ type OpenstackDataPlaneServiceCert struct { // +kubebuilder:validation:Optional KeyUsages []certmgrv1.KeyUsage `json:"keyUsages,omitempty" yaml:"keyUsages,omitempty"` + // CommonName overrides how the certificate Common Name is derived. + // When set to "system-id", the CN is a UUID5 derived from the node's + // ctlplane FQDN, matching the OVN chassis system-id convention. + // When empty, CN defaults to the short hostname. + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Enum=system-id + CommonName string `json:"commonName,omitempty"` + // EDPMRoleServiceName is the value of the _service_name variable from // the edpm-ansible role where this certificate is used. For example if the // certificate is for edpm_ovn from edpm-ansible, EDPMRoleServiceName must be diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml index 917bae442f..38ef4a2943 100644 --- a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml +++ b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml @@ -151,13 +151,22 @@ spec: OpenstackDataPlaneServiceCert defines the property of a TLS cert issued for a dataplane service properties: + commonName: + description: |- + CommonName overrides how the certificate Common Name is derived. + When set to "system-id", the CN is a UUID5 derived from the node's + ctlplane FQDN, matching the OVN chassis system-id convention. + When empty, CN defaults to the short hostname. + enum: + - system-id + type: string contents: description: |- Contents of the certificate - This is a list of strings for properties that are needed in the cert + This is a list of strings for properties that are needed in the cert. + May be empty for client-only certificates that require no SANs. items: type: string - minItems: 1 type: array edpmRoleServiceName: description: |- @@ -241,8 +250,6 @@ spec: pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$ type: string type: array - required: - - contents type: object description: TLSCerts tls certs to be generated type: object diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml index 8a2aee102c..3b74cba0a9 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_metadata.yaml @@ -24,6 +24,12 @@ spec: - digital signature - key encipherment - client auth + rbac: + commonName: system-id + issuer: osp-rootca-issuer-ovn-rbac + keyUsages: + - digital signature + - client auth caCerts: combined-ca-bundle containerImageFields: - EdpmNeutronMetadataAgentImage diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml index 5b570a34bb..7971d842e9 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_neutron_ovn.yaml @@ -25,6 +25,12 @@ spec: - digital signature - key encipherment - client auth + rbac: + commonName: system-id + issuer: osp-rootca-issuer-ovn-rbac + keyUsages: + - digital signature + - client auth caCerts: combined-ca-bundle containerImageFields: - EdpmNeutronOvnAgentImage diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml index 7dbad97a9c..9b4c481159 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn.yaml @@ -20,6 +20,12 @@ spec: - key encipherment - server auth - client auth + rbac: + commonName: system-id + issuer: osp-rootca-issuer-ovn-rbac + keyUsages: + - digital signature + - client auth caCerts: combined-ca-bundle containerImageFields: - OvnControllerImage diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml index 30af41db37..675bb6f714 100644 --- a/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml +++ b/config/services/dataplane_v1beta1_openstackdataplaneservice_ovn_bgp_agent.yaml @@ -23,6 +23,12 @@ spec: - key encipherment - server auth - client auth + rbac: + commonName: system-id + issuer: osp-rootca-issuer-ovn-rbac + keyUsages: + - digital signature + - client auth caCerts: combined-ca-bundle containerImageFields: - EdpmOvnBgpAgentImage diff --git a/internal/dataplane/cert.go b/internal/dataplane/cert.go index 28d9a3ca9f..43b1b44e6d 100644 --- a/internal/dataplane/cert.go +++ b/internal/dataplane/cert.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/google/uuid" infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/certmanager" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" @@ -43,6 +44,17 @@ import ( dataplanev1 "github.com/openstack-k8s-operators/openstack-operator/api/dataplane/v1beta1" ) +// CommonNameSystemID is the sentinel value for OpenstackDataPlaneServiceCert.CommonName +// that triggers UUID5-based CN derivation matching the OVN chassis system-id convention. +const CommonNameSystemID = "system-id" + +// computeSystemID derives a deterministic UUID5 from a name using the DNS +// namespace, matching ovn-operator's ComputeSystemID() and edpm-ansible's +// {{ name | to_uuid(namespace='6ba7b810-...') }}. +func computeSystemID(name string) string { + return uuid.NewSHA1(uuid.NameSpaceDNS, []byte(name)).String() +} + // Generates an organized data structure that is leveraged to create the secrets. func createSecretsDataStructure(secretMaxSize int, certsData map[string][]byte, @@ -180,7 +192,12 @@ func EnsureTLSCerts(ctx context.Context, helper *helper.Helper, nodeName) } - commonName := strings.Split(baseName, ".")[0] + var commonName string + if service.Spec.TLSCerts[certKey].CommonName == CommonNameSystemID { + commonName = computeSystemID(baseName) + } else { + commonName = strings.Split(baseName, ".")[0] + } certSecret, result, err = GetTLSNodeCert(ctx, helper, instance, certName, issuer, labels, commonName, hosts, ips, service.Spec.TLSCerts[certKey].KeyUsages) diff --git a/internal/dataplane/cert_test.go b/internal/dataplane/cert_test.go new file mode 100644 index 0000000000..523010988c --- /dev/null +++ b/internal/dataplane/cert_test.go @@ -0,0 +1,110 @@ +package deployment + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestComputeSystemID(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "short hostname", + input: "edpm-compute-0", + expected: computeSystemID("edpm-compute-0"), + }, + { + name: "FQDN", + input: "edpm-compute-0.ctlplane.example.com", + expected: computeSystemID("edpm-compute-0.ctlplane.example.com"), + }, + { + name: "deterministic: same input always yields same output", + input: "edpm-compute-0", + }, + { + name: "different inputs yield different outputs", + input: "edpm-compute-1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := computeSystemID(tt.input) + + // Must be non-empty + assert.NotEmpty(t, result) + + // Must be deterministic + assert.Equal(t, result, computeSystemID(tt.input), + "computeSystemID must be deterministic") + + if tt.expected != "" { + assert.Equal(t, tt.expected, result) + } + }) + } + + // Different inputs must produce different UUIDs + id0 := computeSystemID("edpm-compute-0") + id1 := computeSystemID("edpm-compute-1") + assert.NotEqual(t, id0, id1, + "different hostnames must produce different system IDs") + + // Verify format is a valid UUID (8-4-4-4-12 hex) + assert.Regexp(t, `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + computeSystemID("test-node"), + "computeSystemID must return a valid UUID string") +} + +func TestCreateSecretsDataStructure(t *testing.T) { + tests := []struct { + name string + secretMaxSize int + certsData map[string][]byte + expectedChunks int + }{ + { + name: "single node fits in one secret", + secretMaxSize: 1048576, + certsData: map[string][]byte{ + "node1-ca.crt": []byte("ca-cert-data"), + "node1-tls.crt": []byte("tls-cert-data"), + "node1-tls.key": []byte("tls-key-data"), + }, + expectedChunks: 1, + }, + { + name: "small max size forces multiple secrets", + secretMaxSize: 1, + certsData: map[string][]byte{ + "node1-ca.crt": []byte("ca-cert-data"), + "node1-tls.crt": []byte("tls-cert-data"), + "node1-tls.key": []byte("tls-key-data"), + "node2-ca.crt": []byte("ca-cert-data"), + "node2-tls.crt": []byte("tls-cert-data"), + "node2-tls.key": []byte("tls-key-data"), + }, + expectedChunks: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := createSecretsDataStructure(tt.secretMaxSize, tt.certsData) + assert.Equal(t, tt.expectedChunks, len(result)) + + // Verify all data is present across chunks + totalKeys := 0 + for _, chunk := range result { + totalKeys += len(chunk) + } + assert.Equal(t, len(tt.certsData), totalKeys, + "all cert data must be present across chunks") + }) + } +} diff --git a/test/functional/dataplane/openstackdataplaneservice_controller_test.go b/test/functional/dataplane/openstackdataplaneservice_controller_test.go index 3056687617..8f9dab8bf5 100644 --- a/test/functional/dataplane/openstackdataplaneservice_controller_test.go +++ b/test/functional/dataplane/openstackdataplaneservice_controller_test.go @@ -18,6 +18,7 @@ package functional import ( "os" + certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports "k8s.io/apimachinery/pkg/types" @@ -63,4 +64,63 @@ var _ = Describe("OpenstackDataplaneService Test", func() { Expect(service.Spec.DeployOnAllNodeSets).To(BeTrue()) }) }) + + When("A service with TLSCerts including system-id CommonName is created", func() { + BeforeEach(func() { + _ = os.Unsetenv("OPERATOR_SERVICES") + DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec( + dataplaneServiceName, + map[string]interface{}{ + "edpmServiceType": "ovn", + "tlsCerts": map[string]interface{}{ + "default": map[string]interface{}{ + "contents": []string{"dnsnames", "ips"}, + "issuer": "osp-rootca-issuer-ovn", + "keyUsages": []string{ + "digital signature", + "key encipherment", + "server auth", + "client auth", + }, + }, + "rbac": map[string]interface{}{ + "commonName": "system-id", + "issuer": "osp-rootca-issuer-ovn-rbac", + "keyUsages": []string{ + "digital signature", + "client auth", + }, + }, + }, + })) + DeferCleanup(th.DeleteService, dataplaneServiceName) + }) + + It("should store TLSCerts with CommonName and empty Contents", func() { + service := GetService(dataplaneServiceName) + + Expect(service.Spec.TLSCerts).To(HaveLen(2)) + + defaultCert := service.Spec.TLSCerts["default"] + Expect(defaultCert.Contents).To(ConsistOf("dnsnames", "ips")) + Expect(defaultCert.Issuer).To(Equal("osp-rootca-issuer-ovn")) + Expect(defaultCert.CommonName).To(BeEmpty()) + Expect(defaultCert.KeyUsages).To(ContainElements( + certmgrv1.UsageServerAuth, + certmgrv1.UsageClientAuth, + )) + + rbacCert := service.Spec.TLSCerts["rbac"] + Expect(rbacCert.CommonName).To(Equal("system-id")) + Expect(rbacCert.Contents).To(BeEmpty()) + Expect(rbacCert.Issuer).To(Equal("osp-rootca-issuer-ovn-rbac")) + Expect(rbacCert.KeyUsages).To(ContainElements( + certmgrv1.UsageDigitalSignature, + certmgrv1.UsageClientAuth, + )) + Expect(rbacCert.KeyUsages).ToNot(ContainElement( + certmgrv1.UsageServerAuth, + )) + }) + }) })