From c5c74a8dfc33974ca022c4887e2a96ebdf3414ad Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Fri, 22 Nov 2024 10:32:13 +0100 Subject: [PATCH 1/7] add runner name in error message --- database/sql/jobs.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/database/sql/jobs.go b/database/sql/jobs.go index 887e041f..424bf8f4 100644 --- a/database/sql/jobs.go +++ b/database/sql/jobs.go @@ -3,6 +3,7 @@ package sql import ( "context" "encoding/json" + "fmt" "log/slog" "github.com/google/uuid" @@ -84,7 +85,8 @@ func (s *sqlDatabase) paramsJobToWorkflowJob(ctx context.Context, job params.Job if job.RunnerName != "" { instance, err := s.getInstanceByName(s.ctx, job.RunnerName) if err != nil { - slog.With(slog.Any("error", err)).ErrorContext(ctx, "failed to get instance by name") + // This usually is very normal as not all jobs run on our runners. + slog.DebugContext(ctx, fmt.Sprintf("failed to get instance by name: %s", job.RunnerName)) } else { workflofJob.InstanceID = &instance.ID } @@ -244,7 +246,8 @@ func (s *sqlDatabase) CreateOrUpdateJob(ctx context.Context, job params.Job) (pa if err == nil { workflowJob.InstanceID = &instance.ID } else { - slog.With(slog.Any("error", err)).ErrorContext(ctx, "failed to get instance by name") + // This usually is very normal as not all jobs run on our runners. + slog.DebugContext(ctx, fmt.Sprintf("failed to get instance by name: %s", job.RunnerName)) } } From 8a31d81fafafca06ff4c32261cb4f336c24b6529 Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Fri, 22 Nov 2024 11:13:00 +0100 Subject: [PATCH 2/7] ignore workflow_jobs without labels --- runner/pool/pool.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 09383e34..83f9c172 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -135,6 +135,14 @@ func (r *basePoolManager) HandleWorkflowJob(job params.WorkflowJob) error { return errors.Wrap(err, "validating owner") } + // we see events where the lables seem to be missing. We should ignore these + // as we can't know if we should handle them or not. + if len(job.WorkflowJob.Labels) == 0 { + slog.WarnContext( + r.ctx, fmt.Sprintf("job has no labels: %s", job.WorkflowJob.Name)) + return nil + } + var jobParams params.Job var err error var triggeredBy int64 From 935c9dcd966c0ef1bbb83837e2200011ecb97592 Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Fri, 22 Nov 2024 16:40:23 +0100 Subject: [PATCH 3/7] Update database/sql/jobs.go Co-authored-by: Gabriel --- database/sql/jobs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/sql/jobs.go b/database/sql/jobs.go index 424bf8f4..d819d4c3 100644 --- a/database/sql/jobs.go +++ b/database/sql/jobs.go @@ -86,7 +86,7 @@ func (s *sqlDatabase) paramsJobToWorkflowJob(ctx context.Context, job params.Job instance, err := s.getInstanceByName(s.ctx, job.RunnerName) if err != nil { // This usually is very normal as not all jobs run on our runners. - slog.DebugContext(ctx, fmt.Sprintf("failed to get instance by name: %s", job.RunnerName)) + slog.DebugContext(ctx, "failed to get instance by name", "instance_name", job.RunnerName) } else { workflofJob.InstanceID = &instance.ID } From d6de59619d1ef36a7591e8055596347fd8f1e715 Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Fri, 22 Nov 2024 16:46:39 +0100 Subject: [PATCH 4/7] commit suggestion --- database/sql/jobs.go | 3 +-- runner/pool/pool.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/database/sql/jobs.go b/database/sql/jobs.go index d819d4c3..b7dda926 100644 --- a/database/sql/jobs.go +++ b/database/sql/jobs.go @@ -3,7 +3,6 @@ package sql import ( "context" "encoding/json" - "fmt" "log/slog" "github.com/google/uuid" @@ -247,7 +246,7 @@ func (s *sqlDatabase) CreateOrUpdateJob(ctx context.Context, job params.Job) (pa workflowJob.InstanceID = &instance.ID } else { // This usually is very normal as not all jobs run on our runners. - slog.DebugContext(ctx, fmt.Sprintf("failed to get instance by name: %s", job.RunnerName)) + slog.DebugContext(ctx, "failed to get instance by name", "instance_name", job.RunnerName) } } diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 83f9c172..8eca237a 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -138,8 +138,7 @@ func (r *basePoolManager) HandleWorkflowJob(job params.WorkflowJob) error { // we see events where the lables seem to be missing. We should ignore these // as we can't know if we should handle them or not. if len(job.WorkflowJob.Labels) == 0 { - slog.WarnContext( - r.ctx, fmt.Sprintf("job has no labels: %s", job.WorkflowJob.Name)) + slog.WarnContext(r.ctx, "job has no labels", "workflow_job", job.WorkflowJob.Name) return nil } From 386aba18c0eb0e9c443da3e707d442187db3d57f Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Tue, 26 Nov 2024 07:49:08 +0100 Subject: [PATCH 5/7] set specific linter version --- Makefile | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Makefile b/Makefile index 2a997f66..98c12805 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,17 @@ create-release-files: release: build-static create-release-files ## Create a release ##@ Lint / Verify +GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint + +## Tool Versions +GOLANGCI_LINT_VERSION ?= v1.61.0 + +.PHONY: golangci-lint +golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. If wrong version is installed, it will be overwritten. +$(GOLANGCI_LINT): $(LOCALBIN) + test -s $(LOCALBIN)/golangci-lint && $(LOCALBIN)/golangci-lint --version | grep -q $(GOLANGCI_LINT_VERSION) || \ + GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) + .PHONY: lint lint: golangci-lint $(GOLANGCI_LINT) ## Run linting. $(GOLANGCI_LINT) run -v --build-tags=testing,integration $(GOLANGCI_LINT_EXTRA_ARGS) From 3a95b8f7040cf96c8f54309561a54f18bf3a0a5b Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Tue, 26 Nov 2024 07:53:02 +0100 Subject: [PATCH 6/7] fix linter finding --- database/sql/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/sql/util.go b/database/sql/util.go index 063c68a6..2e065def 100644 --- a/database/sql/util.go +++ b/database/sql/util.go @@ -402,7 +402,7 @@ func (s *sqlDatabase) updatePool(tx *gorm.DB, pool Pool, param params.UpdatePool } tags := []Tag{} - if param.Tags != nil && len(param.Tags) > 0 { + if len(param.Tags) > 0 { for _, val := range param.Tags { t, err := s.getOrCreateTag(tx, val) if err != nil { From 6167d8c7fd5cd7e149e0f6427ce23ad07f916200 Mon Sep 17 00:00:00 2001 From: Michael Kuhnt Date: Tue, 26 Nov 2024 09:23:58 +0100 Subject: [PATCH 7/7] lint: exclude gosec G115 --- .golangci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index e9004a23..8dee07f5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -38,3 +38,7 @@ linters-settings: goimports: local-prefixes: github.com/cloudbase/garm + + gosec: + excludes: + - G115