From ede76c3c25fdf48f03f78dde597db4aec26f1f7b Mon Sep 17 00:00:00 2001 From: irbekrm Date: Thu, 9 Jun 2022 14:41:35 +0100 Subject: [PATCH 1/2] Clarifies the warning if private key cannot be regenerated, but spec has changed Signed-off-by: irbekrm --- .../certificates/keymanager/keymanager_controller.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/controller/certificates/keymanager/keymanager_controller.go b/pkg/controller/certificates/keymanager/keymanager_controller.go index a8d5347f2..a6aa2788a 100644 --- a/pkg/controller/certificates/keymanager/keymanager_controller.go +++ b/pkg/controller/certificates/keymanager/keymanager_controller.go @@ -52,9 +52,10 @@ import ( ) const ( - ControllerName = "certificates-key-manager" - reasonDecodeFailed = "DecodeFailed" - reasonDeleted = "Deleted" + ControllerName = "certificates-key-manager" + reasonDecodeFailed = "DecodeFailed" + reasonCannotRegenerateKey = "CannotRegenerateKey" + reasonDeleted = "Deleted" ) var ( @@ -259,7 +260,7 @@ func (c *controller) createNextPrivateKeyRotationPolicyNever(ctx context.Context return c.createAndSetNextPrivateKey(ctx, crt) } if len(violations) > 0 { - c.recorder.Eventf(crt, corev1.EventTypeWarning, reasonDecodeFailed, "Existing private key in Secret %q does not match requirements on Certificate resource, mismatching fields: %v", crt.Spec.SecretName, violations) + c.recorder.Eventf(crt, corev1.EventTypeWarning, reasonCannotRegenerateKey, "User intervention required: existing private key in Secret %q does not match requirements on Certificate resource, mismatching fields: %v, but cert-manager cannot create new private key as the Certificate's .spec.privateKey.rotationPolicy is unset or set to Never. To allow cert-manager to create a new private key you can set .spec.privateKey.rotationPolicy to 'Always' (this will result in the private key being regenerated every time a cert is renewed) ", crt.Spec.SecretName, violations) return nil } From bb124a0f6173da9942adae9eed8854da413c4ca1 Mon Sep 17 00:00:00 2001 From: irbekrm Date: Thu, 9 Jun 2022 14:44:57 +0100 Subject: [PATCH 2/2] Corrects the cert.spec.privateKey path in logs Signed-off-by: irbekrm --- .../keymanager/keymanager_controller_test.go | 2 +- pkg/controller/certificates/util.go | 10 +++++----- pkg/controller/certificates/util_test.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/controller/certificates/keymanager/keymanager_controller_test.go b/pkg/controller/certificates/keymanager/keymanager_controller_test.go index 8fde32541..c417932cf 100644 --- a/pkg/controller/certificates/keymanager/keymanager_controller_test.go +++ b/pkg/controller/certificates/keymanager/keymanager_controller_test.go @@ -435,7 +435,7 @@ func TestProcessItem(t *testing.T) { secrets: []runtime.Object{ ownedSecretWithName("testns", "fixed-name", "test", map[string][]byte{"tls.key": mustGenerateECDSA(t, pki.ECCurve256)}), }, - expectedEvents: []string{"Normal Deleted Regenerating private key due to change in fields: [spec.keyAlgorithm]"}, + expectedEvents: []string{"Normal Deleted Regenerating private key due to change in fields: [spec.privateKey.algorithm]"}, expectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewDeleteAction( corev1.SchemeGroupVersion.WithResource("secrets"), diff --git a/pkg/controller/certificates/util.go b/pkg/controller/certificates/util.go index 074dde310..0bfedf9e6 100644 --- a/pkg/controller/certificates/util.go +++ b/pkg/controller/certificates/util.go @@ -60,7 +60,7 @@ func PrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) ([] func rsaPrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) ([]string, error) { rsaPk, ok := pk.(*rsa.PrivateKey) if !ok { - return []string{"spec.keyAlgorithm"}, nil + return []string{"spec.privateKey.algorithm"}, nil } var violations []string // TODO: we should not use implicit defaulting here, and instead rely on @@ -73,7 +73,7 @@ func rsaPrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) keySize = spec.PrivateKey.Size } if rsaPk.N.BitLen() != keySize { - violations = append(violations, "spec.keySize") + violations = append(violations, "spec.privateKey.size") } return violations, nil } @@ -81,7 +81,7 @@ func rsaPrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) func ecdsaPrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) ([]string, error) { ecdsaPk, ok := pk.(*ecdsa.PrivateKey) if !ok { - return []string{"spec.keyAlgorithm"}, nil + return []string{"spec.privateKey.algorithm"}, nil } var violations []string // TODO: we should not use implicit defaulting here, and instead rely on @@ -94,7 +94,7 @@ func ecdsaPrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec expectedKeySize = spec.PrivateKey.Size } if expectedKeySize != ecdsaPk.Curve.Params().BitSize { - violations = append(violations, "spec.keySize") + violations = append(violations, "spec.privateKey.size") } return violations, nil } @@ -102,7 +102,7 @@ func ecdsaPrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec func ed25519PrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) ([]string, error) { _, ok := pk.(ed25519.PrivateKey) if !ok { - return []string{"spec.keyAlgorithm"}, nil + return []string{"spec.privateKey.algorithm"}, nil } return nil, nil diff --git a/pkg/controller/certificates/util_test.go b/pkg/controller/certificates/util_test.go index 9e2694b62..1446fc2e0 100644 --- a/pkg/controller/certificates/util_test.go +++ b/pkg/controller/certificates/util_test.go @@ -72,7 +72,7 @@ func TestPrivateKeyMatchesSpec(t *testing.T) { key: mustGenerateRSA(t, 2048), expectedAlgo: cmapi.RSAKeyAlgorithm, expectedSize: 4096, - violations: []string{"spec.keySize"}, + violations: []string{"spec.privateKey.size"}, }, "should match if keySize and algorithm are correct (ECDSA)": { key: mustGenerateECDSA(t, pki.ECCurve256), @@ -83,13 +83,13 @@ func TestPrivateKeyMatchesSpec(t *testing.T) { key: mustGenerateECDSA(t, pki.ECCurve256), expectedAlgo: cmapi.ECDSAKeyAlgorithm, expectedSize: pki.ECCurve521, - violations: []string{"spec.keySize"}, + violations: []string{"spec.privateKey.size"}, }, "should not match if keyAlgorithm is incorrect": { key: mustGenerateECDSA(t, pki.ECCurve256), expectedAlgo: cmapi.RSAKeyAlgorithm, expectedSize: 2048, - violations: []string{"spec.keyAlgorithm"}, + violations: []string{"spec.privateKey.algorithm"}, }, "should match if keySize and algorithm are correct (Ed25519)": { key: mustGenerateEd25519(t),