certrotation: require UserInfo on PeerRotation, add tests

pkg/operator/certrotation/cabundle_test.go

@@ -1,7 +1,6 @@
 package certrotation
 
 import (
-	"context"
 	gcrypto "crypto"
 	"crypto/rand"
 	"crypto/x509"
@@ -216,7 +215,7 @@ func TestEnsureConfigMapCABundle(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			_, err = c.EnsureConfigMapCABundle(context.TODO(), newCA, "signer-secret")
+			_, err = c.EnsureConfigMapCABundle(t.Context(), newCA, "signer-secret")
 			switch {
 			case err != nil && len(test.expectedError) == 0:
 				t.Error(err)

pkg/operator/certrotation/client_cert_rotation_controller_test.go

@@ -300,7 +300,7 @@ func TestCertRotationController_SyncWorker(t *testing.T) {
 			}
 
 			// Run SyncWorker
-			ctx := context.Background()
+			ctx := t.Context()
 			err := controller.SyncWorker(ctx)
 
 			// Check error

pkg/operator/certrotation/signer_test.go

@@ -1,7 +1,6 @@
 package certrotation
 
 import (
-	"context"
 	"crypto/x509"
 	"encoding/pem"
 	"strings"
@@ -271,7 +270,7 @@ func TestEnsureSigningCertKeyPair(t *testing.T) {
 				RefreshOnlyWhenExpired: test.RefreshOnlyWhenExpired,
 			}
 
-			_, updated, err := c.EnsureSigningCertKeyPair(context.TODO())
+			_, updated, err := c.EnsureSigningCertKeyPair(t.Context())
 			switch {
 			case err != nil && len(test.expectedError) == 0:
 				t.Error(err)
@@ -422,7 +421,7 @@ func TestEnsureSigningCertKeyPair_WithKeyConfig(t *testing.T) {
 		},
 	}
 
-	ca, updated, err := c.EnsureSigningCertKeyPair(context.TODO())
+	ca, updated, err := c.EnsureSigningCertKeyPair(t.Context())
 	if err != nil {
 		t.Fatalf("EnsureSigningCertKeyPair() error = %v", err)
 	}

pkg/operator/certrotation/target.go

@@ -327,10 +327,10 @@ func (r *ClientRotation) CertificateType() pki.CertificateType {
 }
 
 func (r *ClientRotation) NewCertificate(signer *crypto.CA, validity time.Duration, keyGen crypto.KeyPairGenerator) (*crypto.TLSCertificateConfig, error) {
-	if keyGen != nil {
-		return signer.NewClientCertificate(r.UserInfo, keyGen, crypto.WithLifetime(validity))
+	if keyGen == nil {
+		return signer.MakeClientCertificateForDuration(r.UserInfo, validity)
 	}
-	return signer.MakeClientCertificateForDuration(r.UserInfo, validity)
+	return signer.NewClientCertificate(r.UserInfo, keyGen, crypto.WithLifetime(validity))
 }
 
 func (r *ClientRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, exists bool) string {
@@ -356,14 +356,13 @@ func (r *ServingRotation) NewCertificate(signer *crypto.CA, validity time.Durati
 	if len(hostnames) == 0 {
 		return nil, fmt.Errorf("no hostnames set")
 	}
-	if keyGen != nil {
-		return signer.NewServerCertificate(
-			sets.New(hostnames...), keyGen,
-			crypto.WithLifetime(validity),
-			crypto.WithExtensions(r.CertificateExtensionFn...),
-		)
+	if keyGen == nil {
+		return signer.MakeServerCertForDuration(sets.New(hostnames...), validity, r.CertificateExtensionFn...)
 	}
-	return signer.MakeServerCertForDuration(sets.New(hostnames...), validity, r.CertificateExtensionFn...)
+	return signer.NewServerCertificate(sets.New(hostnames...), keyGen,
+		crypto.WithLifetime(validity),
+		crypto.WithExtensions(r.CertificateExtensionFn...),
+	)
 }
 
 func (r *ServingRotation) RecheckChannel() <-chan struct{} {
@@ -423,23 +422,25 @@ func (r *SignerRotation) CertificateType() pki.CertificateType {
 
 func (r *SignerRotation) NewCertificate(signer *crypto.CA, validity time.Duration, keyGen crypto.KeyPairGenerator) (*crypto.TLSCertificateConfig, error) {
 	signerName := fmt.Sprintf("%s_@%d", r.SignerName, time.Now().Unix())
-	if keyGen != nil {
-		return crypto.NewSigningCertificate(signerName, keyGen,
-			crypto.WithSigner(signer),
-			crypto.WithLifetime(validity),
-		)
+	if keyGen == nil {
+		return crypto.MakeCAConfigForDuration(signerName, validity, signer)
 	}
-	return crypto.MakeCAConfigForDuration(signerName, validity, signer)
+	return crypto.NewSigningCertificate(signerName, keyGen,
+		crypto.WithSigner(signer),
+		crypto.WithLifetime(validity),
+	)
 }
 
 // PeerRotation creates certificates used for both server and client authentication
 // (e.g., etcd peer certificates). It uses CertificateTypePeer for PKI profile
 // resolution, which selects the stronger of the serving and client key configurations.
 //
-// When keyGen is non-nil (ConfigurablePKI enabled), it calls NewPeerCertificate
-// which requires UserInfo for the client identity. When keyGen is nil (legacy),
-// it falls back to MakeServerCertForDuration with an extension function that
-// sets both ClientAuth and ServerAuth ExtKeyUsages.
+// UserInfo is always required. When keyGen is non-nil (ConfigurablePKI enabled),
+// NewPeerCertificate encodes UserInfo as the client identity in the certificate
+// subject. When keyGen is nil (legacy), UserInfo.Name must match the sorted first
+// hostname (used as CN by MakeServerCertForDuration), and the certificate falls
+// back to MakeServerCertForDuration with an extension function that sets both
+// ClientAuth and ServerAuth ExtKeyUsages.
 type PeerRotation struct {
 	Hostnames              ServingHostnameFunc
 	UserInfo               user.Info
@@ -456,24 +457,28 @@ func (r *PeerRotation) NewCertificate(signer *crypto.CA, validity time.Duration,
 	if len(hostnames) == 0 {
 		return nil, fmt.Errorf("no hostnames set")
 	}
-	if keyGen != nil {
-		if r.UserInfo == nil {
-			return nil, fmt.Errorf("PeerRotation requires UserInfo for configurable PKI certificates")
+	if r.UserInfo == nil {
+		return nil, fmt.Errorf("PeerRotation requires UserInfo")
+	}
+	if keyGen == nil {
+		// Legacy path: use server cert template with extension fn to add both ExtKeyUsages.
+		// MakeServerCertForDuration sorts hostnames and uses the first sorted value as CN.
+		sortedHostnames := sets.List(sets.New(hostnames...))
+		if cn := sortedHostnames[0]; r.UserInfo.GetName() != cn {
+			return nil, fmt.Errorf("PeerRotation legacy path uses sorted first hostname %q as CN; UserInfo.Name %q conflicts — set UserInfo.Name to match or enable ConfigurablePKI", cn, r.UserInfo.GetName())
+		}
+		peerExtFn := func(cert *x509.Certificate) error {
+			cert.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}
+			return nil
 		}
-		return signer.NewPeerCertificate(
-			sets.New(hostnames...), r.UserInfo, keyGen,
-			crypto.WithLifetime(validity),
-			crypto.WithExtensions(r.CertificateExtensionFn...),
-		)
-	}
-	// Legacy path: use server cert template with extension fn to add both ExtKeyUsages.
-	// The subject CN comes from the first hostname (preserves current behavior).
-	peerExtFn := func(cert *x509.Certificate) error {
-		cert.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}
-		return nil
-	}
-	extensions := append(append([]crypto.CertificateExtensionFunc{}, r.CertificateExtensionFn...), peerExtFn)
-	return signer.MakeServerCertForDuration(sets.New(hostnames...), validity, extensions...)
+		extensions := append(append([]crypto.CertificateExtensionFunc{}, r.CertificateExtensionFn...), peerExtFn)
+		return signer.MakeServerCertForDuration(sets.New(hostnames...), validity, extensions...)
+	}
+	return signer.NewPeerCertificate(
+		sets.New(hostnames...), r.UserInfo, keyGen,
+		crypto.WithLifetime(validity),
+		crypto.WithExtensions(r.CertificateExtensionFn...),
+	)
 }
 
 func (r *PeerRotation) RecheckChannel() <-chan struct{} {

pkg/operator/certrotation/target_test.go

@@ -1,9 +1,9 @@
 package certrotation
 
 import (
-	"context"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -302,7 +302,7 @@ func TestEnsureTargetCertKeyPair(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			_, err = c.EnsureTargetCertKeyPair(context.TODO(), newCA, newCA.Config.Certs)
+			_, err = c.EnsureTargetCertKeyPair(t.Context(), newCA, newCA.Config.Certs)
 			switch {
 			case err != nil && len(test.expectedError) == 0:
 				t.Error(err)
@@ -493,7 +493,7 @@ func TestEnsureTargetSignerCertKeyPair(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			_, err = c.EnsureTargetCertKeyPair(context.TODO(), newCA, newCA.Config.Certs)
+			_, err = c.EnsureTargetCertKeyPair(t.Context(), newCA, newCA.Config.Certs)
 			switch {
 			case err != nil && len(test.expectedError) == 0:
 				t.Error(err)
@@ -727,6 +727,11 @@ func TestClientRotation_NewCertificate_WithKeyPairGenerator(t *testing.T) {
 			keyGen:  crypto.ECDSAKeyPairGenerator{Curve: crypto.P256},
 			wantAlg: x509.ECDSA,
 		},
+		{
+			name:    "ECDSA-P384",
+			keyGen:  crypto.ECDSAKeyPairGenerator{Curve: crypto.P384},
+			wantAlg: x509.ECDSA,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -765,11 +770,21 @@ func TestServingRotation_NewCertificate_WithKeyPairGenerator(t *testing.T) {
 			keyGen:  nil,
 			wantAlg: x509.RSA,
 		},
+		{
+			name:    "RSA-4096",
+			keyGen:  crypto.RSAKeyPairGenerator{Bits: 4096},
+			wantAlg: x509.RSA,
+		},
 		{
 			name:    "ECDSA-P256",
 			keyGen:  crypto.ECDSAKeyPairGenerator{Curve: crypto.P256},
 			wantAlg: x509.ECDSA,
 		},
+		{
+			name:    "ECDSA-P384",
+			keyGen:  crypto.ECDSAKeyPairGenerator{Curve: crypto.P384},
+			wantAlg: x509.ECDSA,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -797,6 +812,113 @@ func TestServingRotation_NewCertificate_WithKeyPairGenerator(t *testing.T) {
 	}
 }
 
+func TestPeerRotation_NewCertificate_WithKeyPairGenerator(t *testing.T) {
+	testCases := []struct {
+		name      string
+		keyGen    crypto.KeyPairGenerator
+		userInfo  user.Info
+		hostnames []string
+		wantAlg   x509.PublicKeyAlgorithm
+		wantErr   string
+	}{
+		{
+			name:      "nil keyGen (legacy) uses RSA with dual ExtKeyUsage",
+			keyGen:    nil,
+			userInfo:  &user.DefaultInfo{Name: "127.0.0.1"},
+			hostnames: []string{"localhost", "127.0.0.1"},
+			wantAlg:   x509.RSA,
+		},
+		{
+			name:      "ECDSA keyGen with UserInfo",
+			keyGen:    crypto.ECDSAKeyPairGenerator{Curve: crypto.P256},
+			userInfo:  &user.DefaultInfo{Name: "peer-user", Groups: []string{"peer-group"}},
+			hostnames: []string{"localhost", "127.0.0.1"},
+			wantAlg:   x509.ECDSA,
+		},
+		{
+			name:      "nil UserInfo errors",
+			keyGen:    nil,
+			userInfo:  nil,
+			hostnames: []string{"localhost"},
+			wantErr:   "requires UserInfo",
+		},
+		{
+			name:      "nil UserInfo with keyGen errors",
+			keyGen:    crypto.ECDSAKeyPairGenerator{Curve: crypto.P256},
+			userInfo:  nil,
+			hostnames: []string{"localhost"},
+			wantErr:   "requires UserInfo",
+		},
+		{
+			name:      "mismatched UserInfo name on legacy path",
+			keyGen:    nil,
+			userInfo:  &user.DefaultInfo{Name: "wrong-name"},
+			hostnames: []string{"localhost", "127.0.0.1"},
+			wantErr:   "conflicts",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ca, err := newTestCACertificate(pkix.Name{CommonName: "test-ca"}, int64(1), metav1.Duration{Duration: time.Hour * 24}, time.Now)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			r := &PeerRotation{
+				Hostnames: func() []string { return tc.hostnames },
+				UserInfo:  tc.userInfo,
+			}
+			certConfig, err := r.NewCertificate(ca, time.Hour, tc.keyGen)
+			if tc.wantErr != "" {
+				if err == nil {
+					t.Fatalf("expected error containing %q, got nil", tc.wantErr)
+				}
+				if !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("expected error containing %q, got %q", tc.wantErr, err.Error())
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("NewCertificate() error = %v", err)
+			}
+
+			cert := certConfig.Certs[0]
+			if cert.PublicKeyAlgorithm != tc.wantAlg {
+				t.Errorf("PublicKeyAlgorithm = %v, want %v", cert.PublicKeyAlgorithm, tc.wantAlg)
+			}
+
+			// Verify both ExtKeyUsages are present
+			if !slices.Contains(cert.ExtKeyUsage, x509.ExtKeyUsageClientAuth) {
+				t.Error("missing ExtKeyUsageClientAuth")
+			}
+			if !slices.Contains(cert.ExtKeyUsage, x509.ExtKeyUsageServerAuth) {
+				t.Error("missing ExtKeyUsageServerAuth")
+			}
+
+			// Verify hostnames in SANs
+			if len(cert.DNSNames) == 0 {
+				t.Error("expected DNS SANs")
+			}
+
+			// Verify Subject based on path
+			if tc.keyGen != nil {
+				// ConfigurablePKI path: UserInfo encoded in Subject
+				if cert.Subject.CommonName != tc.userInfo.GetName() {
+					t.Errorf("CN = %q, want %q", cert.Subject.CommonName, tc.userInfo.GetName())
+				}
+				if len(tc.userInfo.GetGroups()) > 0 && len(cert.Subject.Organization) == 0 {
+					t.Error("expected Organization from UserInfo.Groups")
+				}
+			} else {
+				// Legacy path: CN = sorted first hostname (MakeServerCertForDuration sorts)
+				if cert.Subject.CommonName != tc.userInfo.GetName() {
+					t.Errorf("CN = %q, want %q (must match UserInfo.Name which must match sorted first hostname)", cert.Subject.CommonName, tc.userInfo.GetName())
+				}
+			}
+		})
+	}
+}
+
 func TestSignerRotation_NewCertificate_WithKeyPairGenerator(t *testing.T) {
 	testCases := []struct {
 		name    string
@@ -808,6 +930,16 @@ func TestSignerRotation_NewCertificate_WithKeyPairGenerator(t *testing.T) {
 			keyGen:  nil,
 			wantAlg: x509.RSA,
 		},
+		{
+			name:    "RSA-4096",
+			keyGen:  crypto.RSAKeyPairGenerator{Bits: 4096},
+			wantAlg: x509.RSA,
+		},
+		{
+			name:    "ECDSA-P256",
+			keyGen:  crypto.ECDSAKeyPairGenerator{Curve: crypto.P256},
+			wantAlg: x509.ECDSA,
+		},
 		{
 			name:    "ECDSA-P384",
 			keyGen:  crypto.ECDSAKeyPairGenerator{Curve: crypto.P384},