Skip to content

Commit

Permalink
Hygiene: enable errorlint.
Browse files Browse the repository at this point in the history
There are no expected functional changes in this PR.

Made non-functional code changes to adhere to Golang error standards.

Context: #5899

See related style docs:
- https://pkg.go.dev/errors
- https://go.dev/blog/errors-are-values

/kind cleanup

- [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing
- [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed
- [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits)
- [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code)
- [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
- [N/A] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
- [N/A] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

```release-note
NONE
```
  • Loading branch information
bendory committed Mar 30, 2023
1 parent 7a40e9d commit c54cbbb
Show file tree
Hide file tree
Showing 39 changed files with 103 additions and 92 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ linters:
- errcheck
- errchkjson
- errname
# errorlint TODO: cleanup the issues this linter uncovers https://github.com/tektoncd/pipeline/issues/5899
- errorlint
- goconst
- gocritic
- gofmt
Expand Down
10 changes: 5 additions & 5 deletions cmd/entrypoint/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"encoding/json"
"errors"
"flag"
"fmt"
"log"
Expand Down Expand Up @@ -97,12 +98,11 @@ func main() {
}
if err := subcommands.Process(flag.CommandLine.Args()); err != nil {
log.Println(err.Error())
switch err.(type) {
case subcommands.OK:
var ok subcommands.OK
if errors.As(err, &ok) {
return
default:
os.Exit(1)
}
os.Exit(1)
}

// Copy credentials we're expecting from the legacy credentials helper (creds-init)
Expand Down Expand Up @@ -174,7 +174,7 @@ func main() {

if err := e.Go(); err != nil {
breakpointExitPostFile := e.PostFile + breakpointExitSuffix
switch t := err.(type) {
switch t := err.(type) { // nolint -- checking for multiple types with errors.As is ugly.
case skipError:
log.Print("Skipping step because a previous step failed")
os.Exit(1)
Expand Down
3 changes: 2 additions & 1 deletion cmd/entrypoint/namespaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ limitations under the License.
package main

import (
"errors"
"os/exec"
"testing"

Expand Down Expand Up @@ -60,7 +61,7 @@ func TestDropNetworking(t *testing.T) {

b1, err1 := withNetworking.CombinedOutput()
b2, err2 := withoutNetworking.CombinedOutput()
if err1 != err2 {
if !errors.Is(err1, err2) {
t.Errorf("Expected no errors, got %v %v", err1, err2)
}
if diff := cmp.Diff(string(b1), string(b2)); diff != "" {
Expand Down
5 changes: 3 additions & 2 deletions cmd/entrypoint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package main

import (
"context"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -116,7 +117,7 @@ func (rr *realRunner) Run(ctx context.Context, args ...string) error {

// Start defined command
if err := cmd.Start(); err != nil {
if ctx.Err() == context.DeadlineExceeded {
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return context.DeadlineExceeded
}
return err
Expand All @@ -134,7 +135,7 @@ func (rr *realRunner) Run(ctx context.Context, args ...string) error {

// Wait for command to exit
if err := cmd.Wait(); err != nil {
if ctx.Err() == context.DeadlineExceeded {
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return context.DeadlineExceeded
}
return err
Expand Down
3 changes: 2 additions & 1 deletion cmd/entrypoint/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ func TestRealRunnerTimeout(t *testing.T) {
timeout := time.Millisecond
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

if err := rr.Run(ctx, "sleep", "0.01"); err != nil {
if err != context.DeadlineExceeded {
if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("unexpected error received: %v", err)
}
} else {
Expand Down
9 changes: 6 additions & 3 deletions cmd/entrypoint/subcommands/subcommands_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package subcommands

import (
"errors"
"os"
"path/filepath"
"testing"
Expand All @@ -24,6 +25,8 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
t.Fatalf("error writing source file: %v", err)
}

var ok OK

for _, tc := range []struct {
command string
args []string
Expand All @@ -39,7 +42,7 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
} {
t.Run(tc.command, func(t *testing.T) {
returnValue := Process(append([]string{tc.command}, tc.args...))
if _, ok := returnValue.(OK); !ok {
if !errors.As(returnValue, &ok) {
t.Errorf("unexpected return value from command: %v", returnValue)
}
})
Expand All @@ -49,12 +52,12 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
tektonRoot = tmp

returnValue := Process([]string{StepInitCommand})
if _, ok := returnValue.(OK); !ok {
if !errors.As(returnValue, &ok) {
t.Errorf("unexpected return value from step-init command: %v", returnValue)
}

returnValue = Process([]string{StepInitCommand, "foo", "bar"})
if _, ok := returnValue.(OK); !ok {
if !errors.As(returnValue, &ok) {
t.Errorf("unexpected return value from step-init command w/ params: %v", returnValue)
}
})
Expand Down
9 changes: 5 additions & 4 deletions cmd/entrypoint/waiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package main

import (
"errors"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -149,11 +150,11 @@ func TestRealWaiterWaitWithErrorWaitfile(t *testing.T) {
if err == nil {
t.Errorf("expected skipError upon encounter error waitfile")
}
switch typ := err.(type) {
case skipError:
var skipErr skipError
if errors.As(err, &skipErr) {
close(doneCh)
default:
t.Errorf("unexpected error type %T", typ)
} else {
t.Errorf("unexpected error type %T", err)
}
}()
delay := time.NewTimer(2 * testWaitPollingInterval)
Expand Down
3 changes: 2 additions & 1 deletion internal/sidecarlogresults/sidecarlogresults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -181,7 +182,7 @@ func TestExtractResultsFromLogs_Failure(t *testing.T) {
logs := strings.NewReader(podLogs)

_, err := extractResultsFromLogs(logs, []v1beta1.PipelineResourceResult{}, 4096)
if err != ErrSizeExceeded {
if !errors.Is(err, ErrSizeExceeded) {
t.Fatalf("Expected error %v but got %v", ErrSizeExceeded, err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if cfg, ok := cfgMap[key]; ok {
value, err := strconv.ParseBool(cfg)
if err != nil {
return fmt.Errorf("failed parsing feature flags config %q: %v", cfg, err)
return fmt.Errorf("failed parsing feature flags config %q: %w", cfg, err)
}
*feature = value
return nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/config/resolver/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if cfg, ok := cfgMap[key]; ok {
value, err := strconv.ParseBool(cfg)
if err != nil {
return fmt.Errorf("failed parsing feature flags config %q: %v", cfg, err)
return fmt.Errorf("failed parsing feature flags config %q: %w", cfg, err)
}
*feature = value
return nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (r *ResultType) UnmarshalJSON(data []byte) error {
var asString string

if err := json.Unmarshal(data, &asString); err != nil {
return fmt.Errorf("unsupported value type, neither int nor string: %v", multierror.Append(intErr, err).ErrorOrNil())
return fmt.Errorf("unsupported value type, neither int nor string: %w", multierror.Append(intErr, err).ErrorOrNil())
}

switch asString {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/version/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
func SerializeToMetadata(meta *metav1.ObjectMeta, field interface{}, key string) error {
bytes, err := json.Marshal(field)
if err != nil {
return fmt.Errorf("error serializing field: %s", err)
return fmt.Errorf("error serializing field: %w", err)
}
if meta.Annotations == nil {
meta.Annotations = make(map[string]string)
Expand All @@ -46,7 +46,7 @@ func DeserializeFromMetadata(meta *metav1.ObjectMeta, to interface{}, key string
}
if str, ok := meta.Annotations[key]; ok {
if err := json.Unmarshal([]byte(str), to); err != nil {
return fmt.Errorf("error deserializing key %s from metadata: %s", key, err)
return fmt.Errorf("error deserializing key %s from metadata: %w", key, err)
}
delete(meta.Annotations, key)
if len(meta.Annotations) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/entrypoint/entrypointer.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (e Entrypointer) Go() error {
defer cancel()
}
err = e.Runner.Run(ctx, e.Command...)
if err == context.DeadlineExceeded {
if errors.Is(err, context.DeadlineExceeded) {
output = append(output, v1beta1.PipelineResourceResult{
Key: "Reason",
Value: "TimeoutExceeded",
Expand Down
2 changes: 1 addition & 1 deletion pkg/pipelinerunmetrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (r *Recorder) RunningPipelineRuns(lister listers.PipelineRunLister) error {

prs, err := lister.List(labels.Everything())
if err != nil {
return fmt.Errorf("failed to list pipelineruns while generating metrics : %v", err)
return fmt.Errorf("failed to list pipelineruns while generating metrics : %w", err)
}

var runningPRs int
Expand Down
2 changes: 1 addition & 1 deletion pkg/pod/entrypoint_lookup_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (e *entrypointCache) get(ctx context.Context, ref name.Reference, namespace
ImagePullSecrets: pullSecretsNames,
})
if err != nil {
return nil, fmt.Errorf("error creating k8schain: %v", err)
return nil, fmt.Errorf("error creating k8schain: %w", err)
}

desc, err := remote.Get(ref, remote.WithAuthFromKeychain(kc))
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/events/k8sevent/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func CheckEventsOrdered(t *testing.T, eventChan chan string, testName string, wa
t.Helper()
err := eventsFromChannel(eventChan, wantEvents)
if err != nil {
return fmt.Errorf("error in test %s: %v", testName, err)
return fmt.Errorf("error in test %s: %w", testName, err)
}
return nil
}
Expand Down Expand Up @@ -65,7 +65,7 @@ func eventsFromChannel(c chan string, wantEvents []string) error {
return fmt.Errorf("expected event \"%s\" but got \"%s\" instead", wantEvent, event)
}
} else {
return fmt.Errorf("something went wrong matching the event: %s", err)
return fmt.Errorf("something went wrong matching the event: %w", err)
}
case <-timer:
return fmt.Errorf("received %d events but %d expected. Found events: %#v", len(foundEvents), len(wantEvents), foundEvents)
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/pipelinerun/affinity_assistant.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ func (c *Reconciler) createAffinityAssistants(ctx context.Context, wb []v1beta1.
affinityAssistantStatefulSet := affinityAssistantStatefulSet(affinityAssistantName, pr, claimName, c.Images.NopImage, cfg.Defaults.DefaultAAPodTemplate)
_, err := c.KubeClientSet.AppsV1().StatefulSets(namespace).Create(ctx, affinityAssistantStatefulSet, metav1.CreateOptions{})
if err != nil {
errs = append(errs, fmt.Errorf("failed to create StatefulSet %s: %s", affinityAssistantName, err))
errs = append(errs, fmt.Errorf("failed to create StatefulSet %s: %w", affinityAssistantName, err))
}
if err == nil {
logger.Infof("Created StatefulSet %s in namespace %s", affinityAssistantName, namespace)
}
case err != nil:
errs = append(errs, fmt.Errorf("failed to retrieve StatefulSet %s: %s", affinityAssistantName, err))
errs = append(errs, fmt.Errorf("failed to retrieve StatefulSet %s: %w", affinityAssistantName, err))
}
}
}
Expand Down Expand Up @@ -97,7 +97,7 @@ func (c *Reconciler) cleanupAffinityAssistants(ctx context.Context, pr *v1beta1.
if w.PersistentVolumeClaim != nil || w.VolumeClaimTemplate != nil {
affinityAssistantStsName := getAffinityAssistantName(w.Name, pr.Name)
if err := c.KubeClientSet.AppsV1().StatefulSets(pr.Namespace).Delete(ctx, affinityAssistantStsName, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
errs = append(errs, fmt.Errorf("failed to delete StatefulSet %s: %s", affinityAssistantStsName, err))
errs = append(errs, fmt.Errorf("failed to delete StatefulSet %s: %w", affinityAssistantStsName, err))
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/pipelinerun/cancel.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func cancelPipelineTaskRunsForTaskNames(ctx context.Context, logger *zap.Sugared
logger.Infof("cancelling TaskRun %s", taskRunName)

if err := cancelTaskRun(ctx, taskRunName, pr.Namespace, clientSet); err != nil {
errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %s", taskRunName, err).Error())
errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %w", taskRunName, err).Error())
continue
}
}
Expand All @@ -172,7 +172,7 @@ func cancelPipelineTaskRunsForTaskNames(ctx context.Context, logger *zap.Sugared
logger.Infof("cancelling CustomRun %s", runName)

if err := cancelCustomRun(ctx, runName, pr.Namespace, clientSet); err != nil {
errs = append(errs, fmt.Errorf("Failed to patch CustomRun `%s` with cancellation: %s", runName, err).Error())
errs = append(errs, fmt.Errorf("Failed to patch CustomRun `%s` with cancellation: %w", runName, err).Error())
continue
}
}
Expand All @@ -181,7 +181,7 @@ func cancelPipelineTaskRunsForTaskNames(ctx context.Context, logger *zap.Sugared
logger.Infof("cancelling Run %s", runName)

if err := cancelRun(ctx, runName, pr.Namespace, clientSet); err != nil {
errs = append(errs, fmt.Errorf("Failed to patch Run `%s` with cancellation: %s", runName, err).Error())
errs = append(errs, fmt.Errorf("Failed to patch Run `%s` with cancellation: %w", runName, err).Error())
continue
}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
// list VerificationPolicies for trusted resources
vp, err := c.verificationPolicyLister.VerificationPolicies(pr.Namespace).List(labels.Everything())
if err != nil {
return fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", pr.Namespace, err)
return fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %w", pr.Namespace, err)
}
getPipelineFunc := resources.GetVerifiedPipelineFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, vp)

Expand Down Expand Up @@ -317,7 +317,7 @@ func (c *Reconciler) resolvePipelineState(
// list VerificationPolicies for trusted resources
vp, err := c.verificationPolicyLister.VerificationPolicies(pr.Namespace).List(labels.Everything())
if err != nil {
return nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", pr.Namespace, err)
return nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %w", pr.Namespace, err)
}
fn := tresources.GetVerifiedTaskFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, task.TaskRef, trName, pr.Namespace, pr.Spec.ServiceAccountName, vp)

Expand Down Expand Up @@ -370,12 +370,12 @@ func (c *Reconciler) resolvePipelineState(
pr.Status.MarkFailed(ReasonResourceVerificationFailed, message)
return nil, controller.NewPermanentError(err)
}
switch err := err.(type) {
case *resources.TaskNotFoundError:
var nfErr *resources.TaskNotFoundError
if errors.As(err, &nfErr) {
pr.Status.MarkFailed(ReasonCouldntGetTask,
"Pipeline %s/%s can't be Run; it contains Tasks that don't exist: %s",
pipelineMeta.Namespace, pipelineMeta.Name, err)
default:
pipelineMeta.Namespace, pipelineMeta.Name, nfErr)
} else {
pr.Status.MarkFailed(ReasonFailedValidation,
"PipelineRun %s/%s can't be Run; couldn't resolve all references: %s",
pipelineMeta.Namespace, pr.Name, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/pipelineref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) {
name: "get error when remote resolution return error",
requester: requesterUnsigned,
pipelinerun: *prResolutionError,
expectedErr: fmt.Errorf("failed to get pipeline: error accessing data from remote resource: %v", resolvedUnsigned.DataErr),
expectedErr: fmt.Errorf("failed to get pipeline: error accessing data from remote resource: %w", resolvedUnsigned.DataErr),
},
}
for _, tc := range testcases {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1934,13 +1934,14 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) {
}
for _, pt := range pts {
_, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt)
switch err := err.(type) {
case nil:
t.Fatalf("Expected error getting non-existent Tasks for Pipeline %s but got none", p.Name)
case *TaskNotFoundError:
var tnf *TaskNotFoundError
switch {
case err == nil:
t.Fatalf("Pipeline %s: want error, got nil", p.Name)
case errors.As(err, &tnf):
// expected error
default:
t.Fatalf("Expected specific error type returned by func for non-existent Task for Pipeline %s but got %s", p.Name, err)
t.Fatalf("Pipeline %s: Want %T, got %s of type %T", p.Name, tnf, err, err)
}
}
}
Expand Down Expand Up @@ -1976,7 +1977,7 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) {
if err == nil {
t.Errorf("expected to get err but got nil")
}
if err != trustedresources.ErrResourceVerificationFailed {
if !errors.Is(err, trustedresources.ErrResourceVerificationFailed) {
t.Errorf("expected to get %v but got %v", trustedresources.ErrResourceVerificationFailed, err)
}
}
Expand Down
Loading

0 comments on commit c54cbbb

Please sign in to comment.