From 8be302dc1006d36deeef73d07bfc0a85ef47a7c4 Mon Sep 17 00:00:00 2001 From: JoshVanL Date: Thu, 1 Aug 2019 13:33:43 +0100 Subject: [PATCH] Moves fake lister into /test and removes logging from reporter Signed-off-by: JoshVanL --- BUILD.bazel | 1 + .../certificaterequests/ca/BUILD.bazel | 2 +- pkg/controller/certificaterequests/ca/ca.go | 28 ++++++++++++------- .../certificaterequests/ca/ca_test.go | 18 ++++++------ .../certificaterequests/util/BUILD.bazel | 3 +- .../util/{status.go => reporter.go} | 12 +------- pkg/controller/test/BUILD.bazel | 5 +--- .../fake => test/unit/listers}/BUILD.bazel | 4 +-- .../unit/listers/secret.go | 2 +- 9 files changed, 35 insertions(+), 40 deletions(-) rename pkg/controller/certificaterequests/util/{status.go => reporter.go} (83%) rename {pkg/controller/test/fake => test/unit/listers}/BUILD.bazel (85%) rename pkg/controller/test/fake/secretlister.go => test/unit/listers/secret.go (99%) diff --git a/BUILD.bazel b/BUILD.bazel index 4e0de38a6..6cc3d9324 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -60,6 +60,7 @@ filegroup( "//test/acme/dns:all-srcs", "//test/e2e:all-srcs", "//test/unit/gen:all-srcs", + "//test/unit/listers:all-srcs", "//test/util:all-srcs", "//third_party:all-srcs", "//vendor:all-srcs", diff --git a/pkg/controller/certificaterequests/ca/BUILD.bazel b/pkg/controller/certificaterequests/ca/BUILD.bazel index 107a1c8f4..d28efd349 100644 --- a/pkg/controller/certificaterequests/ca/BUILD.bazel +++ b/pkg/controller/certificaterequests/ca/BUILD.bazel @@ -44,10 +44,10 @@ go_test( "//pkg/apis/certmanager:go_default_library", "//pkg/apis/certmanager/v1alpha1:go_default_library", "//pkg/controller/test:go_default_library", - "//pkg/controller/test/fake:go_default_library", "//pkg/issuer:go_default_library", "//pkg/util/pki:go_default_library", "//test/unit/gen:go_default_library", + "//test/unit/listers:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/controller/certificaterequests/ca/ca.go b/pkg/controller/certificaterequests/ca/ca.go index 6adc63840..3bc922ae3 100644 --- a/pkg/controller/certificaterequests/ca/ca.go +++ b/pkg/controller/certificaterequests/ca/ca.go @@ -79,7 +79,7 @@ func NewCA(ctx *controllerpkg.Context) *CA { func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerObj v1alpha1.GenericIssuer) (*issuerpkg.IssueResponse, error) { log := logf.FromContext(ctx, "sign") - reporter := crutil.NewReporter(log, cr, c.recorder) + reporter := crutil.NewReporter(cr, c.recorder) secretName := issuerObj.GetSpec().CA.SecretName resourceNamespace := c.issuerOptions.ResourceNamespace(issuerObj) @@ -88,36 +88,44 @@ func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerOb caCerts, caKey, err := kube.SecretTLSKeyPair(ctx, c.secretsLister, resourceNamespace, issuerObj.GetSpec().CA.SecretName) if err != nil { log := logf.WithRelatedResourceName(log, issuerObj.GetSpec().CA.SecretName, resourceNamespace, "Secret") - reporter = reporter.WithLog(log) if k8sErrors.IsNotFound(err) { - reporter.Pending(err, "MissingSecret", - fmt.Sprintf("Referenced secret %s/%s not found", resourceNamespace, secretName)) + message := fmt.Sprintf("Referenced secret %s/%s not found", resourceNamespace, secretName) + + reporter.Pending(err, "MissingSecret", message) + log.Error(err, message) return nil, nil } if cmerrors.IsInvalidData(err) { - reporter.Pending(err, "ErrorParsingSecret", - fmt.Sprintf("Failed to parse key cert pair from secret %s/%s", resourceNamespace, secretName)) + message := fmt.Sprintf("Failed to parse signing CA keypair from secret %s/%s", resourceNamespace, secretName) + + reporter.Pending(err, "ErrorParsingSecret", message) + log.Error(err, message) return nil, nil } // We are probably in a network error here so we should backoff and retry - reporter.Pending(err, "ErrorGettingSecret", - fmt.Sprintf("Failed to get key cert pair from secret %s/%s", resourceNamespace, secretName)) + message := fmt.Sprintf("Failed to get certificate key pair from secret %s/%s", resourceNamespace, secretName) + reporter.Pending(err, "ErrorGettingSecret", message) + log.Error(err, message) return nil, err } template, err := pki.GenerateTemplateFromCertificateRequest(cr) if err != nil { - reporter.Failed(err, "ErrorSigning", "Error generating certificate template") + message := "Error generating certificate template" + reporter.Failed(err, "ErrorSigning", message) + log.Error(err, message) return nil, nil } certPEM, caPEM, err := pki.SignCSRTemplate(caCerts, caKey, template) if err != nil { - reporter.Failed(err, "ErrorSigning", "Error signing certificate") + message := "Error signing certificate" + reporter.Failed(err, "ErrorSigning", message) + log.Error(err, message) return nil, err } diff --git a/pkg/controller/certificaterequests/ca/ca_test.go b/pkg/controller/certificaterequests/ca/ca_test.go index d85edd159..f0a7ac2a6 100644 --- a/pkg/controller/certificaterequests/ca/ca_test.go +++ b/pkg/controller/certificaterequests/ca/ca_test.go @@ -39,10 +39,10 @@ import ( "github.com/jetstack/cert-manager/pkg/apis/certmanager" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" testpkg "github.com/jetstack/cert-manager/pkg/controller/test" - testfake "github.com/jetstack/cert-manager/pkg/controller/test/fake" "github.com/jetstack/cert-manager/pkg/issuer" "github.com/jetstack/cert-manager/pkg/util/pki" "github.com/jetstack/cert-manager/test/unit/gen" + testlisters "github.com/jetstack/cert-manager/test/unit/listers" ) func generateRSAPrivateKey(t *testing.T) *rsa.PrivateKey { @@ -247,7 +247,7 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{rootRSANoCASecret}, CertManagerObjects: []runtime.Object{}, ExpectedEvents: []string{ - `Normal ErrorParsingSecret Failed to parse key cert pair from secret default-unit-test-ns/root-ca-secret: error decoding cert PEM block`, + `Normal ErrorParsingSecret Failed to parse signing CA keypair from secret default-unit-test-ns/root-ca-secret: error decoding cert PEM block`, }, CheckFn: mustNoResponse, }, @@ -268,7 +268,7 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{rootRSANoKeySecret}, CertManagerObjects: []runtime.Object{}, ExpectedEvents: []string{ - `Normal ErrorParsingSecret Failed to parse key cert pair from secret default-unit-test-ns/root-ca-secret: error decoding private key PEM block`, + `Normal ErrorParsingSecret Failed to parse signing CA keypair from secret default-unit-test-ns/root-ca-secret: error decoding private key PEM block`, }, CheckFn: mustNoResponse, }, @@ -290,12 +290,12 @@ func TestSign(t *testing.T) { CertManagerObjects: []runtime.Object{}, CheckFn: mustNoResponse, ExpectedEvents: []string{ - "Normal ErrorGettingSecret Failed to get key cert pair from secret default-unit-test-ns/root-ca-secret: this is a network error", + `Normal ErrorGettingSecret Failed to get certificate key pair from secret default-unit-test-ns/root-ca-secret: this is a network error`, }, }, - FakeLister: &testfake.FakeSecretLister{ + fakeLister: &testlisters.FakeSecretLister{ SecretsFn: func(namespace string) clientcorev1.SecretNamespaceLister { - return &testfake.FakeSecretNamespaceLister{ + return &testlisters.FakeSecretNamespaceLister{ GetFn: func(name string) (ret *corev1.Secret, err error) { return nil, errors.New("this is a network error") }, @@ -320,7 +320,7 @@ type testT struct { expectedErr bool - FakeLister *testfake.FakeSecretLister + fakeLister *testlisters.FakeSecretLister } func runTest(t *testing.T, test testT) { @@ -330,8 +330,8 @@ func runTest(t *testing.T, test testT) { c := NewCA(test.builder.Context) - if test.FakeLister != nil { - c.secretsLister = test.FakeLister + if test.fakeLister != nil { + c.secretsLister = test.fakeLister } test.builder.Sync() diff --git a/pkg/controller/certificaterequests/util/BUILD.bazel b/pkg/controller/certificaterequests/util/BUILD.bazel index c0775ba8a..f6bdb0ee2 100644 --- a/pkg/controller/certificaterequests/util/BUILD.bazel +++ b/pkg/controller/certificaterequests/util/BUILD.bazel @@ -2,13 +2,12 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = ["status.go"], + srcs = ["reporter.go"], importpath = "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/util", visibility = ["//visibility:public"], deps = [ "//pkg/api/util:go_default_library", "//pkg/apis/certmanager/v1alpha1:go_default_library", - "//vendor/github.com/go-logr/logr:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", ], diff --git a/pkg/controller/certificaterequests/util/status.go b/pkg/controller/certificaterequests/util/reporter.go similarity index 83% rename from pkg/controller/certificaterequests/util/status.go rename to pkg/controller/certificaterequests/util/reporter.go index 7e8754a22..211634e6f 100644 --- a/pkg/controller/certificaterequests/util/status.go +++ b/pkg/controller/certificaterequests/util/reporter.go @@ -19,7 +19,6 @@ package util import ( "fmt" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" @@ -28,32 +27,23 @@ import ( ) type Reporter struct { - log logr.Logger cr *v1alpha1.CertificateRequest recorder record.EventRecorder } -func NewReporter(log logr.Logger, cr *v1alpha1.CertificateRequest, recorder record.EventRecorder) *Reporter { +func NewReporter(cr *v1alpha1.CertificateRequest, recorder record.EventRecorder) *Reporter { return &Reporter{ - log: log, cr: cr, recorder: recorder, } } -func (r *Reporter) WithLog(log logr.Logger) *Reporter { - r.log = log - return r -} - func (r *Reporter) Failed(err error, reason, message string) { r.recorder.Event(r.cr, corev1.EventTypeWarning, reason, fmt.Sprintf("%s: %v", message, err)) - r.log.Error(err, message) apiutil.SetCertificateRequestCondition(r.cr, v1alpha1.CertificateRequestReasonFailed, v1alpha1.ConditionFalse, reason, message) } func (r *Reporter) Pending(err error, reason, message string) { r.recorder.Event(r.cr, corev1.EventTypeNormal, reason, fmt.Sprintf("%s: %v", message, err)) - r.log.Error(err, message) apiutil.SetCertificateRequestCondition(r.cr, v1alpha1.CertificateRequestReasonPending, v1alpha1.ConditionFalse, reason, message) } diff --git a/pkg/controller/test/BUILD.bazel b/pkg/controller/test/BUILD.bazel index 99618eaa5..0ff8b28b0 100644 --- a/pkg/controller/test/BUILD.bazel +++ b/pkg/controller/test/BUILD.bazel @@ -40,10 +40,7 @@ filegroup( filegroup( name = "all-srcs", - srcs = [ - ":package-srcs", - "//pkg/controller/test/fake:all-srcs", - ], + srcs = [":package-srcs"], tags = ["automanaged"], visibility = ["//visibility:public"], ) diff --git a/pkg/controller/test/fake/BUILD.bazel b/test/unit/listers/BUILD.bazel similarity index 85% rename from pkg/controller/test/fake/BUILD.bazel rename to test/unit/listers/BUILD.bazel index 7a47cbabc..488b63503 100644 --- a/pkg/controller/test/fake/BUILD.bazel +++ b/test/unit/listers/BUILD.bazel @@ -2,8 +2,8 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = ["secretlister.go"], - importpath = "github.com/jetstack/cert-manager/pkg/controller/test/fake", + srcs = ["secret.go"], + importpath = "github.com/jetstack/cert-manager/test/unit/listers", visibility = ["//visibility:public"], deps = [ "//vendor/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/controller/test/fake/secretlister.go b/test/unit/listers/secret.go similarity index 99% rename from pkg/controller/test/fake/secretlister.go rename to test/unit/listers/secret.go index d8a6c41a0..069d7a449 100644 --- a/pkg/controller/test/fake/secretlister.go +++ b/test/unit/listers/secret.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package fake +package lister import ( corev1 "k8s.io/api/core/v1"