Skip to content

Novncproxy cert cleanup #1410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Jun 4, 2025
Merged

Novncproxy cert cleanup #1410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apis/core/v1beta1/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"

Expand Down
13 changes: 13 additions & 0 deletions controllers/core/openstackcontrolplane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment thread
auniyal61 marked this conversation as resolved.

instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneCertCleanupReadyCondition)

return ctrl.Result{}, nil
}

Expand Down
89 changes: 89 additions & 0 deletions pkg/openstack/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package openstack

import (
"context"
"errors"
"fmt"
"strings"
"time"

certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Comment thread
auniyal61 marked this conversation as resolved.

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) {
Comment thread
stuggi marked this conversation as resolved.
if object.CheckOwnerRefExist(instance.GetUID(), cert.OwnerReferences) {
Comment thread
auniyal61 marked this conversation as resolved.
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
}
97 changes: 56 additions & 41 deletions tests/functional/ctlplane/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down
Loading