Commit Graph

74 Commits

Author SHA1 Message Date
Richard Wall
6c544dafa0 Simplify the return statement
pkg/controller/certificates/trigger/trigger_controller_test.go:257:12: if block ends with a return statement, so drop this else and outdent its block

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
2021-05-04 14:57:20 +01:00
Maël Valais
8f5a094b0c trigger-controller: PR comment: failure mode -> failure state
Cf. https://github.com/jetstack/cert-manager/pull/3444#pullrequestreview-629189131

Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 19:14:49 +02:00
Maël Valais
181d4ee281 DataForCertificate: typo certitificate -> certificate
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 19:06:21 +02:00
Maël Valais
a7486d5025 DataForCertificate: "Failure" CR condition -> "Failed"
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 18:58:31 +02:00
Maël Valais
2361f355aa DataForCertificate: PR comment: certificate -> cert-manager certificate
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:44:26 +02:00
Maël Valais
de0de24aad DataForCertificate: PR comment: mode -> state
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:42:17 +02:00
Maël Valais
c875518da1 DataForCertificate: PR comment: mismatch -> does not match
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:34:18 +02:00
Maël Valais
8b41ec1d54 DataForCertificate: PR comment: distinguish X.509 vs. Kubernetes cert
The cert-manager team tends to use the word "certificate" for two very
different contexts:

1. sometimes, we use the word "certificate" to refer to a X.509
   certificate (a blob of ASN.1-encoded data and then PEM-formated);
2. and sometimes we refer to "certificate" as one item of the Kubernetes
   custom resource /apis/cert-manager.io/v1/certificates.

This commit makes sure the reader understands that we are talking about
the Kubernetes object here.

Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:25:48 +02:00
Maël Valais
a724f1ce31 DataForCertificate: PR comment: mismatches is a noun
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:59 +02:00
Maël Valais
c1d722b116 DataForCertificate: fix diagrams' Failed conditions
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 18:09:59 +02:00
Maël Valais
6c9477439c trigger-controller: hint people to look at gatherer.go diagrams
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 18:09:59 +02:00
Maël Valais
497f561ef7 DataForCertificate: hint people to look at gatherer.go diagrams
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 18:09:59 +02:00
Maël Valais
068a1c466f DataForCertificate: better wording for the "error returned"
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 18:09:59 +02:00
Maël Valais
f588d4138a DataForCertificate: explain what the "current" and "next" CRs are used for
Signed-off-by: Maël Valais <mael@vls.dev>
2021-04-06 18:09:47 +02:00
Maël Valais
a1a43b6784 DataForCertificate: PR comment: explain why we return a "duplicate CR" err
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:29 +02:00
Maël Valais
450d27f5d0 trigger-controller: PR comment: and -> if there is
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:28 +02:00
Maël Valais
c1bf35f4ed trigger-controller: further comments on shouldBackoffReissuingOnFailure
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Irbe Krumina <irbekrm@gmail.com>
2021-04-06 18:09:28 +02:00
Maël Valais
a2bbdb7c51 DataForCertificate: explain what is the "next" certificate request
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:28 +02:00
Maël Valais
27f258cf3c trigger-controller: PR comment: use a single "fixedClock"
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Irbe Krumina <irbekrm@gmail.com>
2021-04-06 18:09:28 +02:00
Maël Valais
36c2cc4d3b trigger-controller: PR comment: explain what "if nextCR != nil" is about
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Irbe Krumina <irbekrm@gmail.com>
2021-04-06 18:09:28 +02:00
Maël Valais
85128f26ce trigger-controller: PR comment: rephrase log about skipping issuance
The log message:

    multiple CertificateRequests found for the 'next' revision 2,
    skipping issuance until no more duplicate.

can be better phrased as:

    multiple CertificateRequests are found for the 'next' revision 2,
    issuance is skipped until there are no more duplicates.

Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:28 +02:00
Maël Valais
05c1fb9fc2 trigger-controller: reissue on mismatch using NextRevisionRequest
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:28 +02:00
Maël Valais
eb6d1399fc DataForCertificate: the func now fetches NextRevisionRequest
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:27 +02:00
Maël Valais
9305766ff2 trigger-controller: add two unit tests to showcase #3250
Note that I had initially made createCryptoBundle public since I found
it inconvenient to have to pass a testing.T when we know that we should
never be  failing inside this func (I mean, the failure  zould not be due
to a wrong test case).

After a comment from Maartje, I realize that I could just use an anonymous
function for that purpose.

Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-04-06 18:09:27 +02:00
jetstack-bot
7946df1da7
Merge pull request #3788 from maelvls/refactor-trigger-unit-tests
Refactor trigger-controller unit tests
2021-03-25 11:41:36 +00:00
Maël Valais
7e21f730cc PR comment: typo: "the following are" instead of "is"
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Jake Sanders <i@am.so-aweso.me>
2021-03-25 09:07:45 +01:00
Maël Valais
fe3617a41c PR comment: a sentence starts with a capital letter and ends with a dot
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
2021-03-24 19:19:34 +01:00
joshvanl
dd0b2bf510 Standardise the name of controllers so there is consistency across the
project

Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
2021-03-23 16:08:59 +00:00
Maël Valais
71e707387a trigger-controller: refactor test, inject gatherer and policychain
Injecting the whole Gatherer struct was not necessary for testing
since DataForCertificate is now fully unit-tested. With that, we
can mock the Gatherer.Evaluate function. Since there is no reason
to inject a full Gatherer object into the trigger controller, I chose
to inject a simple policies.Func. I named the function "shouldReissue"
since this is exactly what this function does.

I also refactored the test cases to use the same gen.Certificate
that we use in the rest of the codebase.

Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-23 13:55:11 +01:00
Maël Valais
cdb6c16c6d trigger-controller: log a msg when cert must be reissued
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-21 16:45:58 +01:00
irbekrm
0047174891 Update PR after rebase
Signed-off-by: irbekrm <irbekrm@gmail.com>
2021-03-15 09:00:07 +00:00
irbekrm
a89133b637 Better wording and wrap long comment lines.
Co-authored-by: Maël Valais <mael@vls.dev>
Signed-off-by: irbekrm <irbekrm@gmail.com>
2021-03-15 08:48:23 +00:00
irbekrm
245d0f5c27 Pass DefaultRenewBefore into trigger controller
Signed-off-by: irbekrm <irbekrm@gmail.com>
2021-03-15 08:48:02 +00:00
irbekrm
8d5059b13e Updates Trigger controller integration tests
Signed-off-by: irbekrm <irbekrm@gmail.com>
2021-03-15 08:47:42 +00:00
irbekrm
9e7cd99ea8 CurrentCertificateNearingExpiry looks at x509 cert to determine renewal time
Signed-off-by: irbekrm <irbekrm@gmail.com>
2021-03-15 08:44:14 +00:00
jetstack-bot
9f343ec581
Merge pull request #3475 from maelvls/unit-test-dataforcertificate
DataForCertificate: add unit tests
2021-03-09 18:13:51 +00:00
joshvanl
39a50a1903 Updates unit certificate controller tests to include ObservedGeneration
Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
2021-03-04 17:04:09 +00:00
Maël Valais
34c07a71ce DataForCertificate: force core/v1 informer to create the indexer
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:20:50 +01:00
Maël Valais
ac325bf4e0 PR comment: spelling
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:20:50 +01:00
Maël Valais
680c7b75f6 DataForCertificate: use fake clientset instead of fake lister
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:20:45 +01:00
Maël Valais
46e9cb6c5b DataForCertificates: remove unused "name" field
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:19:02 +01:00
Maël Valais
3af2cb6650 DataForCertificate: expand comments around expectCalled
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:19:02 +01:00
Maël Valais
e0ca10ef2d DataForCertificate: detail why "whereAmI" is used
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:19:02 +01:00
Maël Valais
65701e04ab DataForCertificate: check fake is called with correct input
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:19:01 +01:00
Maël Valais
8b3bec3c9c DataForCertificate: implement Josh's fake idea
Co-Authored-By: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:18:56 +01:00
Maël Valais
38919b7eb2 DataForCertificate: move certRef to test/unit/gen
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:16:16 +01:00
Maël Valais
92bf3c59a0 DataForCertificate: fix tests
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:16:16 +01:00
Maël Valais
5c1fba52a5 Mock lister: fix the wrong stack frames for certificaterequests
The stack frames displayed using assert.Fail was not very informative.
That is due to t.Cleanup being called "outside" of the test case
context. There was no mention of the test file itself, gatherer_test.go
in the following example:

 certificaterequest.go:205:
         Error Trace:    certificaterequest.go:205
                                                 testing.go:872
                                                 testing.go:866
                                                 testing.go:873
                                                 testing.go:949
                                                 testing.go:1121
         Error:          lister.CertificateRequests was expected to be called but was not called
         Test:           TestDataForCertificate/should_return_error_when_the_list_func_returns_an_error

With this patch that vendors a simple version of assert.Fail, we get the
correct stack frames that the user needs in order to locate where this
failure happened:

 certificaterequest.go:254:
         Error Trace:    gatherer_test.go:230
                         gatherer_test.go:240
         Error:          lister.CertificateRequests was expected to be called but was not called
         Test:           TestDataForCertificate/should_return_error_when_the_list_func_returns_an_error

Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:16:16 +01:00
Maël Valais
9eb43bbb96 DataForCertificate: document the behavior and explain "current"
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:16:15 +01:00
Maël Valais
754035de7d DataForCertificate: tests: chained funcs pattern for CR mock
Signed-off-by: Maël Valais <mael@vls.dev>
2021-03-04 17:16:15 +01:00