Skip to content

Commit

Permalink
Enable errorlint and fix all issues (#1223)
Browse files Browse the repository at this point in the history
* Correctly wrap errors everywhere

* Use errors.As and errors.Is for safe checks and casts

* Fix condition
  • Loading branch information
markusthoemmes authored Feb 13, 2021
1 parent d647e44 commit 9667e45
Show file tree
Hide file tree
Showing 18 changed files with 41 additions and 34 deletions.
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ run:

linters:
enable:
- errorlint
- unconvert
- prealloc
disable:
Expand Down
2 changes: 1 addition & 1 deletion cmd/kn/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func run(args []string) error {
// Get only the args provided but no options
func stripFlags(rootCmd *cobra.Command, args []string) ([]string, error) {
if err := rootCmd.ParseFlags(filterHelpOptions(args)); err != nil {
return []string{}, fmt.Errorf("error while parsing flags from args %v: %s", args, err.Error())
return []string{}, fmt.Errorf("error while parsing flags from args %v: %w", args, err)
}
return rootCmd.Flags().Args(), nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/dynamic/lib.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func groupFromUnstructured(u *unstructured.Unstructured) (string, error) {
content := u.UnstructuredContent()
group, found, err := unstructured.NestedString(content, "spec", "group")
if err != nil || !found {
return "", fmt.Errorf("can't find group for source GVR: %v", err)
return "", fmt.Errorf("can't find group for source GVR: %w", err)
}
return group, nil
}
Expand All @@ -62,7 +62,7 @@ func versionFromUnstructured(u *unstructured.Unstructured) (version string, err
// fallback to .spec.version
version, found, err = unstructured.NestedString(content, "spec", "version")
if err != nil || !found {
return version, fmt.Errorf("can't find version for source GVR: %v", err)
return version, fmt.Errorf("can't find version for source GVR: %w", err)
}
} else {
for _, v := range versions {
Expand All @@ -86,7 +86,7 @@ func resourceFromUnstructured(u *unstructured.Unstructured) (string, error) {
content := u.UnstructuredContent()
resource, found, err := unstructured.NestedString(content, "spec", "names", "plural")
if err != nil || !found {
return "", fmt.Errorf("can't find resource for source GVR: %v", err)
return "", fmt.Errorf("can't find resource for source GVR: %w", err)
}
return resource, nil
}
Expand All @@ -95,7 +95,7 @@ func kindFromUnstructured(u *unstructured.Unstructured) (string, error) {
content := u.UnstructuredContent()
kind, found, err := unstructured.NestedString(content, "spec", "names", "kind")
if !found || err != nil {
return "", fmt.Errorf("can't find source kind from source CRD: %v", err)
return "", fmt.Errorf("can't find source kind from source CRD: %w", err)
}
return kind, nil
}
Expand Down
20 changes: 11 additions & 9 deletions pkg/errors/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package errors

import (
"errors"
"net/http"
"strings"

Expand Down Expand Up @@ -52,17 +53,17 @@ func GetError(err error) error {
case isNoRouteToHostError(err):
return newNoRouteToHost(err.Error())
default:
apiStatus, ok := err.(api_errors.APIStatus)
if !ok {
var errAPIStatus api_errors.APIStatus
if !errors.As(err, &errAPIStatus) {
return err
}
if apiStatus.Status().Details == nil {
if errAPIStatus.Status().Details == nil {
return err
}
var knerr *KNError
if isCRDError(apiStatus) {
knerr = newInvalidCRD(apiStatus.Status().Details.Group)
knerr.Status = apiStatus
if isCRDError(errAPIStatus) {
knerr = newInvalidCRD(errAPIStatus.Status().Details.Group)
knerr.Status = errAPIStatus
return knerr
}
return err
Expand All @@ -71,8 +72,9 @@ func GetError(err error) error {

// IsForbiddenError returns true if given error can be converted to API status and of type forbidden access else false
func IsForbiddenError(err error) bool {
if status, ok := err.(api_errors.APIStatus); ok {
return status.Status().Code == int32(http.StatusForbidden)
var errAPIStatus api_errors.APIStatus
if !errors.As(err, &errAPIStatus) {
return false
}
return false
return errAPIStatus.Status().Code == int32(http.StatusForbidden)
}
2 changes: 1 addition & 1 deletion pkg/kn/commands/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ kn options`,
FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true}, // wokeignore:rule=whitelist // TODO(#1031)
}
cmd.SetFlagErrorFunc(func(c *cobra.Command, err error) error {
return fmt.Errorf("%s for '%s'", err.Error(), c.CommandPath())
return fmt.Errorf("%w for '%s'", err, c.CommandPath())
})
cmd.SetUsageFunc(templates.NewGlobalOptionsFunc())
cmd.SetHelpFunc(func(command *cobra.Command, args []string) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/kn/commands/service/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func getRevisionDescriptions(client clientservingv1.KnServingClient, service *se
for _, target := range trafficTargets {
revision, err := extractRevisionFromTarget(client, target)
if err != nil {
return nil, fmt.Errorf("cannot extract revision from service %s: %v", service.Name, err)
return nil, fmt.Errorf("cannot extract revision from service %s: %w", service.Name, err)
}
revisionsSeen.Insert(revision.Name)
desc, err := newRevisionDesc(*revision, &target, service)
Expand Down Expand Up @@ -353,7 +353,7 @@ func completeWithUntargetedRevisions(client clientservingv1.KnServingClient, ser
func newRevisionDesc(revision servingv1.Revision, target *servingv1.TrafficTarget, service *servingv1.Service) (*revisionDesc, error) {
generation, err := strconv.ParseInt(revision.Labels[serving.ConfigurationGenerationLabelKey], 0, 0)
if err != nil {
return nil, fmt.Errorf("cannot extract configuration generation for revision %s: %v", revision.Name, err)
return nil, fmt.Errorf("cannot extract configuration generation for revision %s: %w", revision.Name, err)
}
revisionDesc := revisionDesc{
revision: &revision,
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func waitForService(client clientservingv1.KnServingClient, serviceName string,
func showUrl(client clientservingv1.KnServingClient, serviceName string, originalRevision string, what string, out io.Writer) error {
service, err := client.GetService(serviceName)
if err != nil {
return fmt.Errorf("cannot fetch service '%s' in namespace '%s' for extracting the URL: %v", serviceName, client.Namespace(), err)
return fmt.Errorf("cannot fetch service '%s' in namespace '%s' for extracting the URL: %w", serviceName, client.Namespace(), err)
}

url := service.Status.URL.String()
Expand Down
3 changes: 2 additions & 1 deletion pkg/kn/commands/service/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
var baseRevision *servingv1.Revision
if !cmd.Flags().Changed("image") && editFlags.LockToDigest {
baseRevision, err = client.GetBaseRevision(service)
if _, ok := err.(*clientservingv1.NoBaseRevisionError); ok {
var errNoBaseRevision clientservingv1.NoBaseRevisionError
if errors.As(err, &errNoBaseRevision) {
fmt.Fprintf(cmd.OutOrStdout(), "Warning: No revision found to update image digest")
}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/kn/commands/source/duck/multisourcelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func sinkFromUnstructured(u *unstructured.Unstructured) (*duckv1.Destination, er
content := u.UnstructuredContent()
sink, found, err := unstructured.NestedFieldCopy(content, "spec", "sink")
if err != nil {
return nil, fmt.Errorf("cant find sink in given unstructured object at spec.sink field: %v", err)
return nil, fmt.Errorf("cant find sink in given unstructured object at spec.sink field: %w", err)
}

if !found {
Expand All @@ -118,12 +118,12 @@ func sinkFromUnstructured(u *unstructured.Unstructured) (*duckv1.Destination, er

sinkM, err := json.Marshal(sink)
if err != nil {
return nil, fmt.Errorf("error marshaling sink %v: %v", sink, err)
return nil, fmt.Errorf("error marshaling sink %v: %w", sink, err)
}

var sinkD duckv1.Destination
if err := json.Unmarshal(sinkM, &sinkD); err != nil {
return nil, fmt.Errorf("failed to unmarshal source sink: %v", err)
return nil, fmt.Errorf("failed to unmarshal source sink: %w", err)
}

return &sinkD, nil
Expand All @@ -133,17 +133,17 @@ func conditionsFromUnstructured(u *unstructured.Unstructured) (*duckv1.Condition
content := u.UnstructuredContent()
conds, found, err := unstructured.NestedFieldCopy(content, "status", "conditions")
if !found || err != nil {
return nil, fmt.Errorf("cant find conditions in given unstructured object at status.conditions field: %v", err)
return nil, fmt.Errorf("cant find conditions in given unstructured object at status.conditions field: %w", err)
}

condsM, err := json.Marshal(conds)
if err != nil {
return nil, fmt.Errorf("error marshaling conditions %v: %v", conds, err)
return nil, fmt.Errorf("error marshaling conditions %v: %w", conds, err)
}

var condsD duckv1.Conditions
if err := json.Unmarshal(condsM, &condsD); err != nil {
return nil, fmt.Errorf("failed to unmarshal source status conditions: %v", err)
return nil, fmt.Errorf("failed to unmarshal source status conditions: %w", err)
}

return &condsD, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/trigger/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func NewTriggerUpdateCommand(p *commands.KnParams) *cobra.Command {
updated, removed, err := triggerUpdateFlags.GetUpdateFilters()
if err != nil {
return fmt.Errorf(
"cannot update trigger '%s' because %s", name, err)
"cannot update trigger '%s' because %w", name, err)
}
existing := extractFilters(trigger)
b.Filters(existing.Merge(updated).Remove(removed))
Expand Down
3 changes: 2 additions & 1 deletion pkg/kn/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package config

import (
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -106,7 +107,7 @@ func BootstrapConfig() error {
bootstrapFlagSet.ParseErrorsWhitelist = flag.ParseErrorsWhitelist{UnknownFlags: true} // wokeignore:rule=whitelist // TODO(#1031)
bootstrapFlagSet.Usage = func() {}
err := bootstrapFlagSet.Parse(os.Args)
if err != nil && err != flag.ErrHelp {
if err != nil && !errors.Is(err, flag.ErrHelp) {
return err
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/kn/plugin/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package plugin

import (
"errors"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -449,7 +450,7 @@ func findInDirOrPath(name string, dir string, lookupInPath bool) (string, error)
// Found in path
return path, nil
}
if execErr, ok := err.(*exec.Error); !ok || execErr.Unwrap() != exec.ErrNotFound {
if !errors.Is(err, exec.ErrNotFound) {
return "", fmt.Errorf("error for looking up %s in path: %w", name, err)
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/kn/plugin/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package plugin

import (
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -83,7 +84,7 @@ func verifyPath(path string, seenPlugins map[string]string, eaw VerificationErro
// Verify that plugin actually exists
fileInfo, err := os.Stat(path)
if err != nil {
if err == os.ErrNotExist {
if errors.Is(err, os.ErrNotExist) {
return eaw.AddError("cannot find plugin in %s", path)
}
return eaw.AddError("cannot stat %s: %v", path, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/root/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func NewRootCommand(helpFuncs *template.FuncMap) (*cobra.Command, error) {

// Add some command context when flags can not be parsed
rootCmd.SetFlagErrorFunc(func(c *cobra.Command, err error) error {
return fmt.Errorf("%s for '%s'", err.Error(), c.CommandPath())
return fmt.Errorf("%w for '%s'", err, c.CommandPath())
})

// For glog parse error. TOO: Check why this is needed
Expand Down
2 changes: 1 addition & 1 deletion pkg/printers/tablegenerator.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (h *HumanReadablePrinter) GenerateTable(obj runtime.Object, options PrintOp
func (h *HumanReadablePrinter) TableHandler(columnDefinitions []metav1beta1.TableColumnDefinition, printFunc interface{}) error {
printFuncValue := reflect.ValueOf(printFunc)
if err := ValidateRowPrintHandlerFunc(printFuncValue); err != nil {
utilruntime.HandleError(fmt.Errorf("unable to register print function: %v", err))
utilruntime.HandleError(fmt.Errorf("unable to register print function: %w", err))
return err
}
entry := &handlerEntry{
Expand Down
2 changes: 1 addition & 1 deletion pkg/serving/config_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func UpdateMaxScale(template *servingv1.RevisionTemplateSpec, max int) error {
func UpdateAutoscaleWindow(template *servingv1.RevisionTemplateSpec, window string) error {
_, err := time.ParseDuration(window)
if err != nil {
return fmt.Errorf("invalid duration for 'autoscale-window': %v", err)
return fmt.Errorf("invalid duration for 'autoscale-window': %w", err)
}
return UpdateRevisionTemplateAnnotation(template, autoscaling.WindowAnnotationKey, window)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/logging_http_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (t *LoggingHttpTransport) RoundTrip(r *http.Request) (*http.Response, error
reqBytes, err := httputil.DumpRequestOut(r, true)
if err != nil {
fmt.Fprintln(stream, "error dumping request:", err)
return nil, fmt.Errorf("dumping request: %v", err)
return nil, fmt.Errorf("dumping request: %w", err)
}
fmt.Fprintln(stream, "===== REQUEST =====")
fmt.Fprintln(stream, string(reqBytes))
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/logging_http_transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type fakeTransport struct {
func (d *fakeTransport) RoundTrip(r *http.Request) (*http.Response, error) {
dump, err := httputil.DumpRequest(r, true)
if err != nil {
return nil, fmt.Errorf("dumping request: %v", err)
return nil, fmt.Errorf("dumping request: %w", err)
}
d.requestDump = string(dump)
return &http.Response{
Expand Down

0 comments on commit 9667e45

Please sign in to comment.