csr: add expirationSeconds field to control cert lifetime

This change updates the CSR API to add a new, optional field called
expirationSeconds.  This field is a request to the signer for the
maximum duration the client wishes the cert to have.  The signer is
free to ignore this request based on its own internal policy.  The
signers built-in to KCM will honor this field if it is not set to a
value greater than --cluster-signing-duration.  The minimum allowed
value for this field is 600 seconds (ten minutes).

This change will help enforce safer durations for certificates in
the Kube ecosystem and will help related projects such as
cert-manager with their migration to the Kube CSR API.

Future enhancements may update the Kubelet to take advantage of this
field when it is configured in a way that can tolerate shorter
certificate lifespans with regular rotation.

Signed-off-by: Monis Khan <mok@vmware.com>

pkg/registry/certificates/certificates/storage/metrics.go

@@ -0,0 +1,169 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package storage
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"sync"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
+	"k8s.io/apiserver/pkg/util/dryrun"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/client-go/util/cert"
+	"k8s.io/client-go/util/certificate/csr"
+	"k8s.io/component-base/metrics"
+	"k8s.io/component-base/metrics/legacyregistry"
+	"k8s.io/kubernetes/pkg/apis/certificates"
+	"k8s.io/kubernetes/pkg/features"
+)
+
+const (
+	namespace = "apiserver"
+	subsystem = "certificates_registry"
+)
+
+var (
+	// csrDurationRequested counts and categorizes how many certificates were issued when the client requested a duration.
+	csrDurationRequested = metrics.NewCounterVec(
+		&metrics.CounterOpts{
+			Namespace:      namespace,
+			Subsystem:      subsystem,
+			Name:           "csr_requested_duration_total",
+			Help:           "Total number of issued CSRs with a requested duration, sliced by signer (only kubernetes.io signer names are specifically identified)",
+			StabilityLevel: metrics.ALPHA,
+		},
+		[]string{"signerName"},
+	)
+
+	// csrDurationHonored counts and categorizes how many certificates were issued when the client requested a duration and the signer honored it.
+	csrDurationHonored = metrics.NewCounterVec(
+		&metrics.CounterOpts{
+			Namespace:      namespace,
+			Subsystem:      subsystem,
+			Name:           "csr_honored_duration_total",
+			Help:           "Total number of issued CSRs with a requested duration that was honored, sliced by signer (only kubernetes.io signer names are specifically identified)",
+			StabilityLevel: metrics.ALPHA,
+		},
+		[]string{"signerName"},
+	)
+)
+
+func init() {
+	registerMetricsOnce.Do(func() {
+		legacyregistry.MustRegister(csrDurationRequested)
+		legacyregistry.MustRegister(csrDurationHonored)
+	})
+}
+
+var registerMetricsOnce sync.Once
+
+type counterVecMetric interface {
+	WithLabelValues(...string) metrics.CounterMetric
+}
+
+func countCSRDurationMetric(requested, honored counterVecMetric) genericregistry.BeginUpdateFunc {
+	return func(ctx context.Context, obj, old runtime.Object, options *metav1.UpdateOptions) (genericregistry.FinishFunc, error) {
+		return func(ctx context.Context, success bool) {
+			if !success {
+				return // ignore failures
+			}
+
+			if dryrun.IsDryRun(options.DryRun) {
+				return // ignore things that would not get persisted
+			}
+
+			if !utilfeature.DefaultFeatureGate.Enabled(features.CSRDuration) {
+				return
+			}
+
+			oldCSR, ok := old.(*certificates.CertificateSigningRequest)
+			if !ok {
+				return
+			}
+
+			// if the old CSR already has a certificate, do not double count it
+			if len(oldCSR.Status.Certificate) > 0 {
+				return
+			}
+
+			if oldCSR.Spec.ExpirationSeconds == nil {
+				return // ignore CSRs that are not using the CSR duration feature
+			}
+
+			newCSR, ok := obj.(*certificates.CertificateSigningRequest)
+			if !ok {
+				return
+			}
+			issuedCert := newCSR.Status.Certificate
+
+			// new CSR has no issued certificate yet so do not count it.
+			// note that this means that we will ignore CSRs that set a duration
+			// but never get approved/signed.  this is fine because the point
+			// of these metrics is to understand if the duration is honored
+			// by the signer.  we are not checking the behavior of the approver.
+			if len(issuedCert) == 0 {
+				return
+			}
+
+			signer := compressSignerName(oldCSR.Spec.SignerName)
+
+			// at this point we know that this CSR is going to be persisted and
+			// the cert was just issued and the client requested a duration
+			requested.WithLabelValues(signer).Inc()
+
+			certs, err := cert.ParseCertsPEM(issuedCert)
+			if err != nil {
+				utilruntime.HandleError(fmt.Errorf("metrics recording failed to parse certificate for CSR %s: %w", oldCSR.Name, err))
+				return
+			}
+
+			// now we check to see if the signer honored the requested duration
+			certificate := certs[0]
+			wantDuration := csr.ExpirationSecondsToDuration(*oldCSR.Spec.ExpirationSeconds)
+			actualDuration := certificate.NotAfter.Sub(certificate.NotBefore)
+			if isDurationHonored(wantDuration, actualDuration) {
+				honored.WithLabelValues(signer).Inc()
+			}
+		}, nil
+	}
+}
+
+func isDurationHonored(want, got time.Duration) bool {
+	delta := want - got
+	if delta < 0 {
+		delta = -delta
+	}
+
+	// short-lived cert backdating + 5% of want
+	maxDelta := 5*time.Minute + (want / 20)
+
+	return delta < maxDelta
+}
+
+func compressSignerName(name string) string {
+	if strings.HasPrefix(name, "kubernetes.io/") {
+		return name
+	}
+
+	return "other"
+}

pkg/registry/certificates/certificates/storage/metrics_test.go

@@ -0,0 +1,496 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package storage
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"testing"
+	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	certutil "k8s.io/client-go/util/cert"
+	"k8s.io/client-go/util/certificate/csr"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	"k8s.io/component-base/metrics"
+	"k8s.io/kubernetes/pkg/apis/certificates"
+	"k8s.io/kubernetes/pkg/features"
+	"k8s.io/utils/pointer"
+)
+
+func Test_countCSRDurationMetric(t *testing.T) {
+	caPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	caCert, err := certutil.NewSelfSignedCACert(certutil.Config{CommonName: "test-ca"}, caPrivateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tests := []struct {
+		name                       string
+		disableFeatureGate         bool
+		success                    bool
+		obj, old                   runtime.Object
+		options                    *metav1.UpdateOptions
+		wantSigner                 string
+		wantRequested, wantHonored bool
+	}{
+		{
+			name:    "cert parse failure",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: []byte("junk"),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "fancy",
+					ExpirationSeconds: pointer.Int32(77),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "other",
+			wantRequested: true,
+			wantHonored:   false,
+		},
+		{
+			name:    "kube signer honors duration exactly",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "kubernetes.io/educate-dolphins",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "kubernetes.io/educate-dolphins",
+			wantRequested: true,
+			wantHonored:   true,
+		},
+		{
+			name:    "signer honors duration exactly",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "other",
+			wantRequested: true,
+			wantHonored:   true,
+		},
+		{
+			name:    "signer honors duration but just a little bit less",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour-6*time.Minute, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "other",
+			wantRequested: true,
+			wantHonored:   true,
+		},
+		{
+			name:    "signer honors duration but just a little bit more",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour+6*time.Minute, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "other",
+			wantRequested: true,
+			wantHonored:   true,
+		},
+		{
+			name:    "honors duration lower bound",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, 651*time.Second, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "kubernetes.io/educate-dolphins",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "kubernetes.io/educate-dolphins",
+			wantRequested: true,
+			wantHonored:   true,
+		},
+		{
+			name:    "does not honor duration just outside of lower bound",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, 650*time.Second, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "kubernetes.io/educate-dolphins",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "kubernetes.io/educate-dolphins",
+			wantRequested: true,
+			wantHonored:   false,
+		},
+		{
+			name:    "honors duration upper bound",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, 1349*time.Second, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "kubernetes.io/educate-dolphins",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "kubernetes.io/educate-dolphins",
+			wantRequested: true,
+			wantHonored:   true,
+		},
+		{
+			name:    "does not honor duration just outside of upper bound",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, 1350*time.Second, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "kubernetes.io/educate-dolphins",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(1_000 * time.Second),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "kubernetes.io/educate-dolphins",
+			wantRequested: true,
+			wantHonored:   false,
+		},
+		{
+			name:    "failed update is ignored",
+			success: false,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "dry run is ignored",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{DryRun: []string{"stuff"}},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:               "no metrics when feature gate is turned off",
+			disableFeatureGate: true,
+			success:            true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "old CSR already has a cert so it is ignored",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: []byte("junk"),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "CSRs with no duration are ignored",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: nil,
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "unissued CSRs are ignored",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: nil,
+				},
+			},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "invalid data - nil old object",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old:           nil,
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "invalid data - nil new object",
+			success: true,
+			obj:     nil,
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "invalid data - junk old object",
+			success: true,
+			obj: &certificates.CertificateSigningRequest{
+				Status: certificates.CertificateSigningRequestStatus{
+					Certificate: createCert(t, time.Hour, caPrivateKey, caCert),
+				},
+			},
+			old:           &corev1.Pod{},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+		{
+			name:    "invalid data - junk new object",
+			success: true,
+			obj:     &corev1.Pod{},
+			old: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					SignerName:        "pandas",
+					ExpirationSeconds: csr.DurationToExpirationSeconds(time.Hour),
+				},
+			},
+			options:       &metav1.UpdateOptions{},
+			wantSigner:    "",
+			wantRequested: false,
+			wantHonored:   false,
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.disableFeatureGate {
+				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSRDuration, false)()
+			} else {
+				t.Parallel()
+			}
+
+			testReq := &testCounterVecMetric{}
+			testHon := &testCounterVecMetric{}
+
+			finishFunc, err := countCSRDurationMetric(testReq, testHon)(nil, tt.obj, tt.old, tt.options)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			finishFunc(nil, tt.success)
+
+			if got := testReq.signer; tt.wantRequested && tt.wantSigner != got {
+				t.Errorf("requested signer: want %v, got %v", tt.wantSigner, got)
+			}
+
+			if got := testHon.signer; tt.wantHonored && tt.wantSigner != got {
+				t.Errorf("honored signer: want %v, got %v", tt.wantSigner, got)
+			}
+
+			if got := testReq.called; tt.wantRequested != got {
+				t.Errorf("requested inc: want %v, got %v", tt.wantRequested, got)
+			}
+
+			if got := testHon.called; tt.wantHonored != got {
+				t.Errorf("honored inc: want %v, got %v", tt.wantHonored, got)
+			}
+		})
+	}
+}
+
+func createCert(t *testing.T, duration time.Duration, caPrivateKey *ecdsa.PrivateKey, caCert *x509.Certificate) []byte {
+	t.Helper()
+
+	crPublicKey := &caPrivateKey.PublicKey // this is supposed to be public key of the signee but it does not matter for this test
+
+	now := time.Now()
+	tmpl := &x509.Certificate{Subject: pkix.Name{CommonName: "panda"}, SerialNumber: big.NewInt(1234), NotBefore: now, NotAfter: now.Add(duration)}
+
+	der, err := x509.CreateCertificate(rand.Reader, tmpl, caCert, crPublicKey, caPrivateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
+}
+
+type testCounterVecMetric struct {
+	metrics.CounterMetric
+
+	signer string
+	called bool
+}
+
+func (m *testCounterVecMetric) WithLabelValues(lv ...string) metrics.CounterMetric {
+	if len(lv) != 1 {
+		panic(lv)
+	}
+
+	if len(m.signer) != 0 {
+		panic("unexpected multiple WithLabelValues() calls")
+	}
+
+	signer := lv[0]
+
+	if len(signer) == 0 {
+		panic("invalid empty signer")
+	}
+
+	m.signer = signer
+	return m
+}
+
+func (m *testCounterVecMetric) Inc() {
+	if m.called {
+		panic("unexpected multiple Inc() calls")
+	}
+
+	m.called = true
+}

pkg/registry/certificates/certificates/storage/storage.go

@@ -62,6 +62,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Approva
 	statusStore := *store
 	statusStore.UpdateStrategy = csrregistry.StatusStrategy
 	statusStore.ResetFieldsStrategy = csrregistry.StatusStrategy
+	statusStore.BeginUpdate = countCSRDurationMetric(csrDurationRequested, csrDurationHonored)
 
 	approvalStore := *store
 	approvalStore.UpdateStrategy = csrregistry.ApprovalStrategy

pkg/registry/certificates/certificates/strategy.go

@@ -30,9 +30,11 @@ import (
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	"k8s.io/apiserver/pkg/storage/names"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/certificates"
 	"k8s.io/kubernetes/pkg/apis/certificates/validation"
+	"k8s.io/kubernetes/pkg/features"
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
@@ -91,10 +93,14 @@ func (csrStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 		if extra := user.GetExtra(); len(extra) > 0 {
 			csr.Spec.Extra = map[string]certificates.ExtraValue{}
 			for k, v := range extra {
-				csr.Spec.Extra[k] = certificates.ExtraValue(v)
+				csr.Spec.Extra[k] = v
 			}
 		}
 	}
+	// clear expirationSeconds if the CSRDuration feature is disabled
+	if !utilfeature.DefaultFeatureGate.Enabled(features.CSRDuration) {
+		csr.Spec.ExpirationSeconds = nil
+	}
 
 	// Be explicit that users cannot create pre-approved certificate requests.
 	csr.Status = certificates.CertificateSigningRequestStatus{}

pkg/registry/certificates/certificates/strategy_test.go

@@ -27,14 +27,19 @@ import (
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apiserver/pkg/authentication/user"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	certapi "k8s.io/kubernetes/pkg/apis/certificates"
+	"k8s.io/kubernetes/pkg/features"
+	"k8s.io/utils/pointer"
 )
 
 func TestStrategyCreate(t *testing.T) {
 	tests := map[string]struct {
-		ctx         context.Context
-		obj         runtime.Object
-		expectedObj runtime.Object
+		ctx                context.Context
+		disableFeatureGate bool
+		obj                runtime.Object
+		expectedObj        runtime.Object
 	}{
 		"no user in context, no user in obj": {
 			ctx: genericapirequest.NewContext(),
@@ -112,15 +117,51 @@ func TestStrategyCreate(t *testing.T) {
 			},
 			expectedObj: &certapi.CertificateSigningRequest{
 				Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}},
-			}},
+			},
+		},
+		"expirationSeconds set with gate enabled": {
+			ctx: genericapirequest.NewContext(),
+			obj: &certapi.CertificateSigningRequest{
+				Spec: certapi.CertificateSigningRequestSpec{
+					ExpirationSeconds: pointer.Int32(1234),
+				},
+			},
+			expectedObj: &certapi.CertificateSigningRequest{
+				Spec: certapi.CertificateSigningRequestSpec{
+					ExpirationSeconds: pointer.Int32(1234),
+				},
+				Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}},
+			},
+		},
+		"expirationSeconds set with gate disabled": {
+			ctx:                genericapirequest.NewContext(),
+			disableFeatureGate: true,
+			obj: &certapi.CertificateSigningRequest{
+				Spec: certapi.CertificateSigningRequestSpec{
+					ExpirationSeconds: pointer.Int32(5678),
+				},
+			},
+			expectedObj: &certapi.CertificateSigningRequest{
+				Spec: certapi.CertificateSigningRequestSpec{
+					ExpirationSeconds: nil,
+				},
+				Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}},
+			},
+		},
 	}
 
 	for k, tc := range tests {
-		obj := tc.obj
-		Strategy.PrepareForCreate(tc.ctx, obj)
-		if !reflect.DeepEqual(obj, tc.expectedObj) {
-			t.Errorf("%s: object diff: %s", k, diff.ObjectDiff(obj, tc.expectedObj))
-		}
+		tc := tc
+		t.Run(k, func(t *testing.T) {
+			if tc.disableFeatureGate {
+				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSRDuration, false)()
+			}
+			obj := tc.obj
+			Strategy.PrepareForCreate(tc.ctx, obj)
+			if !reflect.DeepEqual(obj, tc.expectedObj) {
+				t.Errorf("object diff: %s", diff.ObjectDiff(obj, tc.expectedObj))
+			}
+		})
 	}
 }