Skip to content

Commit

Permalink
Merge pull request #6975 from inteon/fix_linters_context
Browse files Browse the repository at this point in the history
Fix contextcheck linter
  • Loading branch information
cert-manager-prow[bot] committed May 7, 2024
2 parents 27fb916 + 1248be8 commit 4cae5b1
Show file tree
Hide file tree
Showing 118 changed files with 1,165 additions and 1,127 deletions.
2 changes: 0 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ issues:
- linters:
- dogsled
- errcheck
- contextcheck
- promlinter
- errname
- exhaustive
- nilerr
- interfacebloat
- noctx
- nilnil
- nakedret
- musttag
Expand Down
7 changes: 5 additions & 2 deletions cmd/acmesolver/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func NewACMESolverCommand(_ context.Context) *cobra.Command {

return nil
},
// nolint:contextcheck // False positive
RunE: func(cmd *cobra.Command, args []string) error {
runCtx := cmd.Context()
log := logf.FromContext(runCtx)
Expand All @@ -54,11 +55,13 @@ func NewACMESolverCommand(_ context.Context) *cobra.Command {
go func() {
defer close(completedCh)
<-runCtx.Done()

// allow a timeout for graceful shutdown
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

if err := s.Shutdown(ctx); err != nil {
// nolint: contextcheck
if err := s.Shutdown(shutdownCtx); err != nil {
log.Error(err, "error shutting down acmesolver server")
}
}()
Expand Down
1 change: 1 addition & 0 deletions cmd/cainjector/app/cainjector.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ servers and webhook servers.`,

return nil
},
// nolint:contextcheck // False positive
RunE: func(cmd *cobra.Command, args []string) error {
return run(cmd.Context(), cainjectorConfig)
},
Expand Down
1 change: 1 addition & 0 deletions cmd/cainjector/app/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ func Run(opts *config.CAInjectorConfiguration, ctx context.Context) error {
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

// nolint: contextcheck
return server.Shutdown(shutdownCtx)
}))

Expand Down
18 changes: 7 additions & 11 deletions cmd/controller/app/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,11 @@ func Run(rootCtx context.Context, opts *config.ControllerConfiguration) error {
g.Go(func() error {
<-rootCtx.Done()
// allow a timeout for graceful shutdown
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

if err := metricsServer.Shutdown(ctx); err != nil {
return err
}
return nil
// nolint: contextcheck
return metricsServer.Shutdown(shutdownCtx)
})
g.Go(func() error {
log.V(logf.InfoLevel).Info("starting metrics server", "address", metricsLn.Addr())
Expand All @@ -149,13 +147,11 @@ func Run(rootCtx context.Context, opts *config.ControllerConfiguration) error {
g.Go(func() error {
<-rootCtx.Done()
// allow a timeout for graceful shutdown
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

if err := profilerServer.Shutdown(ctx); err != nil {
return err
}
return nil
// nolint: contextcheck
return profilerServer.Shutdown(shutdownCtx)
})
g.Go(func() error {
log.V(logf.InfoLevel).Info("starting profiler", "address", profilerLn.Addr())
Expand Down Expand Up @@ -250,7 +246,7 @@ func Run(rootCtx context.Context, opts *config.ControllerConfiguration) error {
g.Go(func() error {
log.V(logf.InfoLevel).Info("starting controller")

return iface.Run(opts.NumberOfConcurrentWorkers, rootCtx.Done())
return iface.Run(opts.NumberOfConcurrentWorkers, rootCtx)
})
}

Expand Down
1 change: 1 addition & 0 deletions cmd/controller/app/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ to renew certificates at an appropriate time before expiry.`,

return nil
},
// nolint:contextcheck // False positive
RunE: func(cmd *cobra.Command, args []string) error {
return run(cmd.Context(), controllerConfig)
},
Expand Down
1 change: 1 addition & 0 deletions cmd/startupapicheck/pkg/check/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ required webhooks are reachable by the K8S API server.`,
PreRunE: func(cmd *cobra.Command, args []string) error {
return o.Complete()
},
// nolint:contextcheck // False positive
RunE: func(cmd *cobra.Command, args []string) error {
return o.Run(cmd.Context(), cmd.OutOrStdout())
},
Expand Down
1 change: 1 addition & 0 deletions cmd/webhook/app/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ functionality for cert-manager.`,

return nil
},
// nolint:contextcheck // False positive
RunE: func(cmd *cobra.Command, args []string) error {
return run(cmd.Context(), webhookConfig)
},
Expand Down
14 changes: 7 additions & 7 deletions internal/vault/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var _ Interface = &Vault{}

// ClientBuilder is a function type that returns a new Interface.
// Can be used in tests to create a mock signer of Vault certificate requests.
type ClientBuilder func(namespace string, _ func(ns string) CreateToken, _ internalinformers.SecretLister, _ v1.GenericIssuer) (Interface, error)
type ClientBuilder func(ctx context.Context, namespace string, _ func(ns string) CreateToken, _ internalinformers.SecretLister, _ v1.GenericIssuer) (Interface, error)

// Interface implements various high level functionality related to connecting
// with a Vault server, verifying its status and signing certificate request for
Expand Down Expand Up @@ -95,7 +95,7 @@ type Vault struct {
// secrets lister.
// Returned errors may be network failures and should be considered for
// retrying.
func New(namespace string, createTokenFn func(ns string) CreateToken, secretsLister internalinformers.SecretLister, issuer v1.GenericIssuer) (Interface, error) {
func New(ctx context.Context, namespace string, createTokenFn func(ns string) CreateToken, secretsLister internalinformers.SecretLister, issuer v1.GenericIssuer) (Interface, error) {
v := &Vault{
createToken: createTokenFn(namespace),
secretsLister: secretsLister,
Expand All @@ -120,7 +120,7 @@ func New(namespace string, createTokenFn func(ns string) CreateToken, secretsLis
// Use the (maybe) namespaced client to authenticate.
// If a Vault namespace is configured, then the authentication endpoints are
// expected to be in that namespace.
if err := v.setToken(clientNS); err != nil {
if err := v.setToken(ctx, clientNS); err != nil {
return nil, err
}

Expand Down Expand Up @@ -180,7 +180,7 @@ func (v *Vault) Sign(csrPEM []byte, duration time.Duration) (cert []byte, ca []b
return extractCertificatesFromVaultCertificateSecret(&vaultResult)
}

func (v *Vault) setToken(client Client) error {
func (v *Vault) setToken(ctx context.Context, client Client) error {
// IMPORTANT: Because of backwards compatibility with older versions that
// incorrectly allowed multiple authentication methods to be specified at
// the time of validation, we must still allow multiple authentication methods
Expand Down Expand Up @@ -212,7 +212,7 @@ func (v *Vault) setToken(client Client) error {

kubernetesAuth := v.issuer.GetSpec().Vault.Auth.Kubernetes
if kubernetesAuth != nil {
token, err := v.requestTokenWithKubernetesAuth(client, kubernetesAuth)
token, err := v.requestTokenWithKubernetesAuth(ctx, client, kubernetesAuth)
if err != nil {
return fmt.Errorf("while requesting a Vault token using the Kubernetes auth: %w", err)
}
Expand Down Expand Up @@ -429,7 +429,7 @@ func (v *Vault) requestTokenWithAppRoleRef(client Client, appRole *v1.VaultAppRo
return token, nil
}

func (v *Vault) requestTokenWithKubernetesAuth(client Client, kubernetesAuth *v1.VaultKubernetesAuth) (string, error) {
func (v *Vault) requestTokenWithKubernetesAuth(ctx context.Context, client Client, kubernetesAuth *v1.VaultKubernetesAuth) (string, error) {
var jwt string
switch {
case kubernetesAuth.SecretRef.Name != "":
Expand Down Expand Up @@ -460,7 +460,7 @@ func (v *Vault) requestTokenWithKubernetesAuth(client Client, kubernetesAuth *v1
audiences := append([]string(nil), kubernetesAuth.ServiceAccountRef.TokenAudiences...)
audiences = append(audiences, defaultAudience)

tokenrequest, err := v.createToken(context.Background(), kubernetesAuth.ServiceAccountRef.Name, &authv1.TokenRequest{
tokenrequest, err := v.createToken(ctx, kubernetesAuth.ServiceAccountRef.Name, &authv1.TokenRequest{
Spec: authv1.TokenRequestSpec{
// Default audience is generated by cert-manager.
// This is the most secure configuration as vault role must explicitly mandate the audience.
Expand Down
5 changes: 4 additions & 1 deletion internal/vault/vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ func TestSetToken(t *testing.T) {
issuer: test.issuer,
}

err := v.setToken(test.fakeClient)
err := v.setToken(context.TODO(), test.fakeClient)
if ((test.expectedErr == nil) != (err == nil)) &&
test.expectedErr != nil &&
test.expectedErr.Error() != err.Error() {
Expand Down Expand Up @@ -1511,6 +1511,7 @@ func TestNewWithVaultNamespaces(t *testing.T) {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c, err := New(
context.TODO(),
"k8s-ns1",
func(ns string) CreateToken { return nil },
listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
Expand Down Expand Up @@ -1567,6 +1568,7 @@ func TestIsVaultInitiatedAndUnsealedIntegration(t *testing.T) {
defer server.Close()

v, err := New(
context.TODO(),
"k8s-ns1",
func(ns string) CreateToken { return nil },
listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
Expand Down Expand Up @@ -1632,6 +1634,7 @@ func TestSignIntegration(t *testing.T) {
defer server.Close()

v, err := New(
context.TODO(),
"k8s-ns1",
func(ns string) CreateToken { return nil },
listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
Expand Down
1 change: 1 addition & 0 deletions pkg/acme/webhook/cmd/server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func NewCommandStartWebhookServer(_ context.Context, groupName string, solvers .
cmd := &cobra.Command{
Short: "Launch an ACME solver API server",
Long: "Launch an ACME solver API server",
// nolint:contextcheck // False positive
RunE: func(c *cobra.Command, args []string) error {
runCtx := c.Context()

Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/acmechallenges/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (c *controller) Sync(ctx context.Context, chOriginal *cmacme.Challenge) (er
// means no CAA check is performed by ACME server or if any valid
// CAA would stop issuance (strongly suspect the former)
if len(dir.CAA) != 0 {
err := dnsutil.ValidateCAA(ch.Spec.DNSName, dir.CAA, ch.Spec.Wildcard, c.dns01Nameservers)
err := dnsutil.ValidateCAA(ctx, ch.Spec.DNSName, dir.CAA, ch.Spec.Wildcard, c.dns01Nameservers)
if err != nil {
ch.Status.Reason = fmt.Sprintf("CAA self-check failed: %s", err)
return err
Expand Down
6 changes: 1 addition & 5 deletions pkg/controller/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"context"
"fmt"
"time"

logf "github.com/cert-manager/cert-manager/pkg/logs"
)

// Builder is used to build controllers that implement the queuingController
Expand Down Expand Up @@ -72,8 +70,6 @@ func (b *Builder) Complete() (Interface, error) {
return nil, err
}

ctx := logf.NewContext(controllerctx.RootContext, logf.FromContext(controllerctx.RootContext), b.name)

if b.impl == nil {
return nil, fmt.Errorf("controller implementation must be non-nil")
}
Expand All @@ -82,5 +78,5 @@ func (b *Builder) Complete() (Interface, error) {
return nil, fmt.Errorf("error registering controller: %v", err)
}

return NewController(ctx, b.name, controllerctx.Metrics, b.impl.ProcessItem, mustSync, b.runDurationFuncs, queue), nil
return NewController(b.name, controllerctx.Metrics, b.impl.ProcessItem, mustSync, b.runDurationFuncs, queue), nil
}
2 changes: 1 addition & 1 deletion pkg/controller/certificaterequests/vault/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (v *Vault) Sign(ctx context.Context, cr *v1.CertificateRequest, issuerObj v

resourceNamespace := v.issuerOptions.ResourceNamespace(issuerObj)

client, err := v.vaultClientBuilder(resourceNamespace, v.createTokenFn, v.secretsLister, issuerObj)
client, err := v.vaultClientBuilder(ctx, resourceNamespace, v.createTokenFn, v.secretsLister, issuerObj)
if k8sErrors.IsNotFound(err) {
message := "Required secret resource not found"

Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/certificaterequests/vault/vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ func runTest(t *testing.T, test testT) {
vault := NewVault(test.builder.Context).(*Vault)

if test.fakeVault != nil {
vault.vaultClientBuilder = func(ns string, _ func(ns string) internalvault.CreateToken, sl internalinformers.SecretLister,
vault.vaultClientBuilder = func(_ context.Context, ns string, _ func(ns string) internalvault.CreateToken, sl internalinformers.SecretLister,
iss cmapi.GenericIssuer) (internalvault.Interface, error) {
return test.fakeVault.New(ns, sl, iss)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,14 +430,14 @@ func (c *controller) createNewCertificateRequest(ctx context.Context, crt *cmapi
return nil
}

if err := c.waitForCertificateRequestToExist(cr.Namespace, cr.Name); err != nil {
if err := c.waitForCertificateRequestToExist(ctx, cr.Namespace, cr.Name); err != nil {
return fmt.Errorf("failed whilst waiting for CertificateRequest to exist - this may indicate an apiserver running slowly. Request will be retried. %w", err)
}
return nil
}

func (c *controller) waitForCertificateRequestToExist(namespace, name string) error {
return wait.PollUntilContextTimeout(context.TODO(), time.Millisecond*100, time.Second*5, false, func(ctx context.Context) (bool, error) {
func (c *controller) waitForCertificateRequestToExist(ctx context.Context, namespace, name string) error {
return wait.PollUntilContextTimeout(ctx, time.Millisecond*100, time.Second*5, false, func(_ context.Context) (bool, error) {
_, err := c.certificateRequestLister.CertificateRequests(namespace).Get(name)
if apierrors.IsNotFound(err) {
return false, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/certificatesigningrequests/vault/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (v *Vault) Sign(ctx context.Context, csr *certificatesv1.CertificateSigning
resourceNamespace := v.issuerOptions.ResourceNamespace(issuerObj)

createTokenFn := func(ns string) internalvault.CreateToken { return v.kclient.CoreV1().ServiceAccounts(ns).CreateToken }
client, err := v.clientBuilder(resourceNamespace, createTokenFn, v.secretsLister, issuerObj)
client, err := v.clientBuilder(ctx, resourceNamespace, createTokenFn, v.secretsLister, issuerObj)
if apierrors.IsNotFound(err) {
message := "Required secret resource not found"
log.Error(err, message)
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/certificatesigningrequests/vault/vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestProcessItem(t *testing.T) {
Status: corev1.ConditionTrue,
}),
),
clientBuilder: func(_ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
clientBuilder: func(_ context.Context, _ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
return nil, apierrors.NewNotFound(schema.GroupResource{}, "test-secret")
},
builder: &testpkg.Builder{
Expand Down Expand Up @@ -191,7 +191,7 @@ func TestProcessItem(t *testing.T) {
Status: corev1.ConditionTrue,
}),
),
clientBuilder: func(_ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
clientBuilder: func(_ context.Context, _ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
return nil, errors.New("generic error")
},
expectedErr: true,
Expand Down Expand Up @@ -235,7 +235,7 @@ func TestProcessItem(t *testing.T) {
Status: corev1.ConditionTrue,
}),
),
clientBuilder: func(_ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
clientBuilder: func(_ context.Context, _ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
return fakevault.New(), nil
},
builder: &testpkg.Builder{
Expand Down Expand Up @@ -297,7 +297,7 @@ func TestProcessItem(t *testing.T) {
Status: corev1.ConditionTrue,
}),
),
clientBuilder: func(_ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
clientBuilder: func(_ context.Context, _ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
return fakevault.New().WithSign(nil, nil, errors.New("sign error")), nil
},
builder: &testpkg.Builder{
Expand Down Expand Up @@ -358,7 +358,7 @@ func TestProcessItem(t *testing.T) {
Status: corev1.ConditionTrue,
}),
),
clientBuilder: func(_ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
clientBuilder: func(_ context.Context, _ string, _ func(ns string) internalvault.CreateToken, _ internalinformers.SecretLister, _ cmapi.GenericIssuer) (internalvault.Interface, error) {
return fakevault.New().WithSign([]byte("signed-cert"), []byte("signing-ca"), nil), nil
},
builder: &testpkg.Builder{
Expand Down
5 changes: 0 additions & 5 deletions pkg/controller/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ type Context struct {
// RootContext is the root context for the controller
RootContext context.Context

// StopCh is a channel that will be closed when the controller is signalled
// to exit
StopCh <-chan struct{}

// FieldManager is the string that should be used as the field manager when
// applying API object. This value is derived from the user agent.
FieldManager string
Expand Down Expand Up @@ -316,7 +312,6 @@ func NewContextFactory(ctx context.Context, opts ContextOptions) (*ContextFactor
log: logf.FromContext(ctx),
ctx: &Context{
RootContext: ctx,
StopCh: ctx.Done(),
KubeSharedInformerFactory: kubeSharedInformerFactory,
SharedInformerFactory: sharedInformerFactory,
GWShared: gwSharedInformerFactory,
Expand Down

0 comments on commit 4cae5b1

Please sign in to comment.