From 9bbba4441de564b7c249821db3543d42afef2414 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Fri, 12 Jan 2024 02:01:02 +0100 Subject: [PATCH] Enable golangci linter forcetypeassert (#3168) Split out from https://github.com/woodpecker-ci/woodpecker/pull/2960 --- .golangci.yaml | 1 + pipeline/backend/kubernetes/kubernetes.go | 14 ++++++++++-- pipeline/backend/local/local.go | 8 ++++++- .../frontend/yaml/constraint/constraint.go | 6 ++++- pipeline/frontend/yaml/types/network.go | 18 ++++++++++++--- server/forge/bitbucket/bitbucket_test.go | 10 +++++---- server/forge/gitea/gitea_test.go | 6 +++-- server/forge/github/github.go | 3 ++- server/forge/github/github_test.go | 11 +++++----- server/queue/fifo.go | 16 ++++++++------ server/queue/fifo_test.go | 22 +++++++++---------- server/router/middleware/session/agent.go | 2 +- server/store/context.go | 3 ++- 13 files changed, 81 insertions(+), 39 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index f8c8a9522..693030583 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -164,6 +164,7 @@ linters: - nolintlint - stylecheck - contextcheck + - forcetypeassert run: timeout: 15m diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 1a3cd524f..152147074 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -216,7 +216,12 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) finished := make(chan bool) podUpdated := func(old, new any) { - pod := new.(*v1.Pod) + pod, ok := new.(*v1.Pod) + if !ok { + log.Error().Msgf("could not parse pod: %v", new) + return + } + if pod.Name == podName { if isImagePullBackOffState(pod) { finished <- true @@ -276,7 +281,12 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) up := make(chan bool) podUpdated := func(old, new any) { - pod := new.(*v1.Pod) + pod, ok := new.(*v1.Pod) + if !ok { + log.Error().Msgf("could not parse pod: %v", new) + return + } + if pod.Name == podName { switch pod.Status.Phase { case v1.PodRunning, v1.PodSucceeded, v1.PodFailed: diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index da4b99a68..a4bda9ecf 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -257,7 +257,13 @@ func (e *local) getState(taskUUID string) (*workflowState, error) { if !ok { return nil, ErrWorkflowStateNotFound } - return state.(*workflowState), nil + + s, ok := state.(*workflowState) + if !ok { + return nil, fmt.Errorf("could not parse state: %v", state) + } + + return s, nil } func (e *local) saveState(taskUUID string, state *workflowState) { diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go index 2e8fe5eeb..2f7f8cdc0 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/pipeline/frontend/yaml/constraint/constraint.go @@ -195,7 +195,11 @@ func (c *Constraint) Match(m metadata.Metadata, global bool, env map[string]stri if err != nil { return false, err } - match = match && result.(bool) + bresult, ok := result.(bool) + if !ok { + return false, fmt.Errorf("could not parse result: %v", result) + } + match = match && bresult } return match, nil diff --git a/pipeline/frontend/yaml/types/network.go b/pipeline/frontend/yaml/types/network.go index d56cdf311..f923283b6 100644 --- a/pipeline/frontend/yaml/types/network.go +++ b/pipeline/frontend/yaml/types/network.go @@ -90,6 +90,8 @@ func handleNetwork(name string, value any) (*Network, error) { network := &Network{ Name: name, } + + var ok bool for mapKey, mapValue := range v { switch mapKey { case "aliases": @@ -99,12 +101,22 @@ func handleNetwork(name string, value any) (*Network, error) { } network.Aliases = []string{} for _, alias := range aliases { - network.Aliases = append(network.Aliases, alias.(string)) + a, ok := alias.(string) + if !ok { + return &Network{}, fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", aliases, aliases) + } + network.Aliases = append(network.Aliases, a) } case "ipv4_address": - network.IPv4Address = mapValue.(string) + network.IPv4Address, ok = mapValue.(string) + if !ok { + return &Network{}, fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", network, network) + } case "ipv6_address": - network.IPv6Address = mapValue.(string) + network.IPv6Address, ok = mapValue.(string) + if !ok { + return &Network{}, fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", network, network) + } default: // Ignorer unknown keys ? continue diff --git a/server/forge/bitbucket/bitbucket_test.go b/server/forge/bitbucket/bitbucket_test.go index ee118d156..2afde4789 100644 --- a/server/forge/bitbucket/bitbucket_test.go +++ b/server/forge/bitbucket/bitbucket_test.go @@ -45,10 +45,12 @@ func Test_bitbucket(t *testing.T) { g.It("Should return client with default endpoint", func() { forge, _ := New(&Opts{Client: "4vyW6b49Z", Secret: "a5012f6c6"}) - g.Assert(forge.(*config).url).Equal(DefaultURL) - g.Assert(forge.(*config).API).Equal(DefaultAPI) - g.Assert(forge.(*config).Client).Equal("4vyW6b49Z") - g.Assert(forge.(*config).Secret).Equal("a5012f6c6") + + f, _ := forge.(*config) + g.Assert(f.url).Equal(DefaultURL) + g.Assert(f.API).Equal(DefaultAPI) + g.Assert(f.Client).Equal("4vyW6b49Z") + g.Assert(f.Secret).Equal("a5012f6c6") }) g.It("Should return the netrc file", func() { diff --git a/server/forge/gitea/gitea_test.go b/server/forge/gitea/gitea_test.go index 384aea57c..167662f21 100644 --- a/server/forge/gitea/gitea_test.go +++ b/server/forge/gitea/gitea_test.go @@ -57,8 +57,10 @@ func Test_gitea(t *testing.T) { URL: "http://localhost:8080", SkipVerify: true, }) - g.Assert(forge.(*Gitea).url).Equal("http://localhost:8080") - g.Assert(forge.(*Gitea).SkipVerify).Equal(true) + + f, _ := forge.(*Gitea) + g.Assert(f.url).Equal("http://localhost:8080") + g.Assert(f.SkipVerify).Equal(true) }) g.It("Should handle malformed url", func() { _, err := New(Opts{URL: "%gh&%ij"}) diff --git a/server/forge/github/github.go b/server/forge/github/github.go index 768a398ab..c9c16a58b 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -434,7 +434,8 @@ func (c *client) newClientToken(ctx context.Context, token string) *github.Clien ) tc := oauth2.NewClient(ctx, ts) if c.SkipVerify { - tc.Transport.(*oauth2.Transport).Base = &http.Transport{ + tp, _ := tc.Transport.(*oauth2.Transport) + tp.Base = &http.Transport{ Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, diff --git a/server/forge/github/github_test.go b/server/forge/github/github_test.go index 219d42c92..74f338dd8 100644 --- a/server/forge/github/github_test.go +++ b/server/forge/github/github_test.go @@ -51,11 +51,12 @@ func Test_github(t *testing.T) { Secret: "I1NiIsInR5", SkipVerify: true, }) - g.Assert(forge.(*client).url).Equal("http://localhost:8080") - g.Assert(forge.(*client).API).Equal("http://localhost:8080/api/v3/") - g.Assert(forge.(*client).Client).Equal("0ZXh0IjoiI") - g.Assert(forge.(*client).Secret).Equal("I1NiIsInR5") - g.Assert(forge.(*client).SkipVerify).Equal(true) + f, _ := forge.(*client) + g.Assert(f.url).Equal("http://localhost:8080") + g.Assert(f.API).Equal("http://localhost:8080/api/v3/") + g.Assert(f.Client).Equal("0ZXh0IjoiI") + g.Assert(f.Secret).Equal("I1NiIsInR5") + g.Assert(f.SkipVerify).Equal(true) }) }) diff --git a/server/queue/fifo.go b/server/queue/fifo.go index 086f1644f..0111696fa 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -203,10 +203,12 @@ func (q *fifo) Info(_ context.Context) InfoT { stats.Stats.Complete = 0 // TODO: implement this for e := q.pending.Front(); e != nil; e = e.Next() { - stats.Pending = append(stats.Pending, e.Value.(*model.Task)) + task, _ := e.Value.(*model.Task) + stats.Pending = append(stats.Pending, task) } for e := q.waitingOnDeps.Front(); e != nil; e = e.Next() { - stats.WaitingOnDeps = append(stats.WaitingOnDeps, e.Value.(*model.Task)) + task, _ := e.Value.(*model.Task) + stats.WaitingOnDeps = append(stats.WaitingOnDeps, task) } for _, entry := range q.running { stats.Running = append(stats.Running, entry.item) @@ -243,7 +245,7 @@ func (q *fifo) process() { q.resubmitExpiredPipelines() q.filterWaiting() for pending, worker := q.assignToWorker(); pending != nil && worker != nil; pending, worker = q.assignToWorker() { - task := pending.Value.(*model.Task) + task, _ := pending.Value.(*model.Task) task.AgentID = worker.agentID delete(q.workers, worker) q.pending.Remove(pending) @@ -261,7 +263,7 @@ func (q *fifo) filterWaiting() { var nextWaiting *list.Element for e := q.waitingOnDeps.Front(); e != nil; e = nextWaiting { nextWaiting = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) q.pending.PushBack(task) } @@ -271,7 +273,7 @@ func (q *fifo) filterWaiting() { var nextPending *list.Element for e := q.pending.Front(); e != nil; e = nextPending { nextPending = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) if q.depsInQueue(task) { log.Debug().Msgf("queue: waiting due to unmet dependencies %v", task.ID) q.waitingOnDeps.PushBack(task) @@ -289,7 +291,7 @@ func (q *fifo) assignToWorker() (*list.Element, *worker) { var next *list.Element for e := q.pending.Front(); e != nil; e = next { next = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) log.Debug().Msgf("queue: trying to assign task: %v with deps %v", task.ID, task.Dependencies) for w := range q.workers { @@ -372,7 +374,7 @@ func (q *fifo) removeFromPending(taskID string) { var next *list.Element for e := q.pending.Front(); e != nil; e = next { next = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) if task.ID == taskID { log.Debug().Msgf("queue: %s is removed from pending", taskID) q.pending.Remove(e) diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index ddc77dba3..49d191e3a 100644 --- a/server/queue/fifo_test.go +++ b/server/queue/fifo_test.go @@ -70,7 +70,7 @@ func TestFifo(t *testing.T) { func TestFifoExpire(t *testing.T) { want := &model.Task{ID: "1"} - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) q.extension = 0 assert.NoError(t, q.Push(noContext, want)) info := q.Info(noContext) @@ -95,7 +95,7 @@ func TestFifoExpire(t *testing.T) { func TestFifoWait(t *testing.T) { want := &model.Task{ID: "1"} - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.Push(noContext, want)) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -148,7 +148,7 @@ func TestFifoDependencies(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -184,7 +184,7 @@ func TestFifoErrors(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -233,7 +233,7 @@ func TestFifoErrors2(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) for i := 0; i < 2; i++ { @@ -280,7 +280,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) obtainedWorkCh := make(chan *model.Task) @@ -371,7 +371,7 @@ func TestFifoTransitiveErrors(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -421,7 +421,7 @@ func TestFifoCancel(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) _, _ = q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -441,7 +441,7 @@ func TestFifoPause(t *testing.T) { ID: "1", } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) var wg sync.WaitGroup wg.Add(1) go func() { @@ -473,7 +473,7 @@ func TestFifoPauseResume(t *testing.T) { ID: "1", } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) q.Pause() assert.NoError(t, q.Push(noContext, task1)) q.Resume() @@ -499,7 +499,7 @@ func TestWaitingVsPending(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) diff --git a/server/router/middleware/session/agent.go b/server/router/middleware/session/agent.go index 52d5f5f17..7f873e03f 100644 --- a/server/router/middleware/session/agent.go +++ b/server/router/middleware/session/agent.go @@ -23,7 +23,7 @@ import ( // AuthorizeAgent authorizes requests from agent to access the queue. func AuthorizeAgent(c *gin.Context) { - secret := c.MustGet("agent").(string) + secret, _ := c.MustGet("agent").(string) if secret == "" { c.String(401, "invalid or empty token.") return diff --git a/server/store/context.go b/server/store/context.go index fc94c53a4..7c3fd23fe 100644 --- a/server/store/context.go +++ b/server/store/context.go @@ -27,7 +27,8 @@ type Setter interface { // FromContext returns the Store associated with this context. func FromContext(c context.Context) Store { - return c.Value(key).(Store) + store, _ := c.Value(key).(Store) + return store } // TryFromContext try to return the Store associated with this context.