From efe2e0628862f559f2313e48f2f766c070bb5910 Mon Sep 17 00:00:00 2001 From: Mangesh Hambarde <1411192+mangeshhambarde@users.noreply.github.com> Date: Tue, 5 Mar 2024 18:20:13 +0000 Subject: [PATCH 1/3] New Ingress annotation for copying custom annotations to secret template Signed-off-by: Mangesh Hambarde <1411192+mangeshhambarde@users.noreply.github.com> --- pkg/apis/certmanager/v1/types.go | 5 +++++ pkg/controller/certificate-shim/helper.go | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/pkg/apis/certmanager/v1/types.go b/pkg/apis/certmanager/v1/types.go index 276722793e9..9e8b6902521 100644 --- a/pkg/apis/certmanager/v1/types.go +++ b/pkg/apis/certmanager/v1/types.go @@ -145,6 +145,11 @@ const ( // controller only processes Ingresses with this annotation either unset, or // set to either the configured value or the empty string. IngressClassAnnotationKey = "kubernetes.io/ingress.class" + + // IngressSecretTemplateAnnotations specifies arbitrary annotations on the Ingress resource to be set in the + // generated Certificate resource's secretTemplate. Existing annotations with the same key are overridden. + // The value is a regex that must fully match the annotation key. + IngressSecretTemplateAnnotations = "cert-manager.io/secret-template-annotations" ) // Annotation names for CertificateRequests diff --git a/pkg/controller/certificate-shim/helper.go b/pkg/controller/certificate-shim/helper.go index 198bee84e9c..eab05db7dd0 100644 --- a/pkg/controller/certificate-shim/helper.go +++ b/pkg/controller/certificate-shim/helper.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "reflect" + "regexp" "strconv" "strings" "time" @@ -268,5 +269,26 @@ func translateAnnotations(crt *cmapi.Certificate, ingLikeAnnotations map[string] } } + if customAnnotationsRegexString, found := ingLikeAnnotations[cmapi.IngressSecretTemplateAnnotations]; found { + customAnnotationsRegex, err := regexp.Compile(customAnnotationsRegexString) + if err != nil { + return fmt.Errorf("%w %q: error parsing regexp: %q", errInvalidIngressAnnotation, cmapi.IngressSecretTemplateAnnotations, customAnnotationsRegexString) + } + for annotationKey, annotationValue := range ingLikeAnnotations { + match := customAnnotationsRegex.FindString(annotationKey) + if len(match) == len(annotationKey) { + if strings.HasPrefix(annotationKey, "cert-manager.io/") { + return fmt.Errorf("%w %q: regex must not match cert-manager.io/ annotations: %q", errInvalidIngressAnnotation, cmapi.IngressSecretTemplateAnnotations, customAnnotationsRegexString) + } + if crt.Spec.SecretTemplate == nil { + crt.Spec.SecretTemplate = &cmapi.CertificateSecretTemplate{ + Annotations: map[string]string{}, + } + } + crt.Spec.SecretTemplate.Annotations[annotationKey] = annotationValue + } + } + } + return nil } From 717269e80945c4b5c2f07067e925b1dca5d866cb Mon Sep 17 00:00:00 2001 From: Mangesh Hambarde <1411192+mangeshhambarde@users.noreply.github.com> Date: Thu, 7 Mar 2024 00:00:59 +0000 Subject: [PATCH 2/3] Add tests Signed-off-by: Mangesh Hambarde <1411192+mangeshhambarde@users.noreply.github.com> --- pkg/controller/certificate-shim/sync_test.go | 101 +++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/pkg/controller/certificate-shim/sync_test.go b/pkg/controller/certificate-shim/sync_test.go index 4914d5b0e33..2f39d5c7a50 100644 --- a/pkg/controller/certificate-shim/sync_test.go +++ b/pkg/controller/certificate-shim/sync_test.go @@ -536,6 +536,107 @@ func TestSync(t *testing.T) { }, }, }, + { + Name: "return a single HTTP01 Certificate for an ingress with a single valid TLS entry and valid secret template annotation", + Issuer: acmeClusterIssuer, + IngressLike: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-name", + Namespace: gen.DefaultTestNamespace, + Annotations: map[string]string{ + cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", + cmapi.IngressSecretTemplateAnnotations: "secret-reflector.com.*", + "secret-replicator.com/ignored-annotation": "ignored-value", + "www.secret-reflector.com/ignored-annotation": "ignored-value", + "secret-reflector.com/reflection-enabled": "true", + "secret-reflector.com/reflection-enabled-namespaces": "example-namespace", + }, + UID: types.UID("ingress-name"), + }, + Spec: networkingv1.IngressSpec{ + TLS: []networkingv1.IngressTLS{ + { + Hosts: []string{"example.com", "www.example.com"}, + SecretName: "example-com-tls", + }, + }, + }, + }, + ClusterIssuerLister: []runtime.Object{acmeClusterIssuer}, + ExpectedEvents: []string{`Normal CreateCertificate Successfully created Certificate "example-com-tls"`}, + ExpectedCreate: []*cmapi.Certificate{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "example-com-tls", + Namespace: gen.DefaultTestNamespace, + OwnerReferences: buildIngressOwnerReferences("ingress-name", gen.DefaultTestNamespace), + }, + Spec: cmapi.CertificateSpec{ + DNSNames: []string{"example.com", "www.example.com"}, + SecretName: "example-com-tls", + SecretTemplate: &cmapi.CertificateSecretTemplate{ + Annotations: map[string]string{ + "secret-reflector.com/reflection-enabled": "true", + "secret-reflector.com/reflection-enabled-namespaces": "example-namespace", + }, + }, + IssuerRef: cmmeta.ObjectReference{ + Name: "issuer-name", + Kind: "ClusterIssuer", + }, + Usages: cmapi.DefaultKeyUsages(), + }, + }, + }, + }, + { + Name: "secret template annotation should not match cert-manager.io/ annotations", + Issuer: acmeClusterIssuer, + IngressLike: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-name", + Namespace: gen.DefaultTestNamespace, + Annotations: map[string]string{ + cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", + cmapi.IngressSecretTemplateAnnotations: ".*cert-manager.io/.*", + }, + UID: types.UID("ingress-name"), + }, + Spec: networkingv1.IngressSpec{ + TLS: []networkingv1.IngressTLS{ + { + Hosts: []string{"example.com", "www.example.com"}, + SecretName: "example-com-tls", + }, + }, + }, + }, + Err: true, + }, + { + Name: "secret template annotation should have valid regex", + Issuer: acmeClusterIssuer, + IngressLike: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-name", + Namespace: gen.DefaultTestNamespace, + Annotations: map[string]string{ + cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", + cmapi.IngressSecretTemplateAnnotations: ")invalid] [regex(", + }, + UID: types.UID("ingress-name"), + }, + Spec: networkingv1.IngressSpec{ + TLS: []networkingv1.IngressTLS{ + { + Hosts: []string{"example.com", "www.example.com"}, + SecretName: "example-com-tls", + }, + }, + }, + }, + Err: true, + }, { Name: "edit-in-place set to false should not trigger editing the ingress in-place", Issuer: acmeClusterIssuer, From f3bfc93bbaf98058302f16b452c68e31a3d60a06 Mon Sep 17 00:00:00 2001 From: Mangesh Hambarde <1411192+mangeshhambarde@users.noreply.github.com> Date: Wed, 13 Mar 2024 14:17:00 +0000 Subject: [PATCH 3/3] JSON encoded secretTemplate as Ingress annotation Signed-off-by: Mangesh Hambarde <1411192+mangeshhambarde@users.noreply.github.com> --- pkg/apis/certmanager/v1/types.go | 7 ++--- pkg/controller/certificate-shim/helper.go | 31 +++++++++----------- pkg/controller/certificate-shim/sync_test.go | 22 +++++++------- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/pkg/apis/certmanager/v1/types.go b/pkg/apis/certmanager/v1/types.go index 9e8b6902521..31e737c60fa 100644 --- a/pkg/apis/certmanager/v1/types.go +++ b/pkg/apis/certmanager/v1/types.go @@ -146,10 +146,9 @@ const ( // set to either the configured value or the empty string. IngressClassAnnotationKey = "kubernetes.io/ingress.class" - // IngressSecretTemplateAnnotations specifies arbitrary annotations on the Ingress resource to be set in the - // generated Certificate resource's secretTemplate. Existing annotations with the same key are overridden. - // The value is a regex that must fully match the annotation key. - IngressSecretTemplateAnnotations = "cert-manager.io/secret-template-annotations" + // IngressSecretTemplate can be used to set the secretTemplate field in the generated Certificate. + // The value is a JSON representation of secretTemplate and must not have any unknown fields. + IngressSecretTemplate = "cert-manager.io/secret-template" ) // Annotation names for CertificateRequests diff --git a/pkg/controller/certificate-shim/helper.go b/pkg/controller/certificate-shim/helper.go index eab05db7dd0..ce6f59152e0 100644 --- a/pkg/controller/certificate-shim/helper.go +++ b/pkg/controller/certificate-shim/helper.go @@ -17,10 +17,10 @@ limitations under the License. package shimhelper import ( + "encoding/json" "errors" "fmt" "reflect" - "regexp" "strconv" "strings" "time" @@ -269,25 +269,22 @@ func translateAnnotations(crt *cmapi.Certificate, ingLikeAnnotations map[string] } } - if customAnnotationsRegexString, found := ingLikeAnnotations[cmapi.IngressSecretTemplateAnnotations]; found { - customAnnotationsRegex, err := regexp.Compile(customAnnotationsRegexString) - if err != nil { - return fmt.Errorf("%w %q: error parsing regexp: %q", errInvalidIngressAnnotation, cmapi.IngressSecretTemplateAnnotations, customAnnotationsRegexString) + if secretTemplateJson, found := ingLikeAnnotations[cmapi.IngressSecretTemplate]; found { + decoder := json.NewDecoder(strings.NewReader(secretTemplateJson)) + decoder.DisallowUnknownFields() + + var secretTemplate = new(cmapi.CertificateSecretTemplate) + if err := decoder.Decode(secretTemplate); err != nil { + return fmt.Errorf("%w %q: error parsing secret template JSON: %v", errInvalidIngressAnnotation, cmapi.IngressSecretTemplate, err) } - for annotationKey, annotationValue := range ingLikeAnnotations { - match := customAnnotationsRegex.FindString(annotationKey) - if len(match) == len(annotationKey) { - if strings.HasPrefix(annotationKey, "cert-manager.io/") { - return fmt.Errorf("%w %q: regex must not match cert-manager.io/ annotations: %q", errInvalidIngressAnnotation, cmapi.IngressSecretTemplateAnnotations, customAnnotationsRegexString) - } - if crt.Spec.SecretTemplate == nil { - crt.Spec.SecretTemplate = &cmapi.CertificateSecretTemplate{ - Annotations: map[string]string{}, - } - } - crt.Spec.SecretTemplate.Annotations[annotationKey] = annotationValue + for annotationKey := range secretTemplate.Annotations { + if strings.HasPrefix(annotationKey, "cert-manager.io/") { + return fmt.Errorf("%w %q: secretTemplate must not have cert-manager.io/ annotations: %q", errInvalidIngressAnnotation, cmapi.IngressSecretTemplate, annotationKey) } } + if len(secretTemplate.Annotations) > 0 || len(secretTemplate.Labels) > 0 { + crt.Spec.SecretTemplate = secretTemplate + } } return nil diff --git a/pkg/controller/certificate-shim/sync_test.go b/pkg/controller/certificate-shim/sync_test.go index 2f39d5c7a50..bd98f36267c 100644 --- a/pkg/controller/certificate-shim/sync_test.go +++ b/pkg/controller/certificate-shim/sync_test.go @@ -544,12 +544,8 @@ func TestSync(t *testing.T) { Name: "ingress-name", Namespace: gen.DefaultTestNamespace, Annotations: map[string]string{ - cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", - cmapi.IngressSecretTemplateAnnotations: "secret-reflector.com.*", - "secret-replicator.com/ignored-annotation": "ignored-value", - "www.secret-reflector.com/ignored-annotation": "ignored-value", - "secret-reflector.com/reflection-enabled": "true", - "secret-reflector.com/reflection-enabled-namespaces": "example-namespace", + cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", + cmapi.IngressSecretTemplate: `{ "annotations": { "example-annotation" : "dummy-value" }, "labels": { "example-label" : "dummy-value" } }`, }, UID: types.UID("ingress-name"), }, @@ -576,8 +572,10 @@ func TestSync(t *testing.T) { SecretName: "example-com-tls", SecretTemplate: &cmapi.CertificateSecretTemplate{ Annotations: map[string]string{ - "secret-reflector.com/reflection-enabled": "true", - "secret-reflector.com/reflection-enabled-namespaces": "example-namespace", + "example-annotation": "dummy-value", + }, + Labels: map[string]string{ + "example-label": "dummy-value", }, }, IssuerRef: cmmeta.ObjectReference{ @@ -590,7 +588,7 @@ func TestSync(t *testing.T) { }, }, { - Name: "secret template annotation should not match cert-manager.io/ annotations", + Name: "secret template annotation should not allow cert-manager.io/ annotations", Issuer: acmeClusterIssuer, IngressLike: &networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -598,7 +596,7 @@ func TestSync(t *testing.T) { Namespace: gen.DefaultTestNamespace, Annotations: map[string]string{ cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", - cmapi.IngressSecretTemplateAnnotations: ".*cert-manager.io/.*", + cmapi.IngressSecretTemplate: `{ "annotations": { "cert-manager.io/disallowed-annotation" : "dummy-value" } }`, }, UID: types.UID("ingress-name"), }, @@ -614,7 +612,7 @@ func TestSync(t *testing.T) { Err: true, }, { - Name: "secret template annotation should have valid regex", + Name: "secret template annotation should not allow unknown fields", Issuer: acmeClusterIssuer, IngressLike: &networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -622,7 +620,7 @@ func TestSync(t *testing.T) { Namespace: gen.DefaultTestNamespace, Annotations: map[string]string{ cmapi.IngressClusterIssuerNameAnnotationKey: "issuer-name", - cmapi.IngressSecretTemplateAnnotations: ")invalid] [regex(", + cmapi.IngressSecretTemplate: `{ "unknown-field": "true" }`, }, UID: types.UID("ingress-name"), },