From 4ce9a79706e1dcf64521ef195cbad396243b5975 Mon Sep 17 00:00:00 2001 From: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> Date: Tue, 1 Aug 2023 10:01:53 +0200 Subject: [PATCH] Enable type assertion linter (#1707) * Check type assertions * Change error * Fix assertion within map value check * Fix some explicitly disabled type assertions * Add timeout to make target * Set timeout for lint step * Set timeout in config file * Try higher timeout * Try gha lint pipeline * Try different format * Just copy paste from sample * Remove workflow * retrigger jobs --- .golangci.yml | 1 + Makefile | 4 ++-- cmd/kyma/alpha/create/module/module.go | 1 + .../enable/module/mock/mock_interactor.go | 1 + cmd/kyma/alpha/list/module/module.go | 23 ++++++++++++++----- internal/cli/alpha/module/module.go | 6 ++++- internal/deploy/bootstrap.go | 10 +++++++- internal/deploy/bootstrap_test.go | 9 +++++--- internal/deploy/kyma.go | 9 ++++++-- pkg/errs/common.go | 5 ++++ 10 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 pkg/errs/common.go diff --git a/.golangci.yml b/.golangci.yml index 036617199..0dd578368 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,6 +6,7 @@ linters: enable: - errcheck - exportloopref + - forcetypeassert - gochecknoinits - gofmt - gosec diff --git a/Makefile b/Makefile index 161701b9c..f95c818e1 100644 --- a/Makefile +++ b/Makefile @@ -14,12 +14,12 @@ LOCALBIN ?= $(shell pwd)/bin $(LOCALBIN): mkdir -p $(LOCALBIN) GOLANG_CI_LINT = $(LOCALBIN)/golangci-lint -GOLANG_CI_LINT_VERSION ?= v1.52.2 +GOLANG_CI_LINT_VERSION ?= v1.53.3 .PHONY: lint lint: GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANG_CI_LINT_VERSION) - $(LOCALBIN)/golangci-lint run -v ./... + $(LOCALBIN)/golangci-lint run -v FLAGS = -ldflags '-s -w -X github.com/kyma-project/cli/cmd/kyma/version.Version=$(VERSION)' diff --git a/cmd/kyma/alpha/create/module/module.go b/cmd/kyma/alpha/create/module/module.go index a1b6a6ad6..a66327f79 100644 --- a/cmd/kyma/alpha/create/module/module.go +++ b/cmd/kyma/alpha/create/module/module.go @@ -357,6 +357,7 @@ func (cmd *command) Run(ctx context.Context) error { } t, err := module.Template(componentVersionAccess, resourceName, channel, modDef.DefaultCR, labels, annotations) + if err != nil { cmd.CurrentStep.Failure() return err diff --git a/cmd/kyma/alpha/enable/module/mock/mock_interactor.go b/cmd/kyma/alpha/enable/module/mock/mock_interactor.go index 696373ec0..5067b460e 100644 --- a/cmd/kyma/alpha/enable/module/mock/mock_interactor.go +++ b/cmd/kyma/alpha/enable/module/mock/mock_interactor.go @@ -1,3 +1,4 @@ +//nolint:forcetypeassert package mock import ( diff --git a/cmd/kyma/alpha/list/module/module.go b/cmd/kyma/alpha/list/module/module.go index 013a6de7d..74d8940e7 100644 --- a/cmd/kyma/alpha/list/module/module.go +++ b/cmd/kyma/alpha/list/module/module.go @@ -192,7 +192,10 @@ func (cmd *command) printKymaActiveTemplates(ctx context.Context, kyma *unstruct templateList := &unstructured.UnstructuredList{Items: make([]unstructured.Unstructured, 0, len(statusItems))} for i := range statusItems { - item, _ := statusItems[i].(map[string]interface{}) + item, ok := statusItems[i].(map[string]interface{}) + if !ok { + continue + } tmplt, _, err := unstructured.NestedMap(item, "template") if err != nil { return fmt.Errorf("could not parse template: %w", err) @@ -214,12 +217,20 @@ func (cmd *command) printKymaActiveTemplates(ctx context.Context, kyma *unstruct if err != nil { return err } - anns := tpl.GetAnnotations() - if anns == nil { - anns = make(map[string]string) + annotations := tpl.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + stateValue, ok := item["state"] + if !ok { + continue + } + stateAnnotationValue, ok := stateValue.(string) + if !ok { + continue } - anns["state.cmd.kyma-project.io"] = item["state"].(string) - tpl.SetAnnotations(anns) + annotations["state.cmd.kyma-project.io"] = stateAnnotationValue + tpl.SetAnnotations(annotations) templateList.Items = append(templateList.Items, *tpl) if templateList.GetKind() == "" { templateList.SetGroupVersionKind(moduleTemplateResource.GroupVersion().WithKind(tpl.GetKind() + "List")) diff --git a/internal/cli/alpha/module/module.go b/internal/cli/alpha/module/module.go index dfc6b8e50..def22837f 100644 --- a/internal/cli/alpha/module/module.go +++ b/internal/cli/alpha/module/module.go @@ -3,6 +3,7 @@ package module import ( "context" "fmt" + "github.com/kyma-project/cli/pkg/errs" "time" "github.com/avast/retry-go" @@ -167,7 +168,10 @@ func (i *DefaultInteractor) WaitUntilReady(ctx context.Context) error { // It checks for v1beta2.StateReady, and if it is set, determines if this state can be trusted by observing // if the status fields match the desired state, and if the lastOperation is filled by the lifecycle-manager. func IsKymaReady(l *zap.SugaredLogger, obj runtime.Object) error { - kyma := obj.(*v1beta2.Kyma) + kyma, ok := obj.(*v1beta2.Kyma) + if !ok { + return errs.ErrTypeAssertKyma + } l.Info(kyma.Status) switch kyma.Status.State { case v1beta2.StateReady: diff --git a/internal/deploy/bootstrap.go b/internal/deploy/bootstrap.go index a6a64a682..397cf9be0 100644 --- a/internal/deploy/bootstrap.go +++ b/internal/deploy/bootstrap.go @@ -3,6 +3,7 @@ package deploy import ( "context" "encoding/json" + "github.com/pkg/errors" "regexp" appsv1 "k8s.io/api/apps/v1" @@ -113,12 +114,19 @@ func patchDeploymentWithInKcpModeFlag(manifestObjs []ctrlClient.Object) error { return nil } +var ErrPatchManifestType = errors.New("failed to cast manifest object to Unstructured") + func patchManifest(deployment *appsv1.Deployment, manifest ctrlClient.Object) error { manifestJSON, err := json.Marshal(deployment) if err != nil { return err } - err = json.Unmarshal(manifestJSON, &manifest.(*unstructured.Unstructured).Object) + unstrct, ok := manifest.(*unstructured.Unstructured) + if !ok { + return ErrPatchManifestType + } + + err = json.Unmarshal(manifestJSON, &unstrct.Object) if err != nil { return err } diff --git a/internal/deploy/bootstrap_test.go b/internal/deploy/bootstrap_test.go index afe113c3d..3a8914495 100644 --- a/internal/deploy/bootstrap_test.go +++ b/internal/deploy/bootstrap_test.go @@ -101,7 +101,7 @@ func TestPatchDeploymentWithInKcpModeFlag(t *testing.T) { t.Errorf("PatchDeploymentWithInKcpMode() error = %v, wantErr %v", err, tt.wantErr) } - hasKcpFlag := validateDeploymentHasKcpFlag(tt.args.manifestObjs) + hasKcpFlag := validateDeploymentHasKcpFlag(t, tt.args.manifestObjs) if !assert.Equal(t, tt.want, hasKcpFlag) { t.Errorf("PatchDeploymentWithInKcpModeFlag() got = %t, want %t", hasKcpFlag, tt.want) } @@ -109,10 +109,13 @@ func TestPatchDeploymentWithInKcpModeFlag(t *testing.T) { } } -func validateDeploymentHasKcpFlag(objs []ctrlClient.Object) bool { +func validateDeploymentHasKcpFlag(t *testing.T, objs []ctrlClient.Object) bool { for _, obj := range objs { if obj.GetObjectKind().GroupVersionKind().Kind == "Deployment" { - manifestJSON, _ := json.Marshal(obj.(*unstructured.Unstructured).Object) + unstr, ok := obj.(*unstructured.Unstructured) + assert.True(t, ok) + manifestJSON, err := json.Marshal(unstr.Object) + assert.NoError(t, err) deployment := &appsv1.Deployment{} if err := json.Unmarshal(manifestJSON, deployment); err == nil { if slices.Contains(deployment.Spec.Template.Spec.Containers[0].Args, "--in-kcp-mode") { diff --git a/internal/deploy/kyma.go b/internal/deploy/kyma.go index 35d5f0121..5621592c9 100644 --- a/internal/deploy/kyma.go +++ b/internal/deploy/kyma.go @@ -3,6 +3,7 @@ package deploy import ( "context" "fmt" + "github.com/kyma-project/cli/pkg/errs" "os" "github.com/avast/retry-go" @@ -65,8 +66,12 @@ func Kyma( if err := k8s.WatchObject( ctx, kyma, - func(kyma ctrlClient.Object) (bool, error) { - return string(kyma.(*v1beta2.Kyma).Status.State) == string(v1beta2.StateReady), nil + func(obj ctrlClient.Object) (bool, error) { + tKyma, ok := obj.(*v1beta2.Kyma) + if !ok { + return false, errs.ErrTypeAssertKyma + } + return string(tKyma.Status.State) == string(v1beta2.StateReady), nil }, ); err != nil { return fmt.Errorf("kyma custom resource did not get ready: %w", err) diff --git a/pkg/errs/common.go b/pkg/errs/common.go new file mode 100644 index 000000000..bf245121b --- /dev/null +++ b/pkg/errs/common.go @@ -0,0 +1,5 @@ +package errs + +import "errors" + +var ErrTypeAssertKyma = errors.New("not of type v1beta2.Kyma")