NO-JIRA: 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

@@ -436,10 +436,12 @@ func (r *SignerRotation) NewCertificate(signer *crypto.CA, validity time.Duratio
 // (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,18 +458,22 @@ func (r *PeerRotation) NewCertificate(signer *crypto.CA, validity time.Duration,
 	if len(hostnames) == 0 {
 		return nil, fmt.Errorf("no hostnames set")
 	}
+	if r.UserInfo == nil {
+		return nil, fmt.Errorf("PeerRotation requires UserInfo")
+	}
 	if keyGen != nil {
-		if r.UserInfo == nil {
-			return nil, fmt.Errorf("PeerRotation requires UserInfo for configurable PKI certificates")
-		}
 		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).
+	// 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

pkg/operator/certrotation/target_test.go

@@ -1,7 +1,6 @@
 package certrotation
 
 import (
-	"context"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"strings"
@@ -302,7 +301,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 +492,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)
@@ -797,6 +796,123 @@ 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
+			hasClientAuth := false
+			hasServerAuth := false
+			for _, usage := range cert.ExtKeyUsage {
+				if usage == x509.ExtKeyUsageClientAuth {
+					hasClientAuth = true
+				}
+				if usage == x509.ExtKeyUsageServerAuth {
+					hasServerAuth = true
+				}
+			}
+			if !hasClientAuth {
+				t.Error("missing ExtKeyUsageClientAuth")
+			}
+			if !hasServerAuth {
+				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