diff --git a/deploy/charts/cert-manager/templates/webhook-deployment.yaml b/deploy/charts/cert-manager/templates/webhook-deployment.yaml index 043c4b15071..db85b947a58 100644 --- a/deploy/charts/cert-manager/templates/webhook-deployment.yaml +++ b/deploy/charts/cert-manager/templates/webhook-deployment.yaml @@ -72,7 +72,7 @@ spec: - --secure-port={{ .Values.webhook.securePort }} {{- end }} {{- if .Values.featureGates }} - - --feature-gates={{ .Values.featureGates }} + - --feature-gates={{ .Values.webhook.featureGates }} {{- end }} {{- $tlsConfig := default $config.tlsConfig "" }} {{ if or (not $config.tlsConfig) (and (not $tlsConfig.dynamic) (not $tlsConfig.filesystem) ) -}} diff --git a/deploy/charts/cert-manager/values.yaml b/deploy/charts/cert-manager/values.yaml index def8de1b900..46513b65a06 100644 --- a/deploy/charts/cert-manager/values.yaml +++ b/deploy/charts/cert-manager/values.yaml @@ -70,7 +70,7 @@ podDisruptionBudget: # or a percentage value (e.g. 25%) # Comma separated list of feature gates that should be enabled on the -# controller pod & webhook pod. +# controller pod. featureGates: "" # The maximum number of challenges that can be scheduled as 'processing' at once @@ -341,6 +341,10 @@ webhook: # Path to a file containing a WebhookConfiguration object used to configure the webhook # - --config= + # Comma separated list of feature gates that should be enabled on the + # webhook pod. + featureGates: "" + resources: {} # requests: # cpu: 10m diff --git a/internal/cainjector/feature/features.go b/internal/cainjector/feature/features.go index 196d3ea6382..951ee7f25e3 100644 --- a/internal/cainjector/feature/features.go +++ b/internal/cainjector/feature/features.go @@ -14,6 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ +// feature contains cainjector feature gate setup code. Do not import this +// package into any code that's shared with other components to prevent +// overwriting other component's featue gates, see i.e +// https://github.com/cert-manager/cert-manager/issues/6011 package feature import ( diff --git a/internal/controller/feature/features.go b/internal/controller/feature/features.go index 10ce0b40c6d..87d4c9ed5a7 100644 --- a/internal/controller/feature/features.go +++ b/internal/controller/feature/features.go @@ -14,6 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ +// feature contains controller's feature gate setup functionality. Do not import +// this package into any code that's shared with other components to prevent +// overwriting other component's featue gates, see i.e +// https://github.com/cert-manager/cert-manager/issues/6011 package feature import ( diff --git a/internal/webhook/feature/features.go b/internal/webhook/feature/features.go index a096cb2744f..d9a44dcffd3 100644 --- a/internal/webhook/feature/features.go +++ b/internal/webhook/feature/features.go @@ -14,6 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ +// feature contains webhook's feature gate setup functionality. Do not import +// this package into any code that's shared with other components to prevent +// overwriting other component's featue gates, see i.e +// https://github.com/cert-manager/cert-manager/issues/6011 package feature import ( diff --git a/make/e2e-setup.mk b/make/e2e-setup.mk index 2c77dc181af..09b3126aadf 100644 --- a/make/e2e-setup.mk +++ b/make/e2e-setup.mk @@ -270,7 +270,7 @@ e2e-setup-certmanager: $(BINDIR)/cert-manager.tgz $(foreach binaryname,controlle --set installCRDs=true \ --set featureGates="$(feature_gates_controller)" \ --set "extraArgs={--kube-api-qps=9000,--kube-api-burst=9000,--concurrent-workers=200}" \ - --set "webhook.extraArgs={--feature-gates=$(feature_gates_webhook)}" \ + --set webhook.featureGates="$(feature_gates_webhook)" \ --set "cainjector.extraArgs={--feature-gates=$(feature_gates_cainjector)}" \ --set "dns01RecursiveNameservers=$(SERVICE_IP_PREFIX).16:53" \ --set "dns01RecursiveNameserversOnly=true" \ diff --git a/pkg/controller/certificates/requestmanager/requestmanager_controller.go b/pkg/controller/certificates/requestmanager/requestmanager_controller.go index 381d3ce0748..2a6cc5c9019 100644 --- a/pkg/controller/certificates/requestmanager/requestmanager_controller.go +++ b/pkg/controller/certificates/requestmanager/requestmanager_controller.go @@ -349,7 +349,8 @@ func (c *controller) deleteRequestsNotMatchingSpec(ctx context.Context, crt *cma func (c *controller) createNewCertificateRequest(ctx context.Context, crt *cmapi.Certificate, pk crypto.Signer, nextRevision int, nextPrivateKeySecretName string) error { log := logf.FromContext(ctx) - x509CSR, err := pki.GenerateCSR(crt) + + x509CSR, err := pki.GenerateCSR(crt, pki.WithUseLiteralSubject(utilfeature.DefaultMutableFeatureGate.Enabled(feature.LiteralCertificateSubject))) if err != nil { log.Error(err, "Failed to generate CSR - will not retry") return nil diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index 5e1087ab42c..a4a64a6e329 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -30,10 +30,8 @@ import ( "net/url" "strings" - "github.com/cert-manager/cert-manager/internal/controller/feature" apiutil "github.com/cert-manager/cert-manager/pkg/api/util" v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature" ) func IPAddressesForCertificate(crt *v1.Certificate) []net.IP { @@ -165,6 +163,7 @@ func BuildCertManagerKeyUsages(ku x509.KeyUsage, eku []x509.ExtKeyUsage) []v1.Ke type generateCSROptions struct { EncodeBasicConstraintsInRequest bool + UseLiteralSubject bool } type GenerateCSROption func(*generateCSROptions) @@ -178,6 +177,12 @@ func WithEncodeBasicConstraintsInRequest(encode bool) GenerateCSROption { } } +func WithUseLiteralSubject(useLiteralSubject bool) GenerateCSROption { + return func(o *generateCSROptions) { + o.UseLiteralSubject = useLiteralSubject + } +} + // GenerateCSR will generate a new *x509.CertificateRequest template to be used // by issuers that utilise CSRs to obtain Certificates. // The CSR will not be signed, and should be passed to either EncodeCSR or @@ -185,14 +190,23 @@ func WithEncodeBasicConstraintsInRequest(encode bool) GenerateCSROption { func GenerateCSR(crt *v1.Certificate, optFuncs ...GenerateCSROption) (*x509.CertificateRequest, error) { opts := &generateCSROptions{ EncodeBasicConstraintsInRequest: false, + UseLiteralSubject: false, } for _, opt := range optFuncs { opt(opts) } - commonName, err := extractCommonName(crt.Spec) - if err != nil { - return nil, err + var ( + commonName = crt.Spec.CommonName + err error + ) + + if opts.UseLiteralSubject { + commonName, err = extractCommonNameFromLiteralSubject(crt.Spec) + if err != nil { + return nil, err + } + } iPAddresses := IPAddressesForCertificate(crt) @@ -250,7 +264,7 @@ func GenerateCSR(crt *v1.Certificate, optFuncs ...GenerateCSROption) (*x509.Cert ExtraExtensions: extraExtensions, } - if isLiteralCertificateSubjectEnabled() && len(crt.Spec.LiteralSubject) > 0 { + if opts.UseLiteralSubject && len(crt.Spec.LiteralSubject) > 0 { rawSubject, err := ParseSubjectStringToRawDERBytes(crt.Spec.LiteralSubject) if err != nil { return nil, err @@ -449,30 +463,25 @@ func SignatureAlgorithm(crt *v1.Certificate) (x509.PublicKeyAlgorithm, x509.Sign return pubKeyAlgo, sigAlgo, nil } -func extractCommonName(spec v1.CertificateSpec) (string, error) { - var commonName = spec.CommonName - if isLiteralCertificateSubjectEnabled() && len(spec.LiteralSubject) > 0 { - commonName = "" - sequence, err := UnmarshalSubjectStringToRDNSequence(spec.LiteralSubject) - if err != nil { - return "", err - } +func extractCommonNameFromLiteralSubject(spec v1.CertificateSpec) (string, error) { + if spec.LiteralSubject == "" { + return spec.CommonName, nil + } + commonName := "" + sequence, err := UnmarshalSubjectStringToRDNSequence(spec.LiteralSubject) + if err != nil { + return "", err + } - for _, rdns := range sequence { - for _, atv := range rdns { - if atv.Type.Equal(OIDConstants.CommonName) { - if str, ok := atv.Value.(string); ok { - commonName = str - } + for _, rdns := range sequence { + for _, atv := range rdns { + if atv.Type.Equal(OIDConstants.CommonName) { + if str, ok := atv.Value.(string); ok { + commonName = str } } } } return commonName, nil - -} - -func isLiteralCertificateSubjectEnabled() bool { - return utilfeature.DefaultFeatureGate.Enabled(feature.LiteralCertificateSubject) } diff --git a/pkg/util/pki/csr_test.go b/pkg/util/pki/csr_test.go index 1b0e5fd2413..7159ce6a164 100644 --- a/pkg/util/pki/csr_test.go +++ b/pkg/util/pki/csr_test.go @@ -29,12 +29,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "github.com/cert-manager/cert-manager/internal/controller/feature" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "github.com/cert-manager/cert-manager/pkg/util" - utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature" ) func buildCertificate(cn string, dnsNames ...string) *cmapi.Certificate { @@ -614,10 +611,10 @@ func TestGenerateCSR(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultMutableFeatureGate, feature.LiteralCertificateSubject, tt.literalCertificateSubjectFeatureEnabled)() got, err := GenerateCSR( tt.crt, WithEncodeBasicConstraintsInRequest(tt.basicConstraintsFeatureEnabled), + WithUseLiteralSubject(tt.literalCertificateSubjectFeatureEnabled), ) if (err != nil) != tt.wantErr { t.Errorf("GenerateCSR() error = %v, wantErr %v", err, tt.wantErr)