From dfd1e512d87f81f226cd3e2f7cba4edbcef59b4a Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 9 Aug 2022 11:12:41 +0100 Subject: [PATCH 1/5] Update CertificateSigningRequest controller to accept a list of RegisterExtraInformerFn, which control the extra informers. Signed-off-by: joshvanl --- .../certificatesigningrequests/controller.go | 68 ++++++++----------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/pkg/controller/certificatesigningrequests/controller.go b/pkg/controller/certificatesigningrequests/controller.go index d8f44f62f..ea9c6bc87 100644 --- a/pkg/controller/certificatesigningrequests/controller.go +++ b/pkg/controller/certificatesigningrequests/controller.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" certificatesv1 "k8s.io/api/certificates/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" authzclient "k8s.io/client-go/kubernetes/typed/authorization/v1" certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1" certificateslisters "k8s.io/client-go/listers/certificates/v1" @@ -50,6 +49,13 @@ type Signer interface { // context. type SignerConstructor func(*controllerpkg.Context) Signer +// RegisterExtraInformerFn is a function used by CertificateSigningRequest +// controller implementations to add custom workqueue functions based on +// informers not covered in the main shared controller implementation. +// The returned set of InformerSyncs will be waited on when the controller +// starts. +type RegisterExtraInformerFn func(*controllerpkg.Context, logr.Logger, workqueue.RateLimitingInterface) ([]cache.InformerSynced, error) + // Controller is a base Kubernetes CertificateSigningRequest controller. It is // responsible for orchestrating and performing shared operations that all // CertificateSigningRequest controllers do, before passing the @@ -81,9 +87,9 @@ type Controller struct { // the signer kind to react to when a certificate signing request is synced signerType string - // extraInformerResources are the set of resources which should cause - // reconciles if owned by a CertifcateRequest. - extraInformerResources []schema.GroupVersionResource + //registerExtraInformers is a list of functions that + //CertificateSigningRequest controllers can use to register custom informers. + registerExtraInformers []RegisterExtraInformerFn // used for testing clock clock.Clock @@ -91,20 +97,19 @@ type Controller struct { // New will construct a new certificatesigningrequest controller using the // given Signer implementation. -// Note: the extraInformers passed here will be 'waited' for when starting to -// ensure their corresponding listers have synced. -// An event handler will then be set on these informers that automatically -// resyncs CertificateSigningRequest resources that 'own' the objects in the -// informer. -// It's the callers responsibility to ensure the Run function on the informer -// is called in order to start the reflector. This is handled automatically -// when the informer factory's Start method is called, if the given informer -// was obtained using a SharedInformerFactory. -func New(signerType string, signerConstructor SignerConstructor, extraInformerResources ...schema.GroupVersionResource) *Controller { +// Note: the registerExtraInfromers passed here will be 'waited' for when +// starting to ensure their corresponding listers have synced. +// The caller is responsible for ensuring the informer work functions are setup +// correctly on any informer. +// It's also the callers responsibility to ensure the Run function on the +// informer is called in order to start the reflector. This is handled +// automatically when the informer factory's Start method is called, if the +// given informer was obtained using a SharedInformerFactory. +func New(signerType string, signerConstructor SignerConstructor, registerExtraInformers ...RegisterExtraInformerFn) *Controller { return &Controller{ signerType: signerType, signerConstructor: signerConstructor, - extraInformerResources: extraInformerResources, + registerExtraInformers: registerExtraInformers, } } @@ -125,27 +130,21 @@ func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin // obtain references to all the informers used by this controller csrInformer := ctx.KubeSharedInformerFactory.Certificates().V1().CertificateSigningRequests() - // build a list of InformerSynced functions that will be returned by the Register method. - // the controller will only begin processing items once all of these informers have synced. + // build a list of InformerSynced functions that will be returned by the + // Register method. The controller will only begin processing items once all + // of these informers have synced. + mustSync := []cache.InformerSynced{ csrInformer.Informer().HasSynced, issuerInformer.Informer().HasSynced, } - // Ensure we also catch all extra informers for this - // CertificateSigningRequest controller instance. - var extraInformers []cache.SharedIndexInformer - for _, i := range c.extraInformerResources { - // TODO (joshvanl): currently we only have an extra informer for - // cert-manager Orders. If extended to other informer factory sets, add a - // switch statement here for the group so that the correct shared informer - // can be selected. - extraInformer, err := ctx.SharedInformerFactory.ForResource(i) + for _, reg := range c.registerExtraInformers { + ms, err := reg(ctx, c.log, c.queue) if err != nil { - return nil, nil, fmt.Errorf("failed to get extra informer for %v: %w", i, err) + return nil, nil, fmt.Errorf("failed to register extra informer: %w", err) } - extraInformers = append(extraInformers, extraInformer.Informer()) - mustSync = append(mustSync, extraInformer.Informer().HasSynced) + mustSync = append(mustSync, ms...) } // if scoped to a single namespace @@ -165,17 +164,6 @@ func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin csrInformer.Informer().AddEventHandler(&controllerpkg.QueuingEventHandler{Queue: c.queue}) issuerInformer.Informer().AddEventHandler(&controllerpkg.BlockingEventHandler{WorkFunc: c.handleGenericIssuer}) - // Ensure we catch extra informers that are owned by certificate signing requests - for _, i := range extraInformers { - i.AddEventHandler(&controllerpkg.BlockingEventHandler{ - WorkFunc: controllerpkg.HandleOwnedResourceNamespacedFunc(c.log, c.queue, - schema.GroupVersionKind{Version: "v1", Group: "certificates.k8s.io", Kind: "CertificateSigningRequest"}, - func(_, name string) (interface{}, error) { - return c.csrLister.Get(name) - }), - }) - } - // create an issuer helper for reading generic issuers c.helper = issuer.NewHelper(issuerInformer.Lister(), clusterIssuerInformer.Lister()) From b03e6f11f528e104c419042b060ba9a1a595aaec Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 9 Aug 2022 11:13:29 +0100 Subject: [PATCH 2/5] Updates ACME CertificateSigningRequest for new informer registration format Signed-off-by: joshvanl --- .../certificatesigningrequests/acme/acme.go | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/controller/certificatesigningrequests/acme/acme.go b/pkg/controller/certificatesigningrequests/acme/acme.go index 8d4eefd9d..f9461ec8e 100644 --- a/pkg/controller/certificatesigningrequests/acme/acme.go +++ b/pkg/controller/certificatesigningrequests/acme/acme.go @@ -22,13 +22,16 @@ import ( "errors" "fmt" + "github.com/go-logr/logr" certificatesv1 "k8s.io/api/certificates/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1" + "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/workqueue" "github.com/cert-manager/cert-manager/pkg/acme" apiutil "github.com/cert-manager/cert-manager/pkg/api/util" @@ -76,7 +79,23 @@ func init() { } func controllerBuilder() *certificatesigningrequests.Controller { - return certificatesigningrequests.New(apiutil.IssuerACME, NewACME, cmacme.SchemeGroupVersion.WithResource("orders")) + return certificatesigningrequests.New(apiutil.IssuerACME, NewACME, + func(ctx *controllerpkg.Context, log logr.Logger, queue workqueue.RateLimitingInterface) ([]cache.InformerSynced, error) { + orderInformer := ctx.SharedInformerFactory.Acme().V1().Orders().Informer() + csrLister := ctx.KubeSharedInformerFactory.Certificates().V1().CertificateSigningRequests().Lister() + + orderInformer.AddEventHandler(&controllerpkg.BlockingEventHandler{ + WorkFunc: controllerpkg.HandleOwnedResourceNamespacedFunc( + log, queue, + certificatesv1.SchemeGroupVersion.WithKind("CertificateSigningRequest"), + func(_, name string) (interface{}, error) { + return csrLister.Get(name) + }, + ), + }) + return []cache.InformerSynced{orderInformer.HasSynced}, nil + }, + ) } func NewACME(ctx *controllerpkg.Context) certificatesigningrequests.Signer { From fc9554a6170b0b33abc0655c85b06d510fba96f9 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 9 Aug 2022 11:14:22 +0100 Subject: [PATCH 3/5] Adds secret handler function for selfsigned CertificateSigningRequest controller, which re-syncs requests which reference the Secret via the `experimental.cert-manager.io/private-key-secret-name` annotation. Signed-off-by: joshvanl --- .../selfsigned/checks.go | 144 ++++++++ .../selfsigned/checks_test.go | 349 ++++++++++++++++++ 2 files changed, 493 insertions(+) create mode 100644 pkg/controller/certificatesigningrequests/selfsigned/checks.go create mode 100644 pkg/controller/certificatesigningrequests/selfsigned/checks_test.go diff --git a/pkg/controller/certificatesigningrequests/selfsigned/checks.go b/pkg/controller/certificatesigningrequests/selfsigned/checks.go new file mode 100644 index 000000000..c6d2073c5 --- /dev/null +++ b/pkg/controller/certificatesigningrequests/selfsigned/checks.go @@ -0,0 +1,144 @@ +/* +Copyright 2022 The cert-manager 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 selfsigned + +import ( + "fmt" + + "github.com/go-logr/logr" + certificatesv1 "k8s.io/api/certificates/v1" + corev1 "k8s.io/api/core/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" + clientv1 "k8s.io/client-go/listers/certificates/v1" + "k8s.io/client-go/util/workqueue" + + apiutil "github.com/cert-manager/cert-manager/pkg/api/util" + cmexperimental "github.com/cert-manager/cert-manager/pkg/apis/experimental/v1alpha1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + controllerpkg "github.com/cert-manager/cert-manager/pkg/controller" + "github.com/cert-manager/cert-manager/pkg/controller/certificatesigningrequests/util" + "github.com/cert-manager/cert-manager/pkg/issuer" + logf "github.com/cert-manager/cert-manager/pkg/logs" +) + +// handleSecretReferenceWorkFunc is a function that returns am informer event +// handler work function, which is used to sync CertificateSigningRequests that +// reference the synced Secret through the +// "experimental.cert-manager.io/private-key-secret-name" annotation. +func handleSecretReferenceWorkFunc(log logr.Logger, + lister clientv1.CertificateSigningRequestLister, + helper issuer.Helper, + queue workqueue.RateLimitingInterface, + issuerOptions controllerpkg.IssuerOptions, +) func(obj any) { + return func(obj any) { + log := log.WithName("handleSecretReference") + secret, ok := obj.(*corev1.Secret) + if !ok { + log.Error(nil, "object is not a secret") + return + } + log = logf.WithResource(log, secret) + requests, err := certificateSigningRequestsForSecret(log, lister, helper, secret, issuerOptions) + if err != nil { + log.Error(err, "failed to determine affected certificate signing requests") + return + } + for _, request := range requests { + log := logf.WithRelatedResource(log, request) + key, err := controllerpkg.KeyFunc(request) + if err != nil { + log.Error(err, "error computing key for resource") + continue + } + queue.Add(key) + } + } +} + +// certificateSigningRequestsForSecret returns a list of +// CertificateSigningRequests which reference an issuer in the same Namespace +// as the Secret (the resource Namespace in the case of ClusterIssuer) via the +// "experimental.cert-manager.io/private-key-secret-name" annotation, and the +// request targets a SelfSigned Issuer or Cluster Issuer. +func certificateSigningRequestsForSecret(log logr.Logger, + lister clientv1.CertificateSigningRequestLister, + helper issuer.Helper, + secret *corev1.Secret, + issuerOptions controllerpkg.IssuerOptions, +) ([]*certificatesv1.CertificateSigningRequest, error) { + dbg := log.V(logf.DebugLevel) + requests, err := lister.List(labels.Everything()) + if err != nil { + return nil, fmt.Errorf("failed to list certificate requests: %w", err) + } + + dbg.Info("checking if self signed certificate signing requests reference secret") + var affected []*certificatesv1.CertificateSigningRequest + for _, request := range requests { + ref, ok := util.SignerIssuerRefFromSignerName(request.Spec.SignerName) + if !ok { + dbg.Info("certificate signing request has malformed signer name,", "signerName", request.Spec.SignerName) + continue + } + + kind, ok := util.IssuerKindFromType(ref.Type) + if !ok { + dbg.Info("certificate signing request signerName type does not match 'issuers' or 'clusterissuers' so skipping processing") + continue + } + + issuerObj, err := helper.GetGenericIssuer(cmmeta.ObjectReference{ + Name: ref.Name, + Kind: kind, + Group: ref.Group, + }, ref.Namespace) + if k8sErrors.IsNotFound(err) { + dbg.Info("issuer not found, skipping") + continue + } + + if err != nil { + log.Error(err, "failed to get issuer") + return nil, err + } + + dbg = logf.WithRelatedResource(dbg, issuerObj) + + if secret.Namespace != issuerOptions.ResourceNamespace(issuerObj) { + dbg.Info("issuer is not in the same namespace scope as the secret, skipping") + continue + } + + dbg.Info("ensuring issuer type matches this controller") + + issuerType, err := apiutil.NameForIssuer(issuerObj) + if err != nil { + dbg.Error(err, "failed to determine issuer type, skipping") + continue + } + + if issuerType == apiutil.IssuerSelfSigned && + request.GetAnnotations()[cmexperimental.CertificateSigningRequestPrivateKeyAnnotationKey] == secret.Name { + dbg.Info("certificate request references secret, syncing") + affected = append(affected, request) + } + } + + return affected, nil +} diff --git a/pkg/controller/certificatesigningrequests/selfsigned/checks_test.go b/pkg/controller/certificatesigningrequests/selfsigned/checks_test.go new file mode 100644 index 000000000..fdb9f3a32 --- /dev/null +++ b/pkg/controller/certificatesigningrequests/selfsigned/checks_test.go @@ -0,0 +1,349 @@ +/* +Copyright 2022 The cert-manager 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 selfsigned + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + certificatesv1 "k8s.io/api/certificates/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/workqueue" + "k8s.io/klog/v2/klogr" + + cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + controllerpkg "github.com/cert-manager/cert-manager/pkg/controller" + testpkg "github.com/cert-manager/cert-manager/pkg/controller/test" + "github.com/cert-manager/cert-manager/pkg/issuer" + "github.com/cert-manager/cert-manager/test/unit/gen" +) + +func Test_handleSecretReferenceWorkFunc(t *testing.T) { + tests := map[string]struct { + secret runtime.Object + existingCSRs []runtime.Object + existingIssuers []runtime.Object + expectedQueue []string + }{ + "if given object is not secret, expect empty queue": { + secret: gen.Certificate("not-a-secret"), + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + expectedQueue: []string{}, + }, + "if no requests then expect empty queue": { + secret: gen.Secret("test-secret", gen.SetSecretNamespace("test-namespace")), + existingCSRs: []runtime.Object{}, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + expectedQueue: []string{}, + }, + "referenced requests should be added to the queue": { + secret: gen.Secret("test-secret", gen.SetSecretNamespace("test-namespace")), + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + expectedQueue: []string{"a", "b"}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + builder := &testpkg.Builder{ + T: t, + KubeObjects: test.existingCSRs, + CertManagerObjects: test.existingIssuers, + } + defer builder.Stop() + builder.Init() + + lister := builder.Context.KubeSharedInformerFactory.Certificates().V1().CertificateSigningRequests().Lister() + helper := issuer.NewHelper( + builder.Context.SharedInformerFactory.Certmanager().V1().Issuers().Lister(), + builder.Context.SharedInformerFactory.Certmanager().V1().ClusterIssuers().Lister(), + ) + + builder.Start() + + queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) + handleSecretReferenceWorkFunc(klogr.New(), lister, helper, queue, + controllerpkg.IssuerOptions{ClusterResourceNamespace: "test-namespace"}, + )(test.secret) + require.Equal(t, len(test.expectedQueue), queue.Len()) + var actualQueue []string + for range test.expectedQueue { + i, _ := queue.Get() + actualQueue = append(actualQueue, i.(string)) + } + assert.ElementsMatch(t, test.expectedQueue, actualQueue) + }) + } +} + +func Test_certificatesRequestsForSecret(t *testing.T) { + secret := gen.Secret("test-secret", gen.SetSecretNamespace("test-namespace")) + tests := map[string]struct { + existingCSRs []runtime.Object + existingIssuers []runtime.Object + clusterResourceNamespace string + expectedAffected []*certificatesv1.CertificateSigningRequest + }{ + "if no existing requests or issuers, then expect none returned": { + existingCSRs: []runtime.Object{}, + existingIssuers: []runtime.Object{}, + clusterResourceNamespace: "test-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{}, + }, + "if no existing requests, then expect none returned": { + existingCSRs: []runtime.Object{}, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + clusterResourceNamespace: "test-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{}, + }, + "if no existing issuers, then expect none returned": { + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{}, + clusterResourceNamespace: "test-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{}, + }, + "if issuers are not self signed then don't return requests": { + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerCA(cmapi.CAIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerCA(cmapi.CAIssuer{}), + ), + }, + clusterResourceNamespace: "test-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{}, + }, + "should not return requests which are in a different namespace": { + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/another-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + clusterResourceNamespace: "another-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{}, + }, + "should sync only requests which match issuers that match namespace of the Secret, ignore secret when cluster resource namespace is different for ClusterIssuers": { + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + clusterResourceNamespace: "NOT_test-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + }, + }, + "should return requests that reference a selfsigned issuer and the secret with the private key": { + existingCSRs: []runtime.Object{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + existingIssuers: []runtime.Object{ + gen.Issuer("a", + gen.SetIssuerNamespace("test-namespace"), + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + gen.ClusterIssuer("b", + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + clusterResourceNamespace: "test-namespace", + expectedAffected: []*certificatesv1.CertificateSigningRequest{ + gen.CertificateSigningRequest("a", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("issuers.cert-manager.io/test-namespace.a"), + ), + gen.CertificateSigningRequest("b", + gen.AddCertificateSigningRequestAnnotations(map[string]string{ + "experimental.cert-manager.io/private-key-secret-name": "test-secret", + }), + gen.SetCertificateSigningRequestSignerName("clusterissuers.cert-manager.io/b"), + ), + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + builder := &testpkg.Builder{ + T: t, + KubeObjects: test.existingCSRs, + CertManagerObjects: test.existingIssuers, + } + defer builder.Stop() + builder.Init() + + lister := builder.Context.KubeSharedInformerFactory.Certificates().V1().CertificateSigningRequests().Lister() + helper := issuer.NewHelper( + builder.Context.SharedInformerFactory.Certmanager().V1().Issuers().Lister(), + builder.Context.SharedInformerFactory.Certmanager().V1().ClusterIssuers().Lister(), + ) + + builder.Start() + + affected, err := certificateSigningRequestsForSecret(klogr.New(), lister, helper, secret.DeepCopy(), controllerpkg.IssuerOptions{ + ClusterResourceNamespace: test.clusterResourceNamespace, + }) + + assert.NoError(t, err) + assert.ElementsMatch(t, test.expectedAffected, affected) + }) + } +} From 7b168cc0592094a9e9bcf921d24743af5b0cbb9a Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 9 Aug 2022 11:16:10 +0100 Subject: [PATCH 4/5] Updates selfsigned CertificateSigningRequest controller with new Secret informer, and no longer mark the request as Failed when the private key Secret is malformed. This behaviour matches the CertificateRequest self signed controller. Signed-off-by: joshvanl --- .../selfsigned/selfsigned.go | 33 ++++++++++++++++--- .../selfsigned/selfsigned_test.go | 26 ++------------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/pkg/controller/certificatesigningrequests/selfsigned/selfsigned.go b/pkg/controller/certificatesigningrequests/selfsigned/selfsigned.go index 9e030309c..c3b577e84 100644 --- a/pkg/controller/certificatesigningrequests/selfsigned/selfsigned.go +++ b/pkg/controller/certificatesigningrequests/selfsigned/selfsigned.go @@ -28,7 +28,9 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1" corelisters "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/workqueue" apiutil "github.com/cert-manager/cert-manager/pkg/api/util" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" @@ -36,10 +38,12 @@ import ( controllerpkg "github.com/cert-manager/cert-manager/pkg/controller" "github.com/cert-manager/cert-manager/pkg/controller/certificatesigningrequests" "github.com/cert-manager/cert-manager/pkg/controller/certificatesigningrequests/util" + "github.com/cert-manager/cert-manager/pkg/issuer" logf "github.com/cert-manager/cert-manager/pkg/logs" cmerrors "github.com/cert-manager/cert-manager/pkg/util/errors" "github.com/cert-manager/cert-manager/pkg/util/kube" "github.com/cert-manager/cert-manager/pkg/util/pki" + "github.com/go-logr/logr" ) const ( @@ -70,7 +74,28 @@ func init() { // create certificate signing request controller for selfsigned issuer controllerpkg.Register(CSRControllerName, func(ctx *controllerpkg.ContextFactory) (controllerpkg.Interface, error) { return controllerpkg.NewBuilder(ctx, CSRControllerName). - For(certificatesigningrequests.New(apiutil.IssuerSelfSigned, NewSelfSigned)). + For(certificatesigningrequests.New( + apiutil.IssuerSelfSigned, NewSelfSigned, + + // Handle informed Secrets which may be referenced by the + // "experimental.cert-manager.io/private-key-secret-name" annotation. + func(ctx *controllerpkg.Context, log logr.Logger, queue workqueue.RateLimitingInterface) ([]cache.InformerSynced, error) { + secretInformer := ctx.KubeSharedInformerFactory.Core().V1().Secrets().Informer() + certificateSigningRequestLister := ctx.KubeSharedInformerFactory.Certificates().V1().CertificateSigningRequests().Lister() + helper := issuer.NewHelper( + ctx.SharedInformerFactory.Certmanager().V1().Issuers().Lister(), + ctx.SharedInformerFactory.Certmanager().V1().ClusterIssuers().Lister(), + ) + secretInformer.AddEventHandler(&controllerpkg.BlockingEventHandler{ + WorkFunc: handleSecretReferenceWorkFunc(log, certificateSigningRequestLister, helper, queue, ctx.IssuerOptions), + }) + return []cache.InformerSynced{ + secretInformer.HasSynced, + ctx.SharedInformerFactory.Certmanager().V1().Issuers().Informer().HasSynced, + ctx.SharedInformerFactory.Certmanager().V1().ClusterIssuers().Informer().HasSynced, + }, nil + }, + )). Complete() }) } @@ -115,16 +140,14 @@ func (s *SelfSigned) Sign(ctx context.Context, csr *certificatesv1.CertificateSi message := fmt.Sprintf("Referenced Secret %s/%s not found", resourceNamespace, secretName) log.Error(err, message) s.recorder.Event(csr, corev1.EventTypeWarning, "SecretNotFound", message) - return err + return nil } if cmerrors.IsInvalidData(err) { message := fmt.Sprintf("Failed to parse signing key from secret %s/%s", resourceNamespace, secretName) log.Error(err, message) s.recorder.Eventf(csr, corev1.EventTypeWarning, "ErrorParsingKey", "%s: %s", message, err) - util.CertificateSigningRequestSetFailed(csr, "ErrorParsingKey", message) - _, err = util.UpdateOrApplyStatus(ctx, s.certClient, csr, certificatesv1.CertificateFailed, s.fieldManager) - return err + return nil } if err != nil { diff --git a/pkg/controller/certificatesigningrequests/selfsigned/selfsigned_test.go b/pkg/controller/certificatesigningrequests/selfsigned/selfsigned_test.go index 193943b6a..e33d2692c 100644 --- a/pkg/controller/certificatesigningrequests/selfsigned/selfsigned_test.go +++ b/pkg/controller/certificatesigningrequests/selfsigned/selfsigned_test.go @@ -260,9 +260,9 @@ func TestProcessItem(t *testing.T) { )), }, }, - expectedErr: true, + expectedErr: false, }, - "an approved CSR but the private key references a Secret that contains bad data should be marked as failed": { + "an approved CSR but the private key references a Secret that contains bad data should fire warning event.": { csr: gen.CertificateSigningRequestFrom(baseCSR, gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ Type: certificatesv1.CertificateApproved, @@ -313,28 +313,6 @@ func TestProcessItem(t *testing.T) { }, }, )), - testpkg.NewAction(coretesting.NewUpdateSubresourceAction( - certificatesv1.SchemeGroupVersion.WithResource("certificatesigningrequests"), - "status", - "", - gen.CertificateSigningRequestFrom(baseCSR.DeepCopy(), - gen.AddCertificateSigningRequestAnnotations(map[string]string{ - "experimental.cert-manager.io/private-key-secret-name": "test-secret", - }), - gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ - Type: certificatesv1.CertificateApproved, - Status: corev1.ConditionTrue, - }), - gen.SetCertificateSigningRequestStatusCondition(certificatesv1.CertificateSigningRequestCondition{ - Type: certificatesv1.CertificateFailed, - Status: corev1.ConditionTrue, - Reason: "ErrorParsingKey", - Message: "Failed to parse signing key from secret default-unit-test-ns/test-secret", - LastTransitionTime: metaFixedClockStart, - LastUpdateTime: metaFixedClockStart, - }), - ), - )), }, }, }, From 52787eabd2b81d5d5cebb839e29cec9c74e73cf3 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 9 Aug 2022 11:17:44 +0100 Subject: [PATCH 5/5] Adds e2e tests for the new SelfSigned CertificateSigningRequest Secret informer Signed-off-by: joshvanl --- .../suite/certificatesigningrequests/doc.go | 21 ++ .../selfsigned/selfsigned.go | 306 ++++++++++++++++++ test/e2e/suite/doc.go | 1 + test/unit/crypto/crypto.go | 4 +- 4 files changed, 330 insertions(+), 2 deletions(-) create mode 100644 test/e2e/suite/certificatesigningrequests/doc.go create mode 100644 test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go diff --git a/test/e2e/suite/certificatesigningrequests/doc.go b/test/e2e/suite/certificatesigningrequests/doc.go new file mode 100644 index 000000000..3ffccb865 --- /dev/null +++ b/test/e2e/suite/certificatesigningrequests/doc.go @@ -0,0 +1,21 @@ +/* +Copyright 2022 The cert-manager 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 certificatesigningrequests + +import ( + _ "github.com/cert-manager/cert-manager/test/e2e/suite/certificatesigningrequests/selfsigned" +) diff --git a/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go b/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go new file mode 100644 index 000000000..18e8e6c7d --- /dev/null +++ b/test/e2e/suite/certificatesigningrequests/selfsigned/selfsigned.go @@ -0,0 +1,306 @@ +/* +Copyright 2022 The cert-manager 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 selfsigned + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + certificatesv1 "k8s.io/api/certificates/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/clock" + + cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/cert-manager/cert-manager/test/e2e/framework" + testcrypto "github.com/cert-manager/cert-manager/test/unit/crypto" +) + +// This test ensures that a self-signed certificatesigningrequests will still +// be signed, even if the private key Secret was created _after_ the +// CertificateSigningRequest was created. +var _ = framework.CertManagerDescribe("CertificateSigningRequests SelfSigned Secret", func() { + f := framework.NewDefaultFramework("certificatesigningrequests-selfsigned-secret") + + var ( + request *certificatesv1.CertificateSigningRequest + issuer cmapi.GenericIssuer + secret *corev1.Secret + bundle *testcrypto.CryptoBundle + ) + + JustBeforeEach(func() { + var err error + bundle, err = testcrypto.CreateCryptoBundle(&cmapi.Certificate{ + Spec: cmapi.CertificateSpec{CommonName: "selfsigned-test"}}, clock.RealClock{}) + Expect(err).NotTo(HaveOccurred()) + }) + + JustAfterEach(func() { + Expect(f.CRClient.Delete(context.TODO(), request)).NotTo(HaveOccurred()) + Expect(f.CRClient.Delete(context.TODO(), issuer)).NotTo(HaveOccurred()) + Expect(f.CRClient.Delete(context.TODO(), secret)).NotTo(HaveOccurred()) + }) + + It("Issuer: the private key Secret is created after the request is created should still be signed", func() { + var err error + issuer, err = f.CertManagerClientSet.CertmanagerV1().Issuers(f.Namespace.Name).Create(context.TODO(), &cmapi.Issuer{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "selfsigned-", Namespace: f.Namespace.Name}, + Spec: cmapi.IssuerSpec{IssuerConfig: cmapi.IssuerConfig{SelfSigned: new(cmapi.SelfSignedIssuer)}}, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("creating request") + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), &certificatesv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "selfsigned-", + Annotations: map[string]string{"experimental.cert-manager.io/private-key-secret-name": "selfsigned-test"}, + }, + Spec: certificatesv1.CertificateSigningRequestSpec{ + Request: bundle.CSRBytes, + SignerName: fmt.Sprintf("issuers.cert-manager.io/%s.%s", f.Namespace.Name, issuer.GetName()), + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("approving request") + request.Status.Conditions = append(request.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, Status: corev1.ConditionTrue, + Reason: "Approved", Message: "approved for cert-manager.io selfigned e2e test", + LastUpdateTime: metav1.NewTime(time.Now()), LastTransitionTime: metav1.NewTime(time.Now()), + }) + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().UpdateApproval(context.TODO(), request.Name, request, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for request to have SecretNotFound event") + Eventually(func() bool { + events, err := f.KubeClientSet.EventsV1().Events("default").List(context.TODO(), metav1.ListOptions{ + FieldSelector: "reason=SecretNotFound,type=Warning", + }) + Expect(err).NotTo(HaveOccurred()) + for _, event := range events.Items { + if event.Regarding.UID == request.UID { + return true + } + } + return false + }, "20s", "1s").Should(BeTrue(), "SecretNotFound event not found for request") + + By("creating Secret with private key should result in the request to be signed") + secret, err = f.KubeClientSet.CoreV1().Secrets(f.Namespace.Name).Create(context.TODO(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "selfsigned-test", Namespace: f.Namespace.Name}, + Data: map[string][]byte{ + "tls.key": bundle.PrivateKeyBytes, + }, + }, metav1.CreateOptions{}) + + Eventually(func() bool { + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), request.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + return len(request.Status.Certificate) > 0 + }, "20s", "1s").Should(BeTrue(), "request was not signed in time: %#+v", request) + }) + + It("Issuer: private key Secret is updated with a valid private key after the request is created should still be signed", func() { + var err error + By("creating Secret with missing private key") + secret, err = f.KubeClientSet.CoreV1().Secrets(f.Namespace.Name).Create(context.TODO(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "selfsigned-test", Namespace: f.Namespace.Name}, + Data: map[string][]byte{}, + }, metav1.CreateOptions{}) + + issuer, err = f.CertManagerClientSet.CertmanagerV1().Issuers(f.Namespace.Name).Create(context.TODO(), &cmapi.Issuer{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "selfsigned-", Namespace: f.Namespace.Name}, + Spec: cmapi.IssuerSpec{IssuerConfig: cmapi.IssuerConfig{SelfSigned: new(cmapi.SelfSignedIssuer)}}, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("creating request") + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), &certificatesv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "selfsigned-", + Annotations: map[string]string{"experimental.cert-manager.io/private-key-secret-name": "selfsigned-test"}, + }, + Spec: certificatesv1.CertificateSigningRequestSpec{ + Request: bundle.CSRBytes, + SignerName: fmt.Sprintf("issuers.cert-manager.io/%s.%s", f.Namespace.Name, issuer.GetName()), + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("approving request") + request.Status.Conditions = append(request.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, Status: corev1.ConditionTrue, + Reason: "Approved", Message: "approved for cert-manager.io selfigned e2e test", + LastUpdateTime: metav1.NewTime(time.Now()), LastTransitionTime: metav1.NewTime(time.Now()), + }) + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().UpdateApproval(context.TODO(), request.Name, request, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for request to have ErrorParsingKey event") + Eventually(func() bool { + events, err := f.KubeClientSet.EventsV1().Events("default").List(context.TODO(), metav1.ListOptions{ + FieldSelector: "reason=ErrorParsingKey,type=Warning", + }) + Expect(err).NotTo(HaveOccurred()) + for _, event := range events.Items { + if event.Regarding.UID == request.UID { + return true + } + } + return false + }, "20s", "1s").Should(BeTrue(), "ErrorParsingKey event not found for request") + + By("updating referenced private key Secret should get the request signed") + secret.Data = map[string][]byte{"tls.key": bundle.PrivateKeyBytes} + _, err = f.KubeClientSet.CoreV1().Secrets(f.Namespace.Name).Update(context.TODO(), secret, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + Eventually(func() bool { + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), request.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + return len(request.Status.Certificate) > 0 + }, "20s", "1s").Should(BeTrue(), "request was not signed in time: %#+v", request) + }) + + It("ClusterIssuer: the private key Secret is created after the request is created should still be signed", func() { + var err error + issuer, err = f.CertManagerClientSet.CertmanagerV1().ClusterIssuers().Create(context.TODO(), &cmapi.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "selfsigned-"}, + Spec: cmapi.IssuerSpec{IssuerConfig: cmapi.IssuerConfig{SelfSigned: new(cmapi.SelfSignedIssuer)}}, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("creating request") + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), &certificatesv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "selfsigned-", + Annotations: map[string]string{"experimental.cert-manager.io/private-key-secret-name": "selfsigned-test"}, + }, + Spec: certificatesv1.CertificateSigningRequestSpec{ + Request: bundle.CSRBytes, + SignerName: fmt.Sprintf("clusterissuers.cert-manager.io/" + issuer.GetName()), + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("approving request") + request.Status.Conditions = append(request.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, Status: corev1.ConditionTrue, + Reason: "Approved", Message: "approved for cert-manager.io selfigned e2e test", + LastUpdateTime: metav1.NewTime(time.Now()), LastTransitionTime: metav1.NewTime(time.Now()), + }) + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().UpdateApproval(context.TODO(), request.Name, request, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for request to have SecretNotFound event") + Eventually(func() bool { + events, err := f.KubeClientSet.EventsV1().Events("default").List(context.TODO(), metav1.ListOptions{ + FieldSelector: "reason=SecretNotFound,type=Warning", + }) + Expect(err).NotTo(HaveOccurred()) + for _, event := range events.Items { + if event.Regarding.UID == request.UID { + return true + } + } + return false + }, "20s", "1s").Should(BeTrue(), "SecretNotFound event not found for request") + + By("creating Secret with private key should result in the request to be signed") + secret, err = f.KubeClientSet.CoreV1().Secrets("cert-manager").Create(context.TODO(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "selfsigned-test", Namespace: "cert-manager"}, + Data: map[string][]byte{ + "tls.key": bundle.PrivateKeyBytes, + }, + }, metav1.CreateOptions{}) + + Eventually(func() bool { + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), request.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + return len(request.Status.Certificate) > 0 + }, "20s", "1s").Should(BeTrue(), "request was not signed in time: %#+v", request) + }) + + It("ClusterIssuer: private key Secret is updated with a valid private key after the request is created should still be signed", func() { + var err error + By("creating Secret with missing private key") + secret, err = f.KubeClientSet.CoreV1().Secrets("cert-manager").Create(context.TODO(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "selfsigned-test", Namespace: "cert-manager"}, + Data: map[string][]byte{}, + }, metav1.CreateOptions{}) + + issuer, err = f.CertManagerClientSet.CertmanagerV1().ClusterIssuers().Create(context.TODO(), &cmapi.ClusterIssuer{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "selfsigned-"}, + Spec: cmapi.IssuerSpec{IssuerConfig: cmapi.IssuerConfig{SelfSigned: new(cmapi.SelfSignedIssuer)}}, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("creating request") + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), &certificatesv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "selfsigned-", + Annotations: map[string]string{"experimental.cert-manager.io/private-key-secret-name": "selfsigned-test"}, + }, + Spec: certificatesv1.CertificateSigningRequestSpec{ + Request: bundle.CSRBytes, + SignerName: fmt.Sprintf("clusterissuers.cert-manager.io/" + issuer.GetName()), + Usages: []certificatesv1.KeyUsage{certificatesv1.UsageClientAuth, certificatesv1.UsageServerAuth}, + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("approving request") + request.Status.Conditions = append(request.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{ + Type: certificatesv1.CertificateApproved, Status: corev1.ConditionTrue, + Reason: "Approved", Message: "approved for cert-manager.io selfigned e2e test", + LastUpdateTime: metav1.NewTime(time.Now()), LastTransitionTime: metav1.NewTime(time.Now()), + }) + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().UpdateApproval(context.TODO(), request.Name, request, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for request to have ErrorParsingKey event") + Eventually(func() bool { + events, err := f.KubeClientSet.EventsV1().Events("default").List(context.TODO(), metav1.ListOptions{ + FieldSelector: "reason=ErrorParsingKey,type=Warning", + }) + Expect(err).NotTo(HaveOccurred()) + for _, event := range events.Items { + if event.Regarding.UID == request.UID { + return true + } + } + return false + }, "20s", "1s").Should(BeTrue(), "ErrorParsingKey event not found for request") + + By("updating referenced private key Secret should get the request signed") + secret.Data = map[string][]byte{"tls.key": bundle.PrivateKeyBytes} + _, err = f.KubeClientSet.CoreV1().Secrets("cert-manager").Update(context.TODO(), secret, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + Eventually(func() bool { + request, err = f.KubeClientSet.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), request.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + return len(request.Status.Certificate) > 0 + }, "20s", "1s").Should(BeTrue(), "request was not signed in time: %#+v", request) + }) + +}) diff --git a/test/e2e/suite/doc.go b/test/e2e/suite/doc.go index cb6ec22db..a63b9a23e 100644 --- a/test/e2e/suite/doc.go +++ b/test/e2e/suite/doc.go @@ -19,6 +19,7 @@ package suite import ( _ "github.com/cert-manager/cert-manager/test/e2e/suite/approval" _ "github.com/cert-manager/cert-manager/test/e2e/suite/certificates" + _ "github.com/cert-manager/cert-manager/test/e2e/suite/certificatesigningrequests" _ "github.com/cert-manager/cert-manager/test/e2e/suite/conformance" _ "github.com/cert-manager/cert-manager/test/e2e/suite/issuers" _ "github.com/cert-manager/cert-manager/test/e2e/suite/serving" diff --git a/test/unit/crypto/crypto.go b/test/unit/crypto/crypto.go index 952b15a3a..be86d46c9 100644 --- a/test/unit/crypto/crypto.go +++ b/test/unit/crypto/crypto.go @@ -74,14 +74,14 @@ type CryptoBundle struct { // MustCreateCryptoBundle creates a CryptoBundle to be used with tests or fails. func MustCreateCryptoBundle(t *testing.T, crt *cmapi.Certificate, clock clock.Clock) CryptoBundle { - c, err := createCryptoBundle(crt, clock) + c, err := CreateCryptoBundle(crt, clock) if err != nil { t.Fatalf("error generating crypto bundle: %v", err) } return *c } -func createCryptoBundle(originalCert *cmapi.Certificate, clock clock.Clock) (*CryptoBundle, error) { +func CreateCryptoBundle(originalCert *cmapi.Certificate, clock clock.Clock) (*CryptoBundle, error) { crt := originalCert.DeepCopy() if crt.Spec.PrivateKey == nil { crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{}