add for certman2: Istio gateways: Allow to specify namespace for TLS …

…secret #316

pkg/certman2/controller/issuer/controlplane/add.go

@@ -8,6 +8,7 @@ package controlplane
 
 import (
 	"context"
+
 	"github.com/gardener/cert-management/pkg/certman2/apis/cert/v1alpha1"
 	"github.com/gardener/cert-management/pkg/certman2/controller/issuer/acme"
 	"github.com/gardener/cert-management/pkg/certman2/controller/issuer/ca"
@@ -73,10 +74,10 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, controlPlaneCluster clust
 				return r.issuersToReconcileOnSecretChanges(ctx, secret)
 			}),
 			builder.WithPredicates(predicate.Funcs{
-				CreateFunc: func(e event.CreateEvent) bool {
+				CreateFunc: func(event.CreateEvent) bool {
 					return false
 				},
-				DeleteFunc: func(e event.DeleteEvent) bool {
+				DeleteFunc: func(event.DeleteEvent) bool {
 					return false
 				},
 				GenericFunc: func(e event.GenericEvent) bool {

pkg/certman2/controller/issuer/controlplane/reconciler.go

@@ -9,6 +9,7 @@ package controlplane
 import (
 	"context"
 	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
@@ -69,7 +70,7 @@ func (r *Reconciler) issuersToReconcileOnSecretChanges(ctx context.Context, secr
 	if err := r.Client.List(ctx, issuerList, client.InNamespace(r.Config.Controllers.Issuer.Namespace)); err != nil {
 		return nil
 	}
-	if issuerList.Items == nil || len(issuerList.Items) == 0 {
+	if len(issuerList.Items) == 0 {
 		return nil
 	}
 	for _, issuer := range issuerList.Items {

pkg/certman2/controller/source/add/add.go

@@ -22,6 +22,7 @@ import (
 	"github.com/gardener/cert-management/pkg/certman2/controller/source/service"
 )
 
+// AddToManager adds all source controllers to the manager.
 func AddToManager(mgr manager.Manager, cfg *config.CertManagerConfiguration, prestartClient client.Client) error {
 	crdState, err := gateways_crd_watchdog.CheckGatewayCRDs(context.Background(), prestartClient)
 	if err != nil {

pkg/certman2/controller/source/certinput.go

@@ -15,12 +15,12 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // CertInput contains basic certificate data.
 type CertInput struct {
-	SecretNamespace     string
-	SecretName          string
+	SecretObjectKey     client.ObjectKey
 	Domains             []string
 	IssuerName          *string
 	FollowCNAME         bool
@@ -32,8 +32,9 @@ type CertInput struct {
 }
 
 // CertInputMap contains a map of secretName to CertInput.
-type CertInputMap map[string]CertInput
+type CertInputMap map[client.ObjectKey]CertInput
 
+// GetCertSourceSpecForService gets the certificate source spec for a service of type loadbalancer.
 func GetCertSourceSpecForService(log logr.Logger, service *corev1.Service) (CertInputMap, error) {
 	secretName, ok := service.Annotations[AnnotSecretname]
 	if !ok {
@@ -43,18 +44,22 @@ func GetCertSourceSpecForService(log logr.Logger, service *corev1.Service) (Cert
 	if secretName == "" {
 		return nil, fmt.Errorf("empty secret name annotation %q", AnnotSecretname)
 	}
+	secretNamespace, ok := service.Annotations[AnnotSecretNamespace]
+	if !ok {
+		secretNamespace = service.Namespace
+	}
 
 	annotatedDomains, _ := getDomainsFromAnnotations(service.Annotations, true)
 	if annotatedDomains == nil {
 		return nil, fmt.Errorf("no valid domain name annotations found for service %q", service.Name)
 	}
 
+	secretObjectKey := client.ObjectKey{Namespace: secretNamespace, Name: secretName}
 	certInput := augmentFromCommonAnnotations(service.Annotations, CertInput{
-		SecretNamespace: service.Namespace,
-		SecretName:      secretName,
+		SecretObjectKey: secretObjectKey,
 		Domains:         annotatedDomains,
 	})
-	return CertInputMap{secretName: certInput}, nil
+	return CertInputMap{certInput.SecretObjectKey: certInput}, nil
 }
 
 func augmentFromCommonAnnotations(annotations map[string]string, certInput CertInput) CertInput {
@@ -72,9 +77,9 @@ func augmentFromCommonAnnotations(annotations map[string]string, certInput CertI
 	if value, ok := annotations[AnnotFollowCNAME]; ok {
 		followCNAME, _ = strconv.ParseBool(value)
 	}
-	preferredChain, _ := annotations[AnnotPreferredChain]
+	preferredChain := annotations[AnnotPreferredChain]
 
-	algorithm, _ := annotations[AnnotPrivateKeyAlgorithm]
+	algorithm := annotations[AnnotPrivateKeyAlgorithm]
 	keySize := 0
 	if keySizeStr, ok := annotations[AnnotPrivateKeySize]; ok {
 		if value, err := strconv.Atoi(keySizeStr); err == nil {
@@ -116,7 +121,7 @@ func getDomainsFromAnnotations(annotations map[string]string, forService bool) (
 		}
 	}
 
-	cn, _ = annotations[AnnotCommonName]
+	cn = annotations[AnnotCommonName]
 	cn = strings.TrimSpace(cn)
 	annotatedDomains = []string{}
 	if cn != "" {
@@ -158,6 +163,7 @@ func copyAnnotations(annotations map[string]string, keys ...string) (result map[
 	return
 }
 
+// CreateSpec creates a CertificateSpec from a CertInput.
 func CreateSpec(src CertInput) certmanv1alpha1.CertificateSpec {
 	spec := certmanv1alpha1.CertificateSpec{}
 	if len(src.Domains) > 0 {
@@ -178,8 +184,8 @@ func CreateSpec(src CertInput) certmanv1alpha1.CertificateSpec {
 		}
 	}
 	spec.SecretRef = &corev1.SecretReference{
-		Name:      src.SecretName,
-		Namespace: src.SecretNamespace,
+		Name:      src.SecretObjectKey.Name,
+		Namespace: src.SecretObjectKey.Namespace,
 	}
 	if src.FollowCNAME {
 		spec.FollowCNAME = &src.FollowCNAME

pkg/certman2/controller/source/certinput_collector.go

@@ -54,10 +54,9 @@ func GetCertInputByCollector(ctx context.Context, log logr.Logger, obj client.Ob
 		} else {
 			domains = mergeCommonName(cn, tls.Hosts)
 		}
-		key := tls.SecretNamespace + "/" + tls.SecretName
+		key := client.ObjectKey{Namespace: tls.SecretNamespace, Name: tls.SecretName}
 		inputMap[key] = augmentFromCommonAnnotations(obj.GetAnnotations(), CertInput{
-			SecretNamespace: tls.SecretNamespace,
-			SecretName:      tls.SecretName,
+			SecretObjectKey: key,
 			Domains:         domains,
 		})
 	}

pkg/certman2/controller/source/gateways_crd_watchdog/add.go

@@ -34,6 +34,7 @@ var relevantCRDs = []string{
 	k8sHTTPRoutesCRD,
 }
 
+// CheckGatewayCRDsState contains the state of the gateway CRD check.
 type CheckGatewayCRDsState struct {
 	relevantCRDDeployed      map[string]string
 	istioGatewayVersion      istio_gateway.Version

pkg/certman2/controller/source/ingress/reconciler.go

@@ -17,6 +17,7 @@ type Reconciler struct {
 	source.ReconcilerBase
 }
 
+// Complete implements the option completer.
 func (r *Reconciler) Complete() {
 	r.GVK = schema.GroupVersionKind{Group: "networking.k8s.io", Version: "v1", Kind: "Ingress"}
 }

pkg/certman2/controller/source/istio_gateway/add.go

@@ -32,7 +32,7 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
 		ControllerManagedBy(mgr).
 		Named(ControllerName).
 		For(newGateway(r.ActiveVersion), builder.WithPredicates(Predicate(newGateway(r.ActiveVersion), r.Class))).
-		Watches(newVirtualService(r.ActiveVersion), handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, virtualService client.Object) []reconcile.Request {
+		Watches(newVirtualService(r.ActiveVersion), handler.EnqueueRequestsFromMapFunc(func(_ context.Context, virtualService client.Object) []reconcile.Request {
 			var requests []reconcile.Request
 			for key := range extractGatewayNames(virtualService) {
 				requests = append(requests, reconcile.Request{NamespacedName: key})

pkg/certman2/controller/source/istio_gateway/reconciler.go

@@ -25,6 +25,7 @@ type Reconciler struct {
 	ActiveVersion Version
 }
 
+// Complete implements the option completer.
 func (r *Reconciler) Complete() {
 	r.GVK = schema.GroupVersionKind{Group: istionetworkingv1.GroupName, Version: string(r.ActiveVersion), Kind: "Gateway"}
 }

pkg/certman2/controller/source/istio_gateway/reconciler_certinputmap_test.go

@@ -284,15 +284,14 @@ func singleCertInput(secretName string, names ...string) source.CertInputMap {
 func toMap(inputs ...source.CertInput) source.CertInputMap {
 	result := source.CertInputMap{}
 	for _, input := range inputs {
-		result[input.SecretNamespace+"/"+input.SecretName] = input
+		result[input.SecretObjectKey] = input
 	}
 	return result
 }
 
 func makeCertInput(secretName string, names ...string) source.CertInput {
 	return source.CertInput{
-		SecretNamespace: "test",
-		SecretName:      secretName,
+		SecretObjectKey: client.ObjectKey{Namespace: "test", Name: secretName},
 		Domains:         names,
 	}
 }

pkg/certman2/controller/source/istio_gateway/reconciler_reconcile.go

@@ -45,12 +45,17 @@ func (r *Reconciler) getCertificateInputMap(ctx context.Context, log logr.Logger
 	return source.GetCertInputByCollector(ctx, log, gateway, func(ctx context.Context, obj client.Object) ([]*source.TLSData, error) {
 		var array []*source.TLSData
 
+		namespace := gateway.GetAnnotations()[source.AnnotSecretNamespace]
+		if namespace == "" {
+			namespace = gateway.GetNamespace()
+		}
+
 		switch data := obj.(type) {
 		case *istionetworkingv1.Gateway:
 			for _, server := range data.Spec.Servers {
 				if server.Tls != nil && server.Tls.CredentialName != "" {
 					array = append(array, &source.TLSData{
-						SecretNamespace: gateway.GetNamespace(),
+						SecretNamespace: namespace,
 						SecretName:      server.Tls.CredentialName,
 						Hosts:           parsedHosts(server.Hosts),
 					})
@@ -60,7 +65,7 @@ func (r *Reconciler) getCertificateInputMap(ctx context.Context, log logr.Logger
 			for _, server := range data.Spec.Servers {
 				if server.Tls != nil && server.Tls.CredentialName != "" {
 					array = append(array, &source.TLSData{
-						SecretNamespace: gateway.GetNamespace(),
+						SecretNamespace: namespace,
 						SecretName:      server.Tls.CredentialName,
 						Hosts:           parsedHosts(server.Hosts),
 					})
@@ -70,7 +75,7 @@ func (r *Reconciler) getCertificateInputMap(ctx context.Context, log logr.Logger
 			for _, server := range data.Spec.Servers {
 				if server.Tls != nil && server.Tls.CredentialName != "" {
 					array = append(array, &source.TLSData{
-						SecretNamespace: gateway.GetNamespace(),
+						SecretNamespace: namespace,
 						SecretName:      server.Tls.CredentialName,
 						Hosts:           parsedHosts(server.Hosts),
 					})

pkg/certman2/controller/source/istio_gateway/reconciler_test.go

@@ -117,7 +117,7 @@ func createReconcilerTestFunc[T client.Object](obj T, version Version) func() {
 				source.AnnotationPurposeKey: source.AnnotationPurposeValueManaged,
 				source.AnnotDnsnames:        "*",
 			})
-			modifyServers(gateway, func(servers []*networkingv1.Server) []*networkingv1.Server {
+			modifyServers(gateway, func([]*networkingv1.Server) []*networkingv1.Server {
 				return []*networkingv1.Server{
 					{
 						Hosts: []string{"host1.example.com"},
@@ -211,6 +211,14 @@ func createReconcilerTestFunc[T client.Object](obj T, version Version) func() {
 				})
 			})
 
+			It("should create correct certificate object with overwritten secret namespace", func() {
+				gateway.GetAnnotations()[source.AnnotSecretNamespace] = "other"
+				test(&certmanv1alpha1.CertificateSpec{
+					CommonName: ptr.To("host1.example.com"),
+					SecretRef:  &corev1.SecretReference{Name: "host1-secret", Namespace: "other"},
+				})
+			})
+
 			It("should update certificate object for service of type load balancer with additional fields", func() {
 				annotations := gateway.GetAnnotations()
 				annotations[source.AnnotCertDNSNames] = fmt.Sprintf("foo1.%s,foo2.%s", longDomain, longDomain)
@@ -270,7 +278,7 @@ func createReconcilerTestFunc[T client.Object](obj T, version Version) func() {
 			})
 
 			It("should create multiple certificates for multiple TLS", func() {
-				modifyServers(gateway, func(servers []*networkingv1.Server) []*networkingv1.Server {
+				modifyServers(gateway, func([]*networkingv1.Server) []*networkingv1.Server {
 					return []*networkingv1.Server{
 						{
 							Hosts: []string{"host1.example.com", "host1-alt.example.com"},

pkg/certman2/controller/source/istio_gateway/versions.go

@@ -21,10 +21,14 @@ import (
 type Version string
 
 const (
-	VersionV1       Version = "v1"
-	VersionV1beta1  Version = "v1beta1"
+	// VersionV1 is the v1 version of the istio gateway.
+	VersionV1 Version = "v1"
+	// VersionV1beta1 is the v1beta1 version of the istio gateway.
+	VersionV1beta1 Version = "v1beta1"
+	// VersionV1alpha3 is the v1alpha3 version of the istio gateway.
 	VersionV1alpha3 Version = "v1alpha3"
-	VersionNone     Version = ""
+	// VersionNone is zero version of the istio gateway.
+	VersionNone Version = ""
 )
 
 // GetPreferredVersion retrieves the preferred version from the custom resource definition.

pkg/certman2/controller/source/k8s_gateway/add.go

@@ -32,7 +32,7 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
 		ControllerManagedBy(mgr).
 		Named(ControllerName).
 		For(newGateway(r.ActiveVersion), builder.WithPredicates(Predicate(newGateway(r.ActiveVersion), r.Class))).
-		Watches(newHTTPRoute(r.ActiveVersion), handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, httpRoute client.Object) []reconcile.Request {
+		Watches(newHTTPRoute(r.ActiveVersion), handler.EnqueueRequestsFromMapFunc(func(_ context.Context, httpRoute client.Object) []reconcile.Request {
 			var requests []reconcile.Request
 			for key := range extractGatewayNames(httpRoute) {
 				requests = append(requests, reconcile.Request{NamespacedName: key})

pkg/certman2/controller/source/k8s_gateway/reconciler.go

@@ -26,6 +26,7 @@ type Reconciler struct {
 	ActiveVersion Version
 }
 
+// Complete implements the option completer.
 func (r *Reconciler) Complete() {
 	r.GVK = schema.GroupVersionKind{Group: gatewayapisv1.GroupName, Version: string(r.ActiveVersion), Kind: gatewayKind}
 }

pkg/certman2/controller/source/k8s_gateway/reconciler_certinputmap_test.go

@@ -360,7 +360,7 @@ func singleCertInput(secretName string, ns *string, names ...string) source.Cert
 func toMap(inputs ...source.CertInput) source.CertInputMap {
 	result := source.CertInputMap{}
 	for _, input := range inputs {
-		result[input.SecretNamespace+"/"+input.SecretName] = input
+		result[input.SecretObjectKey] = input
 	}
 	return result
 }
@@ -371,8 +371,7 @@ func makeCertInput(secretName string, ns *string, names ...string) source.CertIn
 		namespace = *ns
 	}
 	return source.CertInput{
-		SecretNamespace: namespace,
-		SecretName:      secretName,
+		SecretObjectKey: client.ObjectKey{Namespace: namespace, Name: secretName},
 		Domains:         names,
 	}
 }

pkg/certman2/controller/source/k8s_gateway/reconciler_test.go

@@ -114,7 +114,7 @@ func createReconcilerTestFunc[T client.Object](obj T, version Version) func() {
 				source.AnnotationPurposeKey: source.AnnotationPurposeValueManaged,
 				source.AnnotDnsnames:        "*",
 			})
-			modifyListeners(gateway, func(servers []gatewayapisv1.Listener) []gatewayapisv1.Listener {
+			modifyListeners(gateway, func([]gatewayapisv1.Listener) []gatewayapisv1.Listener {
 				return []gatewayapisv1.Listener{
 					{
 						Hostname: ptr.To[gatewayapisv1.Hostname]("host1.example.com"),
@@ -269,7 +269,7 @@ func createReconcilerTestFunc[T client.Object](obj T, version Version) func() {
 			})
 
 			It("should create multiple certificates for multiple TLS", func() {
-				modifyListeners(gateway, func(listeners []gatewayapisv1.Listener) []gatewayapisv1.Listener {
+				modifyListeners(gateway, func([]gatewayapisv1.Listener) []gatewayapisv1.Listener {
 					return []gatewayapisv1.Listener{
 						{
 							Hostname: ptr.To[gatewayapisv1.Hostname]("host1.example.com"),

pkg/certman2/controller/source/k8s_gateway/versions.go

@@ -21,10 +21,14 @@ import (
 type Version string
 
 const (
-	VersionV1       Version = "v1"
-	VersionV1beta1  Version = "v1beta1"
+	// VersionV1 is the v1 version of the Kubernetes Gateway API gateway.
+	VersionV1 Version = "v1"
+	// VersionV1beta1 is the v1beta1 version of the Kubernetes Gateway API gateway.
+	VersionV1beta1 Version = "v1beta1"
+	// VersionV1alpha2 is the v1alpha2 version of the Kubernetes Gateway API gateway.
 	VersionV1alpha2 Version = "v1alpha2"
-	VersionNone     Version = ""
+	// VersionNone is zero version of the Kubernetes Gateway API gateway.
+	VersionNone Version = ""
 )
 
 // GetPreferredVersion retrieves the preferred version from the custom resource definition.

pkg/certman2/controller/source/reconcilerbase.go

@@ -35,16 +35,24 @@ type ReconcilerBase struct {
 
 // DoReconcile reconciles for given object and certInput.
 func (r *ReconcilerBase) DoReconcile(ctx context.Context, log logr.Logger, obj client.Object, certInputMap CertInputMap) (reconcile.Result, error) {
-	newCerts := map[string]*certmanv1alpha1.Certificate{}
+	newCerts := map[client.ObjectKey]*certmanv1alpha1.Certificate{}
 	ownedCerts, err := r.getExistingOwnedCertificates(ctx, client.ObjectKeyFromObject(obj))
 	if err != nil {
 		return reconcile.Result{}, err
 	}
-	for key, certInput := range certInputMap {
+	for key := range certInputMap {
 		var matchingCert *certmanv1alpha1.Certificate
 		for _, ownedCert := range ownedCerts {
-			if ownedCert.Spec.SecretRef != nil && ownedCert.Spec.SecretRef.Name == certInput.SecretName && ownedCert.Spec.SecretRef.Namespace == certInput.SecretNamespace ||
-				ownedCert.Spec.SecretRef == nil && ownedCert.Spec.SecretName != nil && *ownedCert.Spec.SecretName == certInput.SecretName {
+			var refKey client.ObjectKey
+			if ownedCert.Spec.SecretRef != nil {
+				refKey = client.ObjectKey{Namespace: ownedCert.Spec.SecretRef.Namespace, Name: ownedCert.Spec.SecretRef.Name}
+				if refKey.Namespace == "" {
+					refKey.Namespace = obj.GetNamespace()
+				}
+			} else if ownedCert.Spec.SecretName != nil {
+				refKey = client.ObjectKey{Namespace: obj.GetNamespace(), Name: *ownedCert.Spec.SecretName}
+			}
+			if refKey == key {
 				matchingCert = &ownedCert
 				break
 			}

pkg/certman2/controller/source/service/reconciler.go

@@ -17,6 +17,7 @@ type Reconciler struct {
 	source.ReconcilerBase
 }
 
+// Complete implements the option completer.
 func (r *Reconciler) Complete() {
 	r.GVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}
 }