Skip to content

Commit 5d4d6d5

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #790 from stuggi/OSPRH-6746
[TLS] Fix issuer labels when switch to custom issuer
2 parents 84bbcef + 2cd527e commit 5d4d6d5

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)