Merge pull request #6093 from irbekrm/fix_webhook_flags

Fix webhook feature gate

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) ) -}}

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=<path-to-config-file>
 
+  # Comma separated list of feature gates that should be enabled on the
+  # webhook pod.
+  featureGates: ""
+
   resources: {}
     # requests:
     #   cpu: 10m

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 (

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 (

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 (

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" \

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

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,21 +177,36 @@ 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
 // to the x509.CreateCertificateRequest function.
 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)
 }

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)