addressed feedback from review

README.md

@@ -356,7 +356,7 @@ In this case the secret `my-secret` will contains the labels.
 
 ### Specifying private key algorithm and size
 
-By default, the certificate uses `RSA` with a key size of 2048 bits for the private key
+By default, the certificate uses `RSA` with a key size of 2048 bits for the private key.
 Add the `privateKey` section to specify private key algorithm and/or size.
 
 Example:

charts/cert-management/templates/cert.gardener.cloud_certificates.yaml

@@ -204,6 +204,13 @@ spec:
                       to `2048` if not specified. If `algorithm` is set to `ECDSA`,
                       valid values are `256` or `384`, and will default to `256` if
                       not specified. No other values are allowed."
+                    enum:
+                    - 256
+                    - 384
+                    - 2048
+                    - 3072
+                    - 4096
+                    format: int32
                     type: integer
                 type: object
               renew:

pkg/apis/cert/crds/cert.gardener.cloud_certificates.yaml

@@ -199,6 +199,13 @@ spec:
                       to `2048` if not specified. If `algorithm` is set to `ECDSA`,
                       valid values are `256` or `384`, and will default to `256` if
                       not specified. No other values are allowed."
+                    enum:
+                    - 256
+                    - 384
+                    - 2048
+                    - 3072
+                    - 4096
+                    format: int32
                     type: integer
                 type: object
               renew:

pkg/apis/cert/crds/zz_generated_crds.go

@@ -499,6 +499,13 @@ spec:
                       to ` + "`" + `2048` + "`" + ` if not specified. If ` + "`" + `algorithm` + "`" + ` is set to ` + "`" + `ECDSA` + "`" + `,
                       valid values are ` + "`" + `256` + "`" + ` or ` + "`" + `384` + "`" + `, and will default to ` + "`" + `256` + "`" + ` if
                       not specified. No other values are allowed."
+                    enum:
+                    - 256
+                    - 384
+                    - 2048
+                    - 3072
+                    - 4096
+                    format: int32
                     type: integer
                 type: object
               renew:

pkg/apis/cert/v1alpha1/types.go

@@ -108,6 +108,10 @@ const (
 	ECDSAKeyAlgorithm PrivateKeyAlgorithm = "ECDSA"
 )
 
+// PrivateKeySize is the size for the algorithm
+// +kubebuilder:validation:Enum=256;384;2048;3072;4096
+type PrivateKeySize int32
+
 // CertificatePrivateKey contains configuration options for private keys
 // used by the Certificate controller.
 // These include the key algorithm and size.
@@ -120,7 +124,7 @@ type CertificatePrivateKey struct {
 	// key size of 2048 will be used for `RSA` key algorithm and
 	// key size of 256 will be used for `ECDSA` key algorithm.
 	// +optional
-	Algorithm PrivateKeyAlgorithm `json:"algorithm,omitempty"`
+	Algorithm *PrivateKeyAlgorithm `json:"algorithm,omitempty"`
 
 	// Size is the key bit size of the corresponding private key for this certificate.
 	//
@@ -130,7 +134,7 @@ type CertificatePrivateKey struct {
 	// and will default to `256` if not specified.
 	// No other values are allowed.
 	// +optional
-	Size int `json:"size,omitempty"`
+	Size *PrivateKeySize `json:"size,omitempty"`
 }
 
 // BackOffState stores the status for exponential back off on repeated cert request failure

pkg/cert/legobridge/certificate.go

@@ -14,10 +14,11 @@ import (
 	"sync"
 	"time"
 
-	"github.com/gardener/cert-management/pkg/apis/cert/v1alpha1"
+	api "github.com/gardener/cert-management/pkg/apis/cert/v1alpha1"
 	"github.com/gardener/cert-management/pkg/cert/metrics"
 	"github.com/gardener/cert-management/pkg/cert/utils"
 	"github.com/go-acme/lego/v4/certcrypto"
+	"k8s.io/utils/ptr"
 
 	"github.com/go-acme/lego/v4/certificate"
 	"github.com/go-acme/lego/v4/challenge/dns01"
@@ -148,56 +149,68 @@ func obtainForDomains(client *lego.Client, domains []string, input ObtainInput)
 }
 
 // ToKeyType extracts the key type from the
-func ToKeyType(privateKeySpec *v1alpha1.CertificatePrivateKey) (certcrypto.KeyType, error) {
+func ToKeyType(privateKeySpec *api.CertificatePrivateKey) (certcrypto.KeyType, error) {
 	keyType := certcrypto.RSA2048
 	if privateKeySpec != nil {
-		switch privateKeySpec.Algorithm {
-		case "", v1alpha1.RSAKeyAlgorithm:
-			switch privateKeySpec.Size {
+		algorithm := api.RSAKeyAlgorithm
+		if privateKeySpec.Algorithm != nil && *privateKeySpec.Algorithm != "" {
+			algorithm = *privateKeySpec.Algorithm
+		}
+		size := 0
+		if privateKeySpec.Size != nil && *privateKeySpec.Size != 0 {
+			size = int(*privateKeySpec.Size)
+		}
+		switch algorithm {
+		case api.RSAKeyAlgorithm:
+			switch size {
 			case 0, 2048:
 				keyType = certcrypto.RSA2048
 			case 3072:
 				keyType = certcrypto.RSA3072
 			case 4096:
 				keyType = certcrypto.RSA4096
 			default:
-				return "", fmt.Errorf("invalid key size for RSA: %d (allowed values are 2048, 3072, and 4096)", privateKeySpec.Size)
+				return "", fmt.Errorf("invalid key size for RSA: %d (allowed values are 2048, 3072, and 4096)", size)
 			}
-		case v1alpha1.ECDSAKeyAlgorithm:
-			switch privateKeySpec.Size {
+		case api.ECDSAKeyAlgorithm:
+			switch size {
 			case 0, 256:
 				keyType = certcrypto.EC256
 			case 384:
 				keyType = certcrypto.EC384
 			default:
-				return "", fmt.Errorf("invalid key size for ECDSA: %d (allowed values are 256 and 384)", privateKeySpec.Size)
+				return "", fmt.Errorf("invalid key size for ECDSA: %d (allowed values are 256 and 384)", size)
 			}
 		default:
 			return "", fmt.Errorf("invalid private key algorithm %s (allowed values are '%s' and '%s')",
-				privateKeySpec.Algorithm, v1alpha1.RSAKeyAlgorithm, v1alpha1.ECDSAKeyAlgorithm)
+				algorithm, api.RSAKeyAlgorithm, api.ECDSAKeyAlgorithm)
 		}
 	}
 	return keyType, nil
 }
 
 // FromKeyType converts key type back to
-func FromKeyType(keyType certcrypto.KeyType) *v1alpha1.CertificatePrivateKey {
+func FromKeyType(keyType certcrypto.KeyType) *api.CertificatePrivateKey {
 	switch keyType {
 	case certcrypto.RSA2048:
-		return &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 2048}
+		return newCertificatePrivateKey(api.RSAKeyAlgorithm, 2048)
 	case certcrypto.RSA3072:
-		return &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 3072}
+		return newCertificatePrivateKey(api.RSAKeyAlgorithm, 3072)
 	case certcrypto.RSA4096:
-		return &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 4096}
+		return newCertificatePrivateKey(api.RSAKeyAlgorithm, 4096)
 	case certcrypto.EC256:
-		return &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.ECDSAKeyAlgorithm, Size: 256}
+		return newCertificatePrivateKey(api.ECDSAKeyAlgorithm, 256)
 	case certcrypto.EC384:
-		return &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.ECDSAKeyAlgorithm, Size: 384}
+		return newCertificatePrivateKey(api.ECDSAKeyAlgorithm, 384)
 	default:
 		return nil
 	}
 }
 
+func newCertificatePrivateKey(algorithm api.PrivateKeyAlgorithm, size api.PrivateKeySize) *api.CertificatePrivateKey {
+	return &api.CertificatePrivateKey{Algorithm: ptr.To(algorithm), Size: ptr.To(size)}
+}
+
 func obtainForCSR(client *lego.Client, csr []byte, input ObtainInput) (*certificate.Resource, error) {
 	cert, err := extractCertificateRequest(csr)
 	if err != nil {

pkg/cert/legobridge/certificate_test.go

@@ -7,14 +7,19 @@
 package legobridge
 
 import (
-	"github.com/gardener/cert-management/pkg/apis/cert/v1alpha1"
+	api "github.com/gardener/cert-management/pkg/apis/cert/v1alpha1"
+
 	"github.com/go-acme/lego/v4/certcrypto"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 var _ = DescribeTable("KeyType conversion",
-	func(keyType certcrypto.KeyType, key *v1alpha1.CertificatePrivateKey) {
+	func(keyType certcrypto.KeyType, algorithm api.PrivateKeyAlgorithm, size int) {
+		var key *api.CertificatePrivateKey
+		if size >= 0 {
+			key = newCertificatePrivateKey(algorithm, api.PrivateKeySize(size))
+		}
 		actualKeyType, err := ToKeyType(key)
 		if keyType == "" {
 			Expect(err).To(HaveOccurred())
@@ -26,14 +31,15 @@ var _ = DescribeTable("KeyType conversion",
 			Expect(actualKeyType).To(Equal(keyType))
 		}
 	},
-	Entry("default", certcrypto.RSA2048, nil),
-	Entry("RSA with default size", certcrypto.RSA2048, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm}),
-	Entry("RSA2048", certcrypto.RSA2048, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 2048}),
-	Entry("RSA3072", certcrypto.RSA3072, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 3072}),
-	Entry("RSA4096", certcrypto.RSA4096, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 4096}),
-	Entry("ECDSA with default size", certcrypto.EC256, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.ECDSAKeyAlgorithm}),
-	Entry("EC256", certcrypto.EC256, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.ECDSAKeyAlgorithm, Size: 256}),
-	Entry("EC384", certcrypto.EC384, &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.ECDSAKeyAlgorithm, Size: 384}),
-	Entry("RSA with wrong size", certcrypto.KeyType(""), &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.RSAKeyAlgorithm, Size: 8192}),
-	Entry("ECDSA with wrong size", certcrypto.KeyType(""), &v1alpha1.CertificatePrivateKey{Algorithm: v1alpha1.ECDSAKeyAlgorithm, Size: 511}),
+	Entry("default", certcrypto.RSA2048, api.PrivateKeyAlgorithm(""), -1),
+	Entry("empty", certcrypto.RSA2048, api.PrivateKeyAlgorithm(""), 0),
+	Entry("RSA from empty config", certcrypto.RSA2048, api.RSAKeyAlgorithm, 0),
+	Entry("RSA2048", certcrypto.RSA2048, api.RSAKeyAlgorithm, 2048),
+	Entry("RSA3072", certcrypto.RSA3072, api.RSAKeyAlgorithm, 3072),
+	Entry("RSA4096", certcrypto.RSA4096, api.RSAKeyAlgorithm, 4096),
+	Entry("ECDSA with default size", certcrypto.EC256, api.ECDSAKeyAlgorithm, 0),
+	Entry("EC256", certcrypto.EC256, api.ECDSAKeyAlgorithm, 256),
+	Entry("EC384", certcrypto.EC384, api.ECDSAKeyAlgorithm, 384),
+	Entry("RSA with wrong size", certcrypto.KeyType(""), api.RSAKeyAlgorithm, 8192),
+	Entry("ECDSA with wrong size", certcrypto.KeyType(""), api.ECDSAKeyAlgorithm, 511),
 )

pkg/cert/source/reconciler.go

@@ -14,6 +14,7 @@ import (
 
 	core "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/utils/ptr"
 
 	"github.com/gardener/controller-manager-library/pkg/controllermanager/controller"
 	"github.com/gardener/controller-manager-library/pkg/controllermanager/controller/reconcile"
@@ -350,12 +351,7 @@ func (r *sourceReconciler) createEntryFor(logger logger.LogContext, obj resource
 		cert.Spec.PreferredChain = &info.PreferredChain
 	}
 
-	if info.PrivateKeyAlgorithm != "" || info.PrivateKeySize != 0 {
-		cert.Spec.PrivateKey = &api.CertificatePrivateKey{
-			Algorithm: api.PrivateKeyAlgorithm(info.PrivateKeyAlgorithm),
-			Size:      info.PrivateKeySize,
-		}
-	}
+	cert.Spec.PrivateKey = createPrivateKey(info.PrivateKeyAlgorithm, info.PrivateKeySize)
 
 	e, _ := r.SlaveResoures()[0].Wrap(cert)
 
@@ -459,21 +455,9 @@ func (r *sourceReconciler) updateEntry(logger logger.LogContext, info CertInfo,
 			mod.Modify(true)
 		}
 
-		oldAlgorithm := ""
-		oldKeySize := 0
-		if spec.PrivateKey != nil {
-			oldAlgorithm = string(spec.PrivateKey.Algorithm)
-			oldKeySize = spec.PrivateKey.Size
-		}
-		if info.PrivateKeyAlgorithm != oldAlgorithm || info.PrivateKeySize != oldKeySize {
-			if info.PrivateKeyAlgorithm != "" || info.PrivateKeySize != 0 {
-				spec.PrivateKey = &api.CertificatePrivateKey{
-					Algorithm: api.PrivateKeyAlgorithm(info.PrivateKeyAlgorithm),
-					Size:      info.PrivateKeySize,
-				}
-			} else {
-				spec.PrivateKey = nil
-			}
+		newPrivateKey := createPrivateKey(info.PrivateKeyAlgorithm, info.PrivateKeySize)
+		if !reflect.DeepEqual(spec.PrivateKey, newPrivateKey) {
+			spec.PrivateKey = newPrivateKey
 			mod.Modify(true)
 		}
 
@@ -484,3 +468,17 @@ func (r *sourceReconciler) updateEntry(logger logger.LogContext, info CertInfo,
 	}
 	return obj.Modify(f)
 }
+
+func createPrivateKey(algorithm string, size int) *api.CertificatePrivateKey {
+	if algorithm == "" && size == 0 {
+		return nil
+	}
+	obj := &api.CertificatePrivateKey{}
+	if algorithm != "" {
+		obj.Algorithm = ptr.To(api.PrivateKeyAlgorithm(algorithm))
+	}
+	if size != 0 {
+		obj.Size = ptr.To(api.PrivateKeySize(size))
+	}
+	return obj
+}