From aed1d7137e6f9814aefea0656c72bea096c2e480 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Tue, 3 Aug 2021 01:16:39 +0100 Subject: [PATCH] feat: add validation to block annotations containing cert-manager.io/ prefix Signed-off-by: jonathansp --- .../certmanager/validation/certificate.go | 13 ++- .../validation/certificate_test.go | 104 ++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/pkg/internal/apis/certmanager/validation/certificate.go b/pkg/internal/apis/certmanager/validation/certificate.go index e5777fa50..b439eb737 100644 --- a/pkg/internal/apis/certmanager/validation/certificate.go +++ b/pkg/internal/apis/certmanager/validation/certificate.go @@ -20,6 +20,7 @@ import ( "fmt" "net" "net/mail" + "strings" admissionv1 "k8s.io/api/admission/v1" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -181,7 +182,17 @@ func validateSecretTemplateLabels(crt *internalcmapi.CertificateSpec, fldPath *f } func validateSecretTemplateAnnotations(crt *internalcmapi.CertificateSpec, fldPath *field.Path) field.ErrorList { - return apivalidation.ValidateAnnotations(crt.SecretTemplate.Annotations, fldPath.Child("secretTemplate", "annotations")) + el := field.ErrorList{} + + secretTemplateAnnotationsPath := fldPath.Child("secretTemplate", "annotations") + for a := range crt.SecretTemplate.Annotations { + if strings.HasPrefix(a, "cert-manager.io/") { + el = append(el, field.Invalid(secretTemplateAnnotationsPath, a, "cert-manager.io/* annotations are not allowed")) + } + } + + el = append(el, apivalidation.ValidateAnnotations(crt.SecretTemplate.Annotations, secretTemplateAnnotationsPath)...) + return el } func ValidateDuration(crt *internalcmapi.CertificateSpec, fldPath *field.Path) field.ErrorList { diff --git a/pkg/internal/apis/certmanager/validation/certificate_test.go b/pkg/internal/apis/certmanager/validation/certificate_test.go index 70a855066..e75fc77af 100644 --- a/pkg/internal/apis/certmanager/validation/certificate_test.go +++ b/pkg/internal/apis/certmanager/validation/certificate_test.go @@ -19,6 +19,7 @@ package validation import ( "fmt" "reflect" + "strings" "testing" "time" @@ -47,6 +48,7 @@ var ( Version: "test", }, } + maxSecretTemplateAnnotationsBytesLimit = 256 * (1 << 10) // 256 kB ) func strPtr(s string) *string { @@ -626,6 +628,108 @@ func TestValidateCertificate(t *testing.T) { "Certificate"), }, }, + "valid with empty secretTemplate": { + cfg: &internalcmapi.Certificate{ + Spec: internalcmapi.CertificateSpec{ + CommonName: "testcn", + SecretName: "abc", + SecretTemplate: &internalcmapi.CertificateSecretTemplate{ + Annotations: map[string]string{}, + Labels: map[string]string{}, + }, + IssuerRef: cmmeta.ObjectReference{ + Name: "valid", + }, + }, + }, + a: someAdmissionRequest, + }, + "valid with 'CertificateSecretTemplate' labels and annotations": { + cfg: &internalcmapi.Certificate{ + Spec: internalcmapi.CertificateSpec{ + CommonName: "testcn", + SecretName: "abc", + SecretTemplate: &internalcmapi.CertificateSecretTemplate{ + Annotations: map[string]string{ + "my-annotation.com/foo": "app=bar", + }, + Labels: map[string]string{ + "my-label.com/foo": "evn-production", + }, + }, + IssuerRef: cmmeta.ObjectReference{ + Name: "valid", + }, + }, + }, + a: someAdmissionRequest, + }, + "invalid with disallowed 'CertificateSecretTemplate' annotations": { + cfg: &internalcmapi.Certificate{ + Spec: internalcmapi.CertificateSpec{ + CommonName: "testcn", + SecretName: "abc", + SecretTemplate: &internalcmapi.CertificateSecretTemplate{ + Annotations: map[string]string{ + "app.com/valid": "valid", + "cert-manager.io/alt-names": "example.com", + "cert-manager.io/certificate-name": "selfsigned-cert", + }, + }, + IssuerRef: cmmeta.ObjectReference{ + Name: "invalid", + }, + }, + }, + a: someAdmissionRequest, + errs: []*field.Error{ + field.Invalid(fldPath.Child("secretTemplate", "annotations"), "cert-manager.io/alt-names", "cert-manager.io/* annotations are not allowed"), + field.Invalid(fldPath.Child("secretTemplate", "annotations"), "cert-manager.io/certificate-name", "cert-manager.io/* annotations are not allowed"), + }, + }, + "invalid due to too long 'CertificateSecretTemplate' annotations": { + cfg: &internalcmapi.Certificate{ + Spec: internalcmapi.CertificateSpec{ + CommonName: "testcn", + SecretName: "abc", + SecretTemplate: &internalcmapi.CertificateSecretTemplate{ + Annotations: map[string]string{ + "app.com/invalid": strings.Repeat("0", maxSecretTemplateAnnotationsBytesLimit), + }, + }, + IssuerRef: cmmeta.ObjectReference{ + Name: "invalid", + }, + }, + }, + a: someAdmissionRequest, + errs: []*field.Error{ + field.TooLong(fldPath.Child("secretTemplate", "annotations"), "", maxSecretTemplateAnnotationsBytesLimit), + }, + }, + "invalid due to not allowed 'CertificateSecretTemplate' labels": { + cfg: &internalcmapi.Certificate{ + Spec: internalcmapi.CertificateSpec{ + CommonName: "testcn", + SecretName: "abc", + SecretTemplate: &internalcmapi.CertificateSecretTemplate{ + Labels: map[string]string{ + "app.com/invalid-chars": "invalid=chars", + }, + }, + IssuerRef: cmmeta.ObjectReference{ + Name: "invalid", + }, + }, + }, + a: someAdmissionRequest, + errs: []*field.Error{ + field.Invalid( + fldPath.Child("secretTemplate", "labels"), + "invalid=chars", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an "+ + "alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"), + }, + }, } for n, s := range scenarios { t.Run(n, func(t *testing.T) {