From 7b7912022a45de8694554a8361f9e1177d6d4b2a Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Wed, 6 Dec 2023 10:50:00 +0100 Subject: [PATCH] Add feature gate Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- .../apis/certmanager/validation/certificate.go | 15 +++++++++++++++ internal/controller/feature/features.go | 9 +++++++++ internal/webhook/feature/features.go | 9 +++++++++ make/e2e-setup.mk | 6 +++--- pkg/util/pki/csr.go | 10 +++++++++- 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/internal/apis/certmanager/validation/certificate.go b/internal/apis/certmanager/validation/certificate.go index fb07c64cb..81e0753cd 100644 --- a/internal/apis/certmanager/validation/certificate.go +++ b/internal/apis/certmanager/validation/certificate.go @@ -118,6 +118,21 @@ func ValidateCertificateSpec(crt *internalcmapi.CertificateSpec, fldPath *field. el = append(el, validateEmailAddresses(crt, fldPath)...) } + if len(crt.OtherNameSANs) > 0 { + if !utilfeature.DefaultFeatureGate.Enabled(feature.OtherNameSANs) { + el = append(el, field.Forbidden(fldPath.Child("OtherNameSANs"), "Feature gate OtherNameSANs must be enabled on both webhook and controller to use the alpha `otherNameSANs` field")) + } + + for i, otherName := range crt.OtherNameSANs { + if otherName.OID == "" { + el = append(el, field.Required(fldPath.Child("otherNameSANs").Index(i).Child("oid"), "must be specified")) + } + if otherName.StringValue == "" { + el = append(el, field.Required(fldPath.Child("otherNameSANs").Index(i).Child("stringValue"), "must be specified")) + } + } + } + if crt.PrivateKey != nil { switch crt.PrivateKey.Algorithm { case "", internalcmapi.RSAKeyAlgorithm: diff --git a/internal/controller/feature/features.go b/internal/controller/feature/features.go index 48840e972..b4f49c8bb 100644 --- a/internal/controller/feature/features.go +++ b/internal/controller/feature/features.go @@ -126,6 +126,14 @@ const ( // CertificateRequest's usages to be only defined in the CSR, while leaving // the usages field empty. DisallowInsecureCSRUsageDefinition featuregate.Feature = "DisallowInsecureCSRUsageDefinition" + + // Owner: @SpectralHiss + // Alpha: v1.14 + // + // OtherNameSANs adds support for OtherName Subject Alternative Name values in + // Certificate resources. + // Github Issue: https://github.com/cert-manager/cert-manager/issues/6393 + OtherNameSANs featuregate.Feature = "OtherNameSANs" ) func init() { @@ -148,4 +156,5 @@ var defaultCertManagerFeatureGates = map[featuregate.Feature]featuregate.Feature LiteralCertificateSubject: {Default: false, PreRelease: featuregate.Alpha}, UseCertificateRequestBasicConstraints: {Default: false, PreRelease: featuregate.Alpha}, UseCertificateRequestNameConstraints: {Default: false, PreRelease: featuregate.Alpha}, + OtherNameSANs: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/internal/webhook/feature/features.go b/internal/webhook/feature/features.go index f63e32714..e73e7cd87 100644 --- a/internal/webhook/feature/features.go +++ b/internal/webhook/feature/features.go @@ -69,6 +69,14 @@ const ( // This feature will add NameConstraints section in CSR with CA field as true // Github Issue: https://github.com/cert-manager/cert-manager/issues/3655 UseCertificateRequestNameConstraints featuregate.Feature = "UseCertificateRequestNameConstraints" + + // Owner: @SpectralHiss + // Alpha: v1.14 + // + // OtherNameSANs adds support for OtherName Subject Alternative Name values in + // Certificate resources. + // Github Issue: https://github.com/cert-manager/cert-manager/issues/6393 + OtherNameSANs featuregate.Feature = "OtherNameSANs" ) func init() { @@ -88,4 +96,5 @@ var webhookFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ AdditionalCertificateOutputFormats: {Default: false, PreRelease: featuregate.Alpha}, LiteralCertificateSubject: {Default: false, PreRelease: featuregate.Alpha}, UseCertificateRequestNameConstraints: {Default: false, PreRelease: featuregate.Alpha}, + OtherNameSANs: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/make/e2e-setup.mk b/make/e2e-setup.mk index 344c9d4d5..197cbdf7a 100644 --- a/make/e2e-setup.mk +++ b/make/e2e-setup.mk @@ -221,7 +221,7 @@ $(call local-image-tar,vaultretagged): $(call image-tar,vault) tar cf $@ -C /tmp/vault . @rm -rf /tmp/vault -FEATURE_GATES ?= AdditionalCertificateOutputFormats=true,ExperimentalCertificateSigningRequestControllers=true,ExperimentalGatewayAPISupport=true,ServerSideApply=true,LiteralCertificateSubject=true,UseCertificateRequestBasicConstraints=true,UseCertificateRequestNameConstraints=true +FEATURE_GATES ?= AdditionalCertificateOutputFormats=true,ExperimentalCertificateSigningRequestControllers=true,ExperimentalGatewayAPISupport=true,ServerSideApply=true,LiteralCertificateSubject=true,UseCertificateRequestBasicConstraints=true,UseCertificateRequestNameConstraints=true,OtherNameSANs=true ## Set this environment variable to a non empty string to cause cert-manager to ## be installed using best-practice configuration settings, and to install @@ -262,8 +262,8 @@ comma = , # Helm's "--set" interprets commas, which means we want to escape commas # for "--set featureGates". That's why we have "\$(comma)". -feature_gates_controller := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% AdditionalCertificateOutputFormats=% ValidateCAA=% ExperimentalCertificateSigningRequestControllers=% ExperimentalGatewayAPISupport=% ServerSideApply=% LiteralCertificateSubject=% UseCertificateRequestBasicConstraints=% UseCertificateRequestNameConstraints=% SecretsFilteredCaching=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) -feature_gates_webhook := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% AdditionalCertificateOutputFormats=% LiteralCertificateSubject=% UseCertificateRequestNameConstraints=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) +feature_gates_controller := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% AdditionalCertificateOutputFormats=% ValidateCAA=% ExperimentalCertificateSigningRequestControllers=% ExperimentalGatewayAPISupport=% ServerSideApply=% LiteralCertificateSubject=% UseCertificateRequestBasicConstraints=% UseCertificateRequestNameConstraints=% SecretsFilteredCaching=% OtherNameSANs=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) +feature_gates_webhook := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% AdditionalCertificateOutputFormats=% LiteralCertificateSubject=% UseCertificateRequestNameConstraints=% OtherNameSANs=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) feature_gates_cainjector := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% ServerSideApply=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) # Install cert-manager with E2E specific images and deployment settings. diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index c3fdb3873..65398e6c2 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -187,6 +187,7 @@ func BuildCertManagerKeyUsages(ku x509.KeyUsage, eku []x509.ExtKeyUsage) []v1.Ke type generateCSROptions struct { EncodeBasicConstraintsInRequest bool EncodeNameConstraintsInRequest bool + EncodeOtherNameSANs bool UseLiteralSubject bool } @@ -207,6 +208,12 @@ func WithEncodeNameConstraintsInRequest(encode bool) GenerateCSROption { } } +func WithEncodeOtherNameSANs(encodeOtherNameSANs bool) GenerateCSROption { + return func(o *generateCSROptions) { + o.EncodeOtherNameSANs = encodeOtherNameSANs + } +} + func WithUseLiteralSubject(useLiteralSubject bool) GenerateCSROption { return func(o *generateCSROptions) { o.UseLiteralSubject = useLiteralSubject @@ -221,6 +228,7 @@ func GenerateCSR(crt *v1.Certificate, optFuncs ...GenerateCSROption) (*x509.Cert opts := &generateCSROptions{ EncodeBasicConstraintsInRequest: false, EncodeNameConstraintsInRequest: false, + EncodeOtherNameSANs: false, UseLiteralSubject: false, } for _, opt := range optFuncs { @@ -308,7 +316,7 @@ func GenerateCSR(crt *v1.Certificate, optFuncs ...GenerateCSROption) (*x509.Cert } } - if len(crt.Spec.OtherNameSANs) != 0 { + if len(otherNameSANs) != 0 { SANwithotherNameExtension, err := buildSANExtensionIncludingOtherNameSANsForCertificate(crt) if err != nil { return nil, err