Skip to content

Commit

Permalink
Fixup error text and cleanup test instantiation
Browse files Browse the repository at this point in the history
Signed-off-by: James Munnelly <james@munnelly.eu>
  • Loading branch information
munnerz committed Apr 15, 2020
1 parent 38716d6 commit 572e467
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 27 deletions.
9 changes: 3 additions & 6 deletions pkg/controller/expcertificates/issuing/issuing_controller.go
Expand Up @@ -165,8 +165,6 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
return nil
}

nextPrivateKeySecretName := *crt.Status.NextPrivateKeySecretName

// CertificateRequest revisions begin from 1. If no revision is set on the
// status then assume no revision yet set.
nextRevision := 1
Expand Down Expand Up @@ -200,7 +198,7 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
// If the CertificateRequest is valid, verify its status and update
// accordingly.
case cmapi.CertificateRequestReasonIssued:
return c.issueCertificate(ctx, log, nextPrivateKeySecretName, nextRevision, crt, req)
return c.issueCertificate(ctx, log, nextRevision, crt, req)

// CertificateRequest is not in a final state so do nothing.
default:
Expand All @@ -220,7 +218,7 @@ func (c *controller) failIssueCertificate(ctx context.Context, log logr.Logger,
condition := apiutil.GetCertificateRequestCondition(req, cmapi.CertificateRequestConditionReady)

reason = condition.Reason
message = fmt.Sprintf("The certificate has failed to complete and will be retried: %s",
message = fmt.Sprintf("The certificate request has failed to complete and will be retried: %s",
condition.Message)

crt = crt.DeepCopy()
Expand All @@ -239,8 +237,7 @@ func (c *controller) failIssueCertificate(ctx context.Context, log logr.Logger,
// issueCertificate will ensure the public key of the CSR matches the signed
// certificate, and then store the certificate, CA and private key into the
// Secret in the appropriate format type.
func (c *controller) issueCertificate(ctx context.Context, log logr.Logger, nextPrivateKeySecretName string, nextRevision int,
crt *cmapi.Certificate, req *cmapi.CertificateRequest) error {
func (c *controller) issueCertificate(ctx context.Context, log logr.Logger, nextRevision int, crt *cmapi.Certificate, req *cmapi.CertificateRequest) error {

csr, err := utilpki.DecodeX509CertificateRequestBytes(req.Spec.CSRPEM)
if err != nil {
Expand Down
28 changes: 7 additions & 21 deletions pkg/controller/expcertificates/issuing/issuing_controller_test.go
Expand Up @@ -31,7 +31,6 @@ import (

cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2"
cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
controllerpkg "github.com/jetstack/cert-manager/pkg/controller"
testpkg "github.com/jetstack/cert-manager/pkg/controller/test"
"github.com/jetstack/cert-manager/test/unit/gen"
)
Expand Down Expand Up @@ -200,15 +199,15 @@ func TestIssuingController(t *testing.T) {
Type: cmapi.CertificateConditionIssuing,
Status: cmmeta.ConditionFalse,
Reason: "Failed",
Message: "The certificate has failed to complete and will be retried: The certificate request failed because of reasons",
Message: "The certificate request has failed to complete and will be retried: The certificate request failed because of reasons",
LastTransitionTime: &metaFixedClockStart,
}),
gen.SetCertificateLastFailureTime(metaFixedClockStart),
),
)),
},
ExpectedEvents: []string{
"Warning Failed The certificate has failed to complete and will be retried: The certificate request failed because of reasons",
"Warning Failed The certificate request has failed to complete and will be retried: The certificate request failed because of reasons",
},
},
expectedErr: false,
Expand Down Expand Up @@ -456,24 +455,11 @@ func TestIssuingController(t *testing.T) {
test.builder.Init()
defer test.builder.Stop()

secretsLister := test.builder.KubeSharedInformerFactory.Core().V1().Secrets().Lister()

secretManager := newSecretsManager(
test.builder.Client,
secretsLister,
controllerpkg.CertificateOptions{},
)

testManager := &controller{
certificateLister: test.builder.SharedInformerFactory.Certmanager().V1alpha2().Certificates().Lister(),
certificateRequestLister: test.builder.SharedInformerFactory.Certmanager().V1alpha2().CertificateRequests().Lister(),
secretLister: secretsLister,
recorder: test.builder.Recorder,
clock: fixedClock,
client: test.builder.CMClient,
secretsManager: secretManager,
}
// Instantiate/setup the controller
w := controllerWrapper{}
w.Register(test.builder.Context)

// Start the unit test builder
test.builder.Start()

key, err := cache.MetaNamespaceKeyFunc(test.certificate)
Expand All @@ -482,7 +468,7 @@ func TestIssuingController(t *testing.T) {
t.FailNow()
}

err = testManager.ProcessItem(context.Background(), key)
err = w.controller.ProcessItem(context.Background(), key)
if err != nil && !test.expectedErr {
t.Errorf("expected to not get an error, but got: %v", err)
}
Expand Down

0 comments on commit 572e467

Please sign in to comment.