diff --git a/apis/core/v1beta1/conditions.go b/apis/core/v1beta1/conditions.go index 5d72f75d8e..b7fad7d5cc 100644 --- a/apis/core/v1beta1/conditions.go +++ b/apis/core/v1beta1/conditions.go @@ -149,6 +149,9 @@ const ( // OpenStackControlPlaneInstanceHaCMReadyCondition Status=True condition which indicates if InstanceHa CM is ready OpenStackControlPlaneInstanceHaCMReadyCondition condition.Type = "OpenStackControlPlaneInstanceHaCMReadyCondition" + + // OpenStackControlPlaneCertCleanupReadyCondition Status=True condition which indicates global certification cleanup is Ready + OpenStackControlPlaneCertCleanupReadyCondition condition.Type = "OpenStackControlPlaneCertCleanupReadyCondition" ) // Common Messages used by API objects. @@ -280,6 +283,9 @@ const ( // OpenStackControlPlaneNovaReadyErrorMessage OpenStackControlPlaneNovaReadyErrorMessage = "OpenStackControlPlane Nova error occured %s" + // OpenStackControlPlaneCertCleanupReadyErrorMessage + OpenStackControlPlaneCertCleanupReadyErrorMessage = "OpenStackControlPlane Cert cleanup error occured %s" + // OpenStackControlPlaneHeatReadyInitMessage OpenStackControlPlaneHeatReadyInitMessage = "OpenStackControlPlane Heat not started" diff --git a/controllers/core/openstackcontrolplane_controller.go b/controllers/core/openstackcontrolplane_controller.go index 216a188e39..7d2ac06990 100644 --- a/controllers/core/openstackcontrolplane_controller.go +++ b/controllers/core/openstackcontrolplane_controller.go @@ -528,6 +528,19 @@ func (r *OpenStackControlPlaneReconciler) reconcileNormal(ctx context.Context, i return ctrlResult, nil } + ctrlResult, errs := openstack.DeleteCertsAndRoutes(ctx, instance, helper) + if errs != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + corev1beta1.OpenStackControlPlaneCertCleanupReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + corev1beta1.OpenStackControlPlaneCertCleanupReadyErrorMessage, + errs)) + return ctrlResult, errs + } + + instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneCertCleanupReadyCondition) + return ctrl.Result{}, nil } diff --git a/pkg/openstack/common.go b/pkg/openstack/common.go index 9686559506..b1658bcc06 100644 --- a/pkg/openstack/common.go +++ b/pkg/openstack/common.go @@ -2,7 +2,9 @@ package openstack import ( "context" + "errors" "fmt" + "strings" "time" certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -22,6 +24,7 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/clusterdns" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + "github.com/openstack-k8s-operators/lib-common/modules/common/object" "github.com/openstack-k8s-operators/lib-common/modules/common/route" "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/service" @@ -33,6 +36,9 @@ import ( novav1 "github.com/openstack-k8s-operators/nova-operator/api/v1beta1" octaviav1 "github.com/openstack-k8s-operators/octavia-operator/api/v1beta1" corev1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1" + + // corev1 "k8s.io/api/core/v1" + corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1" ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1" placementv1 "github.com/openstack-k8s-operators/placement-operator/api/v1beta1" swiftv1 "github.com/openstack-k8s-operators/swift-operator/api/v1beta1" @@ -805,6 +811,15 @@ func hasCertInOverrideSpec(overrideSpec route.OverrideSpec) bool { overrideSpec.Spec.TLS.Key != "" } +func serviceExists(route string, services *k8s_corev1.ServiceList) bool { + for _, svc := range services.Items { + if svc.Name == route { + return true + } + } + return false +} + func DeleteCertificate( ctx context.Context, helper *helper.Helper, @@ -824,3 +839,77 @@ func DeleteCertificate( helper.GetLogger().Info(fmt.Sprintf("Deleting cert %s", certName)) return cert.Delete(ctx, helper) } + +func DeleteCertsAndRoutes( + ctx context.Context, + instance *corev1beta1.OpenStackControlPlane, + helper *helper.Helper, +) (ctrl.Result, error) { + + log := GetLogger(ctx) + + // Retrieve all routes, certs and services in the namespace + routes, err := GetRoutesListWithLabel(ctx, helper, instance.Namespace, nil) + if err != nil { + return ctrl.Result{}, fmt.Errorf("could not get routes: %w", err) + } + + certs := &certmgrv1.CertificateList{} + if err := helper.GetClient().List(ctx, certs, client.InNamespace(instance.Namespace)); err != nil { + return ctrl.Result{}, fmt.Errorf("could not get certificates: %w", err) + } + + services := &k8s_corev1.ServiceList{} + if err := helper.GetClient().List(ctx, services, client.InNamespace(instance.Namespace)); err != nil { + return ctrl.Result{}, fmt.Errorf("could not get services: %w", err) + } + + var delErrs []error + for _, route := range routes.Items { + + if !object.CheckOwnerRefExist(instance.GetUID(), route.OwnerReferences) { + continue + } + + if serviceExists(route.Spec.To.Name, services) { + continue + } + + // Delete certs by service and route-name + for _, cert := range certs.Items { + if _, ok := cert.Labels[serviceCertSelector]; ok && strings.Contains(cert.Name, route.Name) { + if object.CheckOwnerRefExist(instance.GetUID(), cert.OwnerReferences) { + log.Info("Deleting certificate", ":", cert.Name) + err := DeleteCertificate(ctx, helper, instance.Namespace, cert.Name) + if err != nil { + delErrs = append(delErrs, fmt.Errorf("cert deletion for '%s' failed, because: %w", cert.Name, err)) + } + } + } + + // NOTE(auniyal): This is specifically to cleanup novncproxy certs, others service certs do not use commonName as of now + // TODO: this can be removed once we can map service, route and certs with `osctlplane-service` label + if strings.Contains(cert.Spec.CommonName, route.Name) { + if object.CheckOwnerRefExist(instance.GetUID(), cert.OwnerReferences) { + log.Info("Deleting certificate", ":", cert.Name) + err := DeleteCertificate(ctx, helper, instance.Namespace, cert.Name) + if err != nil { + delErrs = append(delErrs, fmt.Errorf("cert deletion for '%s' failed, because: %w", cert.Name, err)) + } + } + } + } + + log.Info("Deleting route", ":", route.Name) + _, err := EnsureDeleted(ctx, helper, &route) + if err != nil { + delErrs = append(delErrs, fmt.Errorf("route deletion for '%s' failed, because: %w", route.Name, err)) + } + } + + if len(delErrs) > 0 { + return ctrl.Result{}, errors.Join(delErrs...) + } + + return ctrl.Result{}, nil +} diff --git a/tests/functional/ctlplane/base_test.go b/tests/functional/ctlplane/base_test.go index 0a5ecfa7e2..c2d8b22788 100644 --- a/tests/functional/ctlplane/base_test.go +++ b/tests/functional/ctlplane/base_test.go @@ -44,47 +44,50 @@ import ( ) type Names struct { - Namespace string - OpenStackControlplaneName types.NamespacedName - OpenStackVersionName types.NamespacedName - KeystoneAPIName types.NamespacedName - MemcachedName types.NamespacedName - MemcachedCertName types.NamespacedName - CinderName types.NamespacedName - ManilaName types.NamespacedName - GlanceName types.NamespacedName - NeutronName types.NamespacedName - HorizonName types.NamespacedName - HeatName types.NamespacedName - TelemetryName types.NamespacedName - DBName types.NamespacedName - DBCertName types.NamespacedName - DBCell1Name types.NamespacedName - DBCell1CertName types.NamespacedName - RabbitMQName types.NamespacedName - RabbitMQCertName types.NamespacedName - RabbitMQCell1Name types.NamespacedName - RabbitMQCell1CertName types.NamespacedName - ServiceAccountName types.NamespacedName - RoleName types.NamespacedName - RoleBindingName types.NamespacedName - RootCAPublicName types.NamespacedName - RootCAInternalName types.NamespacedName - RootCAOvnName types.NamespacedName - RootCALibvirtName types.NamespacedName - SelfSignedIssuerName types.NamespacedName - CustomIssuerName types.NamespacedName - CustomServiceCertSecretName types.NamespacedName - CABundleName types.NamespacedName - OpenStackClientName types.NamespacedName - OVNNorthdName types.NamespacedName - OVNNorthdCertName types.NamespacedName - OVNControllerName types.NamespacedName - OVNControllerCertName types.NamespacedName - OVNDbServerNBName types.NamespacedName - OVNDbServerSBName types.NamespacedName - NeutronOVNCertName types.NamespacedName - OpenStackTopology []types.NamespacedName + Namespace string + OpenStackControlplaneName types.NamespacedName + OpenStackVersionName types.NamespacedName + KeystoneAPIName types.NamespacedName + MemcachedName types.NamespacedName + MemcachedCertName types.NamespacedName + CinderName types.NamespacedName + ManilaName types.NamespacedName + GlanceName types.NamespacedName + NeutronName types.NamespacedName + HorizonName types.NamespacedName + HeatName types.NamespacedName + TelemetryName types.NamespacedName + DBName types.NamespacedName + DBCertName types.NamespacedName + DBCell1Name types.NamespacedName + DBCell1CertName types.NamespacedName + RabbitMQName types.NamespacedName + RabbitMQCertName types.NamespacedName + RabbitMQCell1Name types.NamespacedName + RabbitMQCell1CertName types.NamespacedName + NoVNCProxyCell1CertPublicRouteName types.NamespacedName + NoVNCProxyCell1CertPublicSvcName types.NamespacedName + NoVNCProxyCell1CertVencryptName types.NamespacedName + ServiceAccountName types.NamespacedName + RoleName types.NamespacedName + RoleBindingName types.NamespacedName + RootCAPublicName types.NamespacedName + RootCAInternalName types.NamespacedName + RootCAOvnName types.NamespacedName + RootCALibvirtName types.NamespacedName + SelfSignedIssuerName types.NamespacedName + CustomIssuerName types.NamespacedName + CustomServiceCertSecretName types.NamespacedName + CABundleName types.NamespacedName + OpenStackClientName types.NamespacedName + OVNNorthdName types.NamespacedName + OVNNorthdCertName types.NamespacedName + OVNControllerName types.NamespacedName + OVNControllerCertName types.NamespacedName + OVNDbServerNBName types.NamespacedName + OVNDbServerSBName types.NamespacedName + NeutronOVNCertName types.NamespacedName + OpenStackTopology []types.NamespacedName } func CreateNames(openstackControlplaneName types.NamespacedName) Names { @@ -200,6 +203,18 @@ func CreateNames(openstackControlplaneName types.NamespacedName) Names { Namespace: openstackControlplaneName.Namespace, Name: "cert-rabbitmq-cell1-svc", }, + NoVNCProxyCell1CertPublicRouteName: types.NamespacedName{ + Name: "cert-nova-novncproxy-cell1-public-route", + Namespace: openstackControlplaneName.Namespace, + }, + NoVNCProxyCell1CertPublicSvcName: types.NamespacedName{ + Name: "cert-nova-novncproxy-cell1-public-svc", + Namespace: openstackControlplaneName.Namespace, + }, + NoVNCProxyCell1CertVencryptName: types.NamespacedName{ + Name: "cert-nova-novncproxy-cell1-vencrypt", + Namespace: openstackControlplaneName.Namespace, + }, OpenStackClientName: types.NamespacedName{ Namespace: openstackControlplaneName.Namespace, Name: "openstackclient", diff --git a/tests/functional/ctlplane/openstackoperator_controller_test.go b/tests/functional/ctlplane/openstackoperator_controller_test.go index 80148177d1..c58d51a16b 100644 --- a/tests/functional/ctlplane/openstackoperator_controller_test.go +++ b/tests/functional/ctlplane/openstackoperator_controller_test.go @@ -45,9 +45,11 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" manilav1 "github.com/openstack-k8s-operators/manila-operator/api/v1beta1" + novav1 "github.com/openstack-k8s-operators/nova-operator/api/v1beta1" clientv1 "github.com/openstack-k8s-operators/openstack-operator/apis/client/v1beta1" corev1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1" ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1" + placementv1 "github.com/openstack-k8s-operators/placement-operator/api/v1beta1" ) var _ = Describe("OpenStackOperator controller", func() { @@ -2882,7 +2884,7 @@ var _ = Describe("OpenStackOperator Webhook", func() { }) }) -var _ = Describe("OpenStackOperator controller galera and rabbitmq", func() { +var _ = Describe("OpenStackOperator controller nova cell deletion", func() { BeforeEach(func() { // lib-common uses OPERATOR_TEMPLATES env var to locate the "templates" // directory of the operator. We need to set them othervise lib-common @@ -2915,6 +2917,14 @@ var _ = Describe("OpenStackOperator controller galera and rabbitmq", func() { th.CreateCertSecret(names.RabbitMQCertName) th.CreateCertSecret(names.RabbitMQCell1CertName) + // 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.NeutronOVNCertName)) + + // create cert secrets for memcached instance + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.MemcachedCertName)) + extGalera := CreateGaleraConfig(namespace, GetDefaultGaleraSpec()) extGaleraName.Name = extGalera.GetName() extGaleraName.Namespace = extGalera.GetNamespace() @@ -2933,6 +2943,13 @@ var _ = Describe("OpenStackOperator controller galera and rabbitmq", func() { th.DeleteInstance, CreateOpenStackControlPlane(names.OpenStackControlplaneName, spec), ) + + // enable TLS + Eventually(func(g Gomega) { + OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) + OSCtlplane.Spec.TLS.PodLevel.Enabled = true + g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) }) It("cell1 galera should be deleted from CR", func() { @@ -3015,13 +3032,6 @@ var _ = Describe("OpenStackOperator controller galera and rabbitmq", func() { var secretName types.NamespacedName var certName types.NamespacedName - // enable TLS - Eventually(func(g Gomega) { - OSCtlplane = GetOpenStackControlPlane(names.OpenStackControlplaneName) - OSCtlplane.Spec.TLS.PodLevel.Enabled = true - g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed()) - }, timeout, interval).Should(Succeed()) - // rabbitmq exists Eventually(func(g Gomega) { rabbitmq := GetRabbitMQCluster(names.RabbitMQCell1Name) @@ -3086,6 +3096,133 @@ var _ = Describe("OpenStackOperator controller galera and rabbitmq", func() { }) - }) + When("The novncproxy k8s service is created for cell1", func() { + /* + - generate certs and routes for novncproxy + - enable nova and dependencies + - create novncproxy service + - find and verify certs and routes are created + - reproduce cell1 deletion + - remove cell1 from oscp CR + - delete novncproxy service + - verify if there are no residue certs and routes + */ + + BeforeEach(func() { + // enable Nova and dependencies + Eventually(func(g Gomega) { + OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) + OSCtlplane.Spec.Nova.Enabled = true + OSCtlplane.Spec.Nova.Template = &novav1.NovaSpecCore{} + // enable "Galera, Memcached, RabbitMQ, Keystone, Glance, Neutron, Placement" too + + OSCtlplane.Spec.Keystone.Enabled = true + OSCtlplane.Spec.Glance.Enabled = true + OSCtlplane.Spec.Neutron.Enabled = true + OSCtlplane.Spec.Placement.Enabled = true + + if OSCtlplane.Spec.Placement.Template == nil { + OSCtlplane.Spec.Placement.Template = &placementv1.PlacementAPISpecCore{} + OSCtlplane.Spec.Placement.Template.APITimeout = 10 + } + g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + // nova-novncproxy-cell1-public + novncProxyPublicSvcName := types.NamespacedName{ + Name: "nova-novncproxy-cell1-public", + Namespace: namespace} + + th.CreateService( + novncProxyPublicSvcName, + map[string]string{ + "osctlplane-service": "nova-novncproxy", + "osctlplane": "", + "cell": "cell1", + }, + k8s_corev1.ServiceSpec{ + Ports: []k8s_corev1.ServicePort{ + { + Name: "nova-novncproxy-cell1-public", + Port: int32(6080), + Protocol: k8s_corev1.ProtocolTCP, + }, + }, + }) + + novncProxySvc := th.GetService(novncProxyPublicSvcName) + + if novncProxySvc.Annotations == nil { + novncProxySvc.Annotations = map[string]string{} + } + + novncProxySvc.Annotations[service.AnnotationIngressCreateKey] = "true" + novncProxySvc.Annotations[service.AnnotationEndpointKey] = "public" + + Expect(th.K8sClient.Status().Update(th.Ctx, novncProxySvc)).To(Succeed()) + // novncProxySvc = th.GetService(novncProxyPublicSvcName) + // logger.Info("", "XXX novncproxy labels", novncProxySvc.Labels) + + // vnproxy certs + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.NoVNCProxyCell1CertPublicRouteName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.NoVNCProxyCell1CertPublicSvcName)) + DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.NoVNCProxyCell1CertVencryptName)) + + }) + + It("novncproxy certs and routes should be deleted on respective cell deletion", func() { + certNames := []types.NamespacedName{ + {Name: "nova-novncproxy-cell1-public-route", Namespace: namespace}, + {Name: "nova-novncproxy-cell1-public-svc", Namespace: namespace}, + {Name: "nova-novncproxy-cell1-vencrypt", Namespace: namespace}, + } + + // verify all certs for novncproxy exists + Eventually(func(g Gomega) { + for _, certName := range certNames { + cert := crtmgr.GetCert(certName) + g.Expect(cert).NotTo(BeNil()) + } + }, timeout, interval).Should(Succeed()) + + // verify route is present + Eventually(func(g Gomega) { + novncproxyRouteName := types.NamespacedName{Name: "nova-novncproxy-cell1-public", Namespace: namespace} + novncproxyRoute := &routev1.Route{} + + g.Expect(th.K8sClient.Get(th.Ctx, novncproxyRouteName, novncproxyRoute)).Should(Succeed()) + g.Expect(novncproxyRoute.Spec.TLS.Certificate).Should(Not(BeEmpty())) + g.Expect(novncproxyRoute.Spec.TLS.Key).Should(Not(BeEmpty())) + g.Expect(novncproxyRoute.Spec.TLS.CACertificate).Should(Not(BeEmpty())) + }, timeout, interval).Should(Succeed()) + + // remove from oscp + Eventually(func(g Gomega) { + OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) + delete(OSCtlplane.Spec.Nova.Template.CellTemplates, "cell1") + g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + th.DeleteService( + types.NamespacedName{ + Name: "nova-novncproxy-cell1-public", + Namespace: namespace, + }) + + // verify all certs for novncproxy + for _, certName := range certNames { + crtmgr.AssertCertDoesNotExist(certName) + } + + // verify route for novncproxy + novncproxyRouteName := types.NamespacedName{Name: "nova-novncproxy-cell1-public", Namespace: namespace} + novncproxyRoute := &routev1.Route{} + Eventually(func(g Gomega) { + err := th.K8sClient.Get(th.Ctx, novncproxyRouteName, novncproxyRoute) + g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) + }) + + }) + }) })