Skip to content

Commit 2cd527e

Browse files
stuggistuggi
committed
[TLS] Fix issuer labels when switch to custom issuer
Right now when first deploy with default TLS settings and later switch to a custom issuer, we'll end up with two issuers to have the CA identifier label. As a result the dataplane operator is waiting as there are more then one issuer identified. This changes to remove the identifier label on all issuers, except the one configured for the current issuer. This allows to switch * from default issuer to customer * from custom issuer back to default * and from custom issuer to a different custom issuer Jira: OSPRH-6746
1 parent 84bbcef commit 2cd527e

File tree

3 files changed

+191
-13
lines changed

3 files changed

+191
-13
lines changed

‎apis/core/v1beta1/openstackcontrolplane_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ const (
5454
// RabbitMqContainerImage is the fall-back container image for RabbitMQ
5555
RabbitMqContainerImage = "quay.io/podified-antelope-centos9/openstack-rabbitmq:current-podified"
5656

57+
// IngressCaName -
58+
IngressCaName = tls.DefaultCAPrefix + string(service.EndpointPublic)
59+
// InternalCaName -
60+
InternalCaName = tls.DefaultCAPrefix + string(service.EndpointInternal)
5761
// OvnDbCaName -
5862
OvnDbCaName = tls.DefaultCAPrefix + "ovn"
5963
// LibvirtCaName -

‎pkg/openstack/ca.go‎

Lines changed: 123 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
1818
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
1919
"github.com/openstack-k8s-operators/lib-common/modules/common/secret"
20-
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
2120
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2221
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
2322
"golang.org/x/exp/slices"
@@ -100,12 +99,24 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
10099
issuerLabels := map[string]string{certmanager.RootCAIssuerPublicLabel: ""}
101100
issuerAnnotations := getIssuerAnnotations(&instance.Spec.TLS.Ingress.Cert)
102101
if !instance.Spec.TLS.Ingress.Ca.IsCustomIssuer() {
102+
// remove issuerLabels from any custom issuer in the namespace.
103+
err := removeIssuerLabel(
104+
ctx,
105+
helper,
106+
corev1.IngressCaName,
107+
instance.Namespace,
108+
issuerLabels,
109+
)
110+
if err != nil {
111+
return ctrl.Result{}, err
112+
}
113+
103114
ctrlResult, err = ensureRootCA(
104115
ctx,
105116
instance,
106117
helper,
107118
issuerReq,
108-
tls.DefaultCAPrefix+string(service.EndpointPublic),
119+
corev1.IngressCaName,
109120
issuerLabels,
110121
issuerAnnotations,
111122
bundle,
@@ -166,12 +177,24 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
166177
issuerLabels = map[string]string{certmanager.RootCAIssuerInternalLabel: ""}
167178
issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Internal.Cert)
168179
if !instance.Spec.TLS.PodLevel.Internal.Ca.IsCustomIssuer() {
180+
// remove issuerLabels from any custom issuer in the namespace.
181+
err := removeIssuerLabel(
182+
ctx,
183+
helper,
184+
corev1.InternalCaName,
185+
instance.Namespace,
186+
issuerLabels,
187+
)
188+
if err != nil {
189+
return ctrl.Result{}, err
190+
}
191+
169192
ctrlResult, err = ensureRootCA(
170193
ctx,
171194
instance,
172195
helper,
173196
issuerReq,
174-
tls.DefaultCAPrefix+string(service.EndpointInternal),
197+
corev1.InternalCaName,
175198
issuerLabels,
176199
issuerAnnotations,
177200
bundle,
@@ -232,6 +255,18 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
232255
issuerLabels = map[string]string{certmanager.RootCAIssuerLibvirtLabel: ""}
233256
issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Libvirt.Cert)
234257
if !instance.Spec.TLS.PodLevel.Libvirt.Ca.IsCustomIssuer() {
258+
// remove issuerLabels from any custom issuer in the namespace.
259+
err := removeIssuerLabel(
260+
ctx,
261+
helper,
262+
corev1.LibvirtCaName,
263+
instance.Namespace,
264+
issuerLabels,
265+
)
266+
if err != nil {
267+
return ctrl.Result{}, err
268+
}
269+
235270
ctrlResult, err = ensureRootCA(
236271
ctx,
237272
instance,
@@ -297,6 +332,18 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
297332
issuerLabels = map[string]string{certmanager.RootCAIssuerOvnDBLabel: ""}
298333
issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Ovn.Cert)
299334
if !instance.Spec.TLS.PodLevel.Ovn.Ca.IsCustomIssuer() {
335+
// remove issuerLabels from any custom issuer in the namespace.
336+
err := removeIssuerLabel(
337+
ctx,
338+
helper,
339+
corev1.OvnDbCaName,
340+
instance.Namespace,
341+
issuerLabels,
342+
)
343+
if err != nil {
344+
return ctrl.Result{}, err
345+
}
346+
300347
ctrlResult, err = ensureRootCA(
301348
ctx,
302349
instance,
@@ -765,8 +812,21 @@ func addIssuerLabelAnnotation(
765812
labels map[string]string,
766813
annotations map[string]string,
767814
) (string, error) {
815+
// remove issuer labels from all issuers in the namespace,
816+
// except the one passed to the func.
817+
err := removeIssuerLabel(
818+
ctx,
819+
helper,
820+
name,
821+
namespace,
822+
labels,
823+
)
824+
if err != nil {
825+
return "", err
826+
}
827+
768828
var caCertSecretName string
769-
// get issuer
829+
// get issuer
770830
issuer, err := certmanager.GetIssuerByName(
771831
ctx,
772832
helper,
@@ -785,31 +845,83 @@ func addIssuerLabelAnnotation(
785845
// merge annotations
786846
issuer.Annotations = util.MergeMaps(issuer.Annotations, annotations)
787847

848+
err = patchIssuer(ctx, helper, beforeIssuer, issuer)
849+
if err != nil {
850+
return caCertSecretName, err
851+
}
852+
853+
return caCertSecretName, nil
854+
}
855+
856+
// remove issuer labels from all issuers in the namespace,
857+
// except the one passed to the func.
858+
func removeIssuerLabel(
859+
ctx context.Context,
860+
helper *helper.Helper,
861+
name string,
862+
namespace string,
863+
labels map[string]string,
864+
) error {
865+
if len(labels) > 0 {
866+
issuerList := &certmgrv1.IssuerList{}
867+
listOpts := []client.ListOption{
868+
client.InNamespace(namespace),
869+
client.MatchingLabels(labels),
870+
}
871+
872+
err := helper.GetClient().List(ctx, issuerList, listOpts...)
873+
if err != nil {
874+
return fmt.Errorf("error getting issuer by label: %w", err)
875+
}
876+
877+
for _, issuer := range issuerList.Items {
878+
if issuer.Name != name {
879+
beforeIssuer := issuer.DeepCopyObject().(client.Object)
880+
for k := range labels {
881+
delete(issuer.Labels, k)
882+
}
883+
884+
err = patchIssuer(ctx, helper, beforeIssuer, &issuer)
885+
if err != nil {
886+
return err
887+
}
888+
}
889+
}
890+
}
891+
892+
return nil
893+
}
894+
895+
func patchIssuer(
896+
ctx context.Context,
897+
helper *helper.Helper,
898+
beforeIssuer client.Object,
899+
issuer *certmgrv1.Issuer,
900+
) error {
788901
// patch issuer
789902
patch := client.MergeFrom(beforeIssuer)
790903
diff, err := patch.Data(issuer)
791904
if err != nil {
792-
return caCertSecretName, err
905+
return err
793906
}
794-
795907
// Unmarshal patch data into a local map for logging
796908
patchDiff := map[string]interface{}{}
797909
if err := json.Unmarshal(diff, &patchDiff); err != nil {
798-
return caCertSecretName, err
910+
return err
799911
}
800912

801913
if _, ok := patchDiff["metadata"]; ok {
802914
err = helper.GetClient().Patch(ctx, issuer, patch)
803915
if k8s_errors.IsConflict(err) {
804-
return caCertSecretName, fmt.Errorf("error metadata update conflict: %w", err)
916+
return fmt.Errorf("error metadata update conflict: %w", err)
805917
} else if err != nil && !k8s_errors.IsNotFound(err) {
806-
return caCertSecretName, fmt.Errorf("error metadata update failed: %w", err)
918+
return fmt.Errorf("error metadata update failed: %w", err)
807919
}
808920

809-
helper.GetLogger().Info(fmt.Sprintf("Issuer %s labels patched - diff %+v", name, patchDiff["metadata"]))
921+
helper.GetLogger().Info(fmt.Sprintf("Issuer %s labels patched - diff %+v", issuer.Name, patchDiff["metadata"]))
810922
}
811923

812-
return caCertSecretName, nil
924+
return nil
813925
}
814926

815927
func getIssuerAnnotations(certConfig *corev1.CertConfig) map[string]string {

‎tests/functional/openstackoperator_controller_test.go‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
. "github.com/onsi/gomega" //revive:disable:dot-imports
2626

2727
//revive:disable-next-line:dot-imports
28-
"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
2928
. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
3029

3130
k8s_corev1 "k8s.io/api/core/v1"
@@ -37,6 +36,7 @@ import (
3736

3837
routev1 "github.com/openshift/api/route/v1"
3938
cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1"
39+
"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
4040
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
4141
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
4242
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
@@ -765,7 +765,7 @@ var _ = Describe("OpenStackOperator controller", func() {
765765
Expect(OSCtlplane.Spec.Neutron.APIOverride.Route.Annotations).Should(HaveKeyWithValue("haproxy.router.openshift.io/timeout", "120s"))
766766
})
767767

768-
It("should create selfsigned issuer and public+internal CA and issuer", func() {
768+
It("should create selfsigned issuer and public, internal, libvirt and ovn CA and issuer", func() {
769769
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
770770

771771
Expect(OSCtlplane.Spec.TLS.Ingress.Enabled).Should(BeTrue())
@@ -789,6 +789,7 @@ var _ = Describe("OpenStackOperator controller", func() {
789789
issuer := crtmgr.GetIssuer(names.RootCAPublicName)
790790
g.Expect(issuer).Should(Not(BeNil()))
791791
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAPublicName.Name))
792+
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerPublicLabel))
792793
}, timeout, interval).Should(Succeed())
793794
Eventually(func(g Gomega) {
794795
// ca cert
@@ -802,6 +803,7 @@ var _ = Describe("OpenStackOperator controller", func() {
802803
issuer := crtmgr.GetIssuer(names.RootCAInternalName)
803804
g.Expect(issuer).Should(Not(BeNil()))
804805
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAInternalName.Name))
806+
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerInternalLabel))
805807
}, timeout, interval).Should(Succeed())
806808
Eventually(func(g Gomega) {
807809
// ca cert
@@ -815,6 +817,7 @@ var _ = Describe("OpenStackOperator controller", func() {
815817
issuer := crtmgr.GetIssuer(names.RootCAOvnName)
816818
g.Expect(issuer).Should(Not(BeNil()))
817819
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAOvnName.Name))
820+
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerOvnDBLabel))
818821
}, timeout, interval).Should(Succeed())
819822
Eventually(func(g Gomega) {
820823
// ca cert
@@ -828,6 +831,7 @@ var _ = Describe("OpenStackOperator controller", func() {
828831
issuer := crtmgr.GetIssuer(names.RootCALibvirtName)
829832
g.Expect(issuer).Should(Not(BeNil()))
830833
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCALibvirtName.Name))
834+
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerLibvirtLabel))
831835
}, timeout, interval).Should(Succeed())
832836

833837
th.ExpectCondition(
@@ -926,6 +930,64 @@ var _ = Describe("OpenStackOperator controller", func() {
926930
)
927931
}, timeout, interval).Should(Succeed())
928932
})
933+
934+
When("The TLSe OpenStackControlplane instance switches to use a custom public issuer", func() {
935+
BeforeEach(func() {
936+
// create custom issuer
937+
DeferCleanup(k8sClient.Delete, ctx, crtmgr.CreateIssuer(names.CustomIssuerName))
938+
DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.CustomIssuerName))
939+
940+
// update ctlplane to use the custom isssuer
941+
Eventually(func(g Gomega) {
942+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
943+
OSCtlplane.Spec.TLS.Ingress.Ca.CustomIssuer = ptr.To(names.CustomIssuerName.Name)
944+
g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed())
945+
}, timeout, interval).Should(Succeed())
946+
})
947+
948+
It("should remove the certmanager.RootCAIssuerPublicLabel label from the defaultIssuer", func() {
949+
Eventually(func(g Gomega) {
950+
issuer := crtmgr.GetIssuer(names.RootCAPublicName)
951+
g.Expect(issuer).Should(Not(BeNil()))
952+
g.Expect(issuer.Labels).Should(Not(HaveKey(certmanager.RootCAIssuerPublicLabel)))
953+
}, timeout, interval).Should(Succeed())
954+
})
955+
956+
It("should add the certmanager.RootCAIssuerPublicLabel label to the customIssuer", func() {
957+
Eventually(func(g Gomega) {
958+
issuer := crtmgr.GetIssuer(names.CustomIssuerName)
959+
g.Expect(issuer).Should(Not(BeNil()))
960+
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerPublicLabel))
961+
}, timeout, interval).Should(Succeed())
962+
})
963+
964+
When("The TLSe OpenStackControlplane instance switches again back to default public issuer", func() {
965+
BeforeEach(func() {
966+
// update ctlplane to NOT use the custom isssuer
967+
Eventually(func(g Gomega) {
968+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
969+
OSCtlplane.Spec.TLS.Ingress.Ca.CustomIssuer = nil
970+
g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed())
971+
}, timeout, interval).Should(Succeed())
972+
})
973+
974+
It("should remove the certmanager.RootCAIssuerPublicLabel label from the defaultIssuer", func() {
975+
Eventually(func(g Gomega) {
976+
issuer := crtmgr.GetIssuer(names.RootCAPublicName)
977+
g.Expect(issuer).Should(Not(BeNil()))
978+
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerPublicLabel))
979+
}, timeout, interval).Should(Succeed())
980+
})
981+
982+
It("should add the certmanager.RootCAIssuerPublicLabel label to the customIssuer", func() {
983+
Eventually(func(g Gomega) {
984+
issuer := crtmgr.GetIssuer(names.CustomIssuerName)
985+
g.Expect(issuer).Should(Not(BeNil()))
986+
g.Expect(issuer.Labels).Should(Not(HaveKey(certmanager.RootCAIssuerPublicLabel)))
987+
}, timeout, interval).Should(Succeed())
988+
})
989+
})
990+
})
929991
})
930992

931993
When("A TLSe OpenStackControlplane instance with custom public issuer is created", func() {

0 commit comments

Comments
 (0)