From 162777aab20a9224c35f1c6fe7a7aaf522710490 Mon Sep 17 00:00:00 2001 From: Renato Costa Date: Fri, 26 Aug 2022 15:08:30 -0400 Subject: [PATCH 1/2] Fix incorrect uses of loop variable This fixes two instances where loop variables were being incorrectly used: - using a loop variable in a closure passed to `ginkgo.It()` is incorrect, as the capture happens by reference and only the last test case will be executed (multiple times). - a similar issue happens in the context of a goroutine; specifically, we need to create a copy of the `runDurationFunc` before calling it in a goroutine as done by the controller's `Run` function. With regards to the second issue, I believe it never came to the surface because, in production code, only one `runDurationFunc` is passed; tests don't exercise the multiple funcs path either. Issues were automatically found with the `loopvarcapture` linter. Signed-off-by: Renato Costa --- pkg/controller/controller.go | 1 + test/e2e/suite/issuers/selfsigned/certificaterequest.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index a040e9d64..72e7a6dc9 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -119,6 +119,7 @@ func (c *controller) Run(workers int, stopCh <-chan struct{}) error { } for _, f := range c.runDurationFuncs { + f := f // capture range variable go wait.Until(func() { f.fn(ctx) }, f.duration, stopCh) } diff --git a/test/e2e/suite/issuers/selfsigned/certificaterequest.go b/test/e2e/suite/issuers/selfsigned/certificaterequest.go index cf1c42345..84996004d 100644 --- a/test/e2e/suite/issuers/selfsigned/certificaterequest.go +++ b/test/e2e/suite/issuers/selfsigned/certificaterequest.go @@ -163,6 +163,7 @@ var _ = framework.CertManagerDescribe("SelfSigned CertificateRequest", func() { }, } for _, v := range cases { + v := v // capture range variable It("should generate a signed certificate valid for "+v.label, func() { crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) From 5d170983223cd2bf221ab5691c2277ece0364c52 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Sun, 28 Aug 2022 21:17:08 +0200 Subject: [PATCH 2/2] fix broken test Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- test/e2e/suite/issuers/selfsigned/certificaterequest.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/suite/issuers/selfsigned/certificaterequest.go b/test/e2e/suite/issuers/selfsigned/certificaterequest.go index 84996004d..159709c78 100644 --- a/test/e2e/suite/issuers/selfsigned/certificaterequest.go +++ b/test/e2e/suite/issuers/selfsigned/certificaterequest.go @@ -173,6 +173,7 @@ var _ = framework.CertManagerDescribe("SelfSigned CertificateRequest", func() { _, err = crClient.Create(context.TODO(), gen.CertificateRequestFrom(basicCR, gen.SetCertificateRequestCSR(csr), + gen.SetCertificateRequestDuration(v.inputDuration), ), metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred())