Skip to content

Commit

Permalink
Merge pull request #790 from stuggi/OSPRH-6746
Browse files Browse the repository at this point in the history 
[TLS] Fix issuer labels when switch to custom issuer
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored May 23, 2024
2 parents 84bbcef + 2cd527e commit 5d4d6d5
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 13 deletions.
4 changes: 4 additions & 0 deletions apis/core/v1beta1/openstackcontrolplane_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ const (
// RabbitMqContainerImage is the fall-back container image for RabbitMQ
RabbitMqContainerImage = "quay.io/podified-antelope-centos9/openstack-rabbitmq:current-podified"

// IngressCaName -
IngressCaName = tls.DefaultCAPrefix + string(service.EndpointPublic)
// InternalCaName -
InternalCaName = tls.DefaultCAPrefix + string(service.EndpointInternal)
// OvnDbCaName -
OvnDbCaName = tls.DefaultCAPrefix + "ovn"
// LibvirtCaName -
Expand Down
134 changes: 123 additions & 11 deletions pkg/openstack/ca.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"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/secret"
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
"golang.org/x/exp/slices"
Expand Down Expand Up @@ -100,12 +99,24 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
issuerLabels := map[string]string{certmanager.RootCAIssuerPublicLabel: ""}
issuerAnnotations := getIssuerAnnotations(&instance.Spec.TLS.Ingress.Cert)
if !instance.Spec.TLS.Ingress.Ca.IsCustomIssuer() {
// remove issuerLabels from any custom issuer in the namespace.
err := removeIssuerLabel(
ctx,
helper,
corev1.IngressCaName,
instance.Namespace,
issuerLabels,
)
if err != nil {
return ctrl.Result{}, err
}

ctrlResult, err = ensureRootCA(
ctx,
instance,
helper,
issuerReq,
tls.DefaultCAPrefix+string(service.EndpointPublic),
corev1.IngressCaName,
issuerLabels,
issuerAnnotations,
bundle,
Expand Down Expand Up @@ -166,12 +177,24 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
issuerLabels = map[string]string{certmanager.RootCAIssuerInternalLabel: ""}
issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Internal.Cert)
if !instance.Spec.TLS.PodLevel.Internal.Ca.IsCustomIssuer() {
// remove issuerLabels from any custom issuer in the namespace.
err := removeIssuerLabel(
ctx,
helper,
corev1.InternalCaName,
instance.Namespace,
issuerLabels,
)
if err != nil {
return ctrl.Result{}, err
}

ctrlResult, err = ensureRootCA(
ctx,
instance,
helper,
issuerReq,
tls.DefaultCAPrefix+string(service.EndpointInternal),
corev1.InternalCaName,
issuerLabels,
issuerAnnotations,
bundle,
Expand Down Expand Up @@ -232,6 +255,18 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
issuerLabels = map[string]string{certmanager.RootCAIssuerLibvirtLabel: ""}
issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Libvirt.Cert)
if !instance.Spec.TLS.PodLevel.Libvirt.Ca.IsCustomIssuer() {
// remove issuerLabels from any custom issuer in the namespace.
err := removeIssuerLabel(
ctx,
helper,
corev1.LibvirtCaName,
instance.Namespace,
issuerLabels,
)
if err != nil {
return ctrl.Result{}, err
}

ctrlResult, err = ensureRootCA(
ctx,
instance,
Expand Down Expand Up @@ -297,6 +332,18 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
issuerLabels = map[string]string{certmanager.RootCAIssuerOvnDBLabel: ""}
issuerAnnotations = getIssuerAnnotations(&instance.Spec.TLS.PodLevel.Ovn.Cert)
if !instance.Spec.TLS.PodLevel.Ovn.Ca.IsCustomIssuer() {
// remove issuerLabels from any custom issuer in the namespace.
err := removeIssuerLabel(
ctx,
helper,
corev1.OvnDbCaName,
instance.Namespace,
issuerLabels,
)
if err != nil {
return ctrl.Result{}, err
}

ctrlResult, err = ensureRootCA(
ctx,
instance,
Expand Down Expand Up @@ -765,8 +812,21 @@ func addIssuerLabelAnnotation(
labels map[string]string,
annotations map[string]string,
) (string, error) {
// remove issuer labels from all issuers in the namespace,
// except the one passed to the func.
err := removeIssuerLabel(
ctx,
helper,
name,
namespace,
labels,
)
if err != nil {
return "", err
}

var caCertSecretName string
// get issuer
// get issuer
issuer, err := certmanager.GetIssuerByName(
ctx,
helper,
Expand All @@ -785,31 +845,83 @@ func addIssuerLabelAnnotation(
// merge annotations
issuer.Annotations = util.MergeMaps(issuer.Annotations, annotations)

err = patchIssuer(ctx, helper, beforeIssuer, issuer)
if err != nil {
return caCertSecretName, err
}

return caCertSecretName, nil
}

// remove issuer labels from all issuers in the namespace,
// except the one passed to the func.
func removeIssuerLabel(
ctx context.Context,
helper *helper.Helper,
name string,
namespace string,
labels map[string]string,
) error {
if len(labels) > 0 {
issuerList := &certmgrv1.IssuerList{}
listOpts := []client.ListOption{
client.InNamespace(namespace),
client.MatchingLabels(labels),
}

err := helper.GetClient().List(ctx, issuerList, listOpts...)
if err != nil {
return fmt.Errorf("error getting issuer by label: %w", err)
}

for _, issuer := range issuerList.Items {
if issuer.Name != name {
beforeIssuer := issuer.DeepCopyObject().(client.Object)
for k := range labels {
delete(issuer.Labels, k)
}

err = patchIssuer(ctx, helper, beforeIssuer, &issuer)
if err != nil {
return err
}
}
}
}

return nil
}

func patchIssuer(
ctx context.Context,
helper *helper.Helper,
beforeIssuer client.Object,
issuer *certmgrv1.Issuer,
) error {
// patch issuer
patch := client.MergeFrom(beforeIssuer)
diff, err := patch.Data(issuer)
if err != nil {
return caCertSecretName, err
return err
}

// Unmarshal patch data into a local map for logging
patchDiff := map[string]interface{}{}
if err := json.Unmarshal(diff, &patchDiff); err != nil {
return caCertSecretName, err
return err
}

if _, ok := patchDiff["metadata"]; ok {
err = helper.GetClient().Patch(ctx, issuer, patch)
if k8s_errors.IsConflict(err) {
return caCertSecretName, fmt.Errorf("error metadata update conflict: %w", err)
return fmt.Errorf("error metadata update conflict: %w", err)
} else if err != nil && !k8s_errors.IsNotFound(err) {
return caCertSecretName, fmt.Errorf("error metadata update failed: %w", err)
return fmt.Errorf("error metadata update failed: %w", err)
}

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

return caCertSecretName, nil
return nil
}

func getIssuerAnnotations(certConfig *corev1.CertConfig) map[string]string {
Expand Down
66 changes: 64 additions & 2 deletions tests/functional/openstackoperator_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
. "github.com/onsi/gomega" //revive:disable:dot-imports

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

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

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

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

Expect(OSCtlplane.Spec.TLS.Ingress.Enabled).Should(BeTrue())
Expand All @@ -789,6 +789,7 @@ var _ = Describe("OpenStackOperator controller", func() {
issuer := crtmgr.GetIssuer(names.RootCAPublicName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAPublicName.Name))
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerPublicLabel))
}, timeout, interval).Should(Succeed())
Eventually(func(g Gomega) {
// ca cert
Expand All @@ -802,6 +803,7 @@ var _ = Describe("OpenStackOperator controller", func() {
issuer := crtmgr.GetIssuer(names.RootCAInternalName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAInternalName.Name))
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerInternalLabel))
}, timeout, interval).Should(Succeed())
Eventually(func(g Gomega) {
// ca cert
Expand All @@ -815,6 +817,7 @@ var _ = Describe("OpenStackOperator controller", func() {
issuer := crtmgr.GetIssuer(names.RootCAOvnName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCAOvnName.Name))
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerOvnDBLabel))
}, timeout, interval).Should(Succeed())
Eventually(func(g Gomega) {
// ca cert
Expand All @@ -828,6 +831,7 @@ var _ = Describe("OpenStackOperator controller", func() {
issuer := crtmgr.GetIssuer(names.RootCALibvirtName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Spec.CA.SecretName).Should(Equal(names.RootCALibvirtName.Name))
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerLibvirtLabel))
}, timeout, interval).Should(Succeed())

th.ExpectCondition(
Expand Down Expand Up @@ -926,6 +930,64 @@ var _ = Describe("OpenStackOperator controller", func() {
)
}, timeout, interval).Should(Succeed())
})

When("The TLSe OpenStackControlplane instance switches to use a custom public issuer", func() {
BeforeEach(func() {
// create custom issuer
DeferCleanup(k8sClient.Delete, ctx, crtmgr.CreateIssuer(names.CustomIssuerName))
DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.CustomIssuerName))

// update ctlplane to use the custom isssuer
Eventually(func(g Gomega) {
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
OSCtlplane.Spec.TLS.Ingress.Ca.CustomIssuer = ptr.To(names.CustomIssuerName.Name)
g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed())
}, timeout, interval).Should(Succeed())
})

It("should remove the certmanager.RootCAIssuerPublicLabel label from the defaultIssuer", func() {
Eventually(func(g Gomega) {
issuer := crtmgr.GetIssuer(names.RootCAPublicName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Labels).Should(Not(HaveKey(certmanager.RootCAIssuerPublicLabel)))
}, timeout, interval).Should(Succeed())
})

It("should add the certmanager.RootCAIssuerPublicLabel label to the customIssuer", func() {
Eventually(func(g Gomega) {
issuer := crtmgr.GetIssuer(names.CustomIssuerName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerPublicLabel))
}, timeout, interval).Should(Succeed())
})

When("The TLSe OpenStackControlplane instance switches again back to default public issuer", func() {
BeforeEach(func() {
// update ctlplane to NOT use the custom isssuer
Eventually(func(g Gomega) {
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
OSCtlplane.Spec.TLS.Ingress.Ca.CustomIssuer = nil
g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed())
}, timeout, interval).Should(Succeed())
})

It("should remove the certmanager.RootCAIssuerPublicLabel label from the defaultIssuer", func() {
Eventually(func(g Gomega) {
issuer := crtmgr.GetIssuer(names.RootCAPublicName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Labels).Should(HaveKey(certmanager.RootCAIssuerPublicLabel))
}, timeout, interval).Should(Succeed())
})

It("should add the certmanager.RootCAIssuerPublicLabel label to the customIssuer", func() {
Eventually(func(g Gomega) {
issuer := crtmgr.GetIssuer(names.CustomIssuerName)
g.Expect(issuer).Should(Not(BeNil()))
g.Expect(issuer.Labels).Should(Not(HaveKey(certmanager.RootCAIssuerPublicLabel)))
}, timeout, interval).Should(Succeed())
})
})
})
})

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

0 comments on commit 5d4d6d5

Please sign in to comment.