Skip to content

Commit

Permalink
Upgrade Go to 1.23.4 and linter to 1.62.2 (#535)
Browse files Browse the repository at this point in the history
### What

* Upgrade to Go 1.23.4
* Remove Go version check from Makefile and make all default target
* Upgrade golangci-lint to 1.62.2 and fix issues

### Why

To make sure we are running the latest toolchain and that the code is
fixed accordingly. The changes to the Make target were done because Go
command is smart enough to download and install the version specified in
`go.mod` (assuming `GOOTOOLCHAIN=auto`) so the check is superfluous.
Moving it below `all` also makes it so that running `make` runs `make
all` again.

---------

Signed-off-by: Igor Suleymanov <[email protected]>
  • Loading branch information
radiohead authored Dec 10, 2024
1 parent fbb7327 commit 05e9e38
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 97 deletions.
105 changes: 53 additions & 52 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,49 @@
run:
tests: false
skip-dirs:

issues:
# Exclude specific dirs
exclude-dirs:
- test/
- internal/deepmap/
# Excluding specific linters per-path
exclude-rules:
- path: _test\.go
linters:
- gomnd
- gocognit

linters:
disable-all: true
enable:
- bodyclose
- dogsled
- dupl
- errcheck
- copyloopvar
- funlen
- gochecknoinits
- gocognit
- goconst
- gocritic
- gocyclo
- gofmt
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- ineffassign
- misspell
- nakedret
- prealloc
- revive
- staticcheck
- typecheck
- unconvert
- unparam
- unused
- whitespace

linters-settings:
funlen:
Expand Down Expand Up @@ -55,56 +96,16 @@ linters-settings:
disabled: true
gocognit:
min-complexity: 50
gomnd:
settings:
mnd:
# the list of enabled checks, see https://github.com/tommy-muehle/go-mnd/#checks for description.
checks:
- argument
- case
- condition
- operation
- return
ignored-numbers: 0,1
mnd:
# the list of enabled checks, see https://github.com/tommy-muehle/go-mnd/#checks for description.
checks:
- argument
- case
- condition
- operation
- return
ignored-numbers:
- '0'
- '1'
misspell:
locale: US

linters:
disable-all: true
enable:
- bodyclose
- dogsled
- dupl
- errcheck
- exportloopref
- funlen
- gochecknoinits
- gocognit
- goconst
- gocritic
- gocyclo
- gofmt
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- ineffassign
- misspell
- nakedret
- prealloc
- revive
- staticcheck
- typecheck
- unconvert
- unparam
- unused
- whitespace

issues:
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
- path: _test\.go
linters:
- gomnd
- gocognit
20 changes: 8 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,6 @@ GOBINARY := $(shell which go)

BIN_DIR := target

.PHONY: check-go-version
check-go-version:
@if [ -z "$(GOBINARY)" ]; then \
echo "Error: No Go binary found. It's a no-go!"; \
exit 1; \
fi; \
if [ "$$($(GOBINARY) version | awk '{print $$3}' | sed 's/go//')" != "$(GOVERSION)" ]; then \
echo "Error: Go version $(GOVERSION) is required, but version $$($(GOBINARY) version | awk '{print $$3}' | sed 's/go//') is installed."; \
exit 1; \
fi

all: check-go-version deps lint test build

deps: $(GOSUM) $(GOWORKSUM)
Expand All @@ -34,7 +23,14 @@ $(GOSUM): $(SOURCES) $(GOMOD)
$(GOWORKSUM): $(GOWORK) $(GOMOD)
go work sync

LINTER_VERSION := 1.60.3
.PHONY: check-go-version
check-go-version:
@if [ -z "$(GOBINARY)" ]; then \
echo "Error: No Go binary found. It's a no-go!"; \
exit 1; \
fi

LINTER_VERSION := 1.62.2
LINTER_BINARY := $(BIN_DIR)/golangci-lint-$(LINTER_VERSION)

.PHONY: lint
Expand Down
6 changes: 3 additions & 3 deletions codegen/jennies/resourceobject.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ func (r *ResourceObjectGenerator) generateObjectFile(kind codegen.Kind, version
return nil, err
}
for it.Next() {
if it.Label() == "spec" || it.Label() == "metadata" {
if it.Selector().String() == "spec" || it.Selector().String() == "metadata" {
continue
}
md.Subresources = append(md.Subresources, templates.SubresourceMetadata{
TypeName: typePrefix + exportField(it.Label()),
JSONName: it.Label(),
TypeName: typePrefix + exportField(it.Selector().String()),
JSONName: it.Selector().String(),
})
}
b := bytes.Buffer{}
Expand Down
6 changes: 3 additions & 3 deletions codegen/jennies/typescript.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ func (*TypeScriptResourceTypes) generateObjectFile(kind codegen.Kind, version *c
return nil, err
}
for it.Next() {
if it.Label() == "spec" || it.Label() == "metadata" {
if it.Selector().String() == "spec" || it.Selector().String() == "metadata" {
continue
}
metadata.Subresources = append(metadata.Subresources, templates.SubresourceMetadata{
TypeName: exportField(it.Label()),
JSONName: it.Label(),
TypeName: exportField(it.Selector().String()),
JSONName: it.Selector().String(),
})
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/grafana/grafana-app-sdk

go 1.23.0
go 1.23.4

retract (
v0.20.0 // Errors in release pipeline didn't allow the binaries to be built for this release, which can break automated workflows that depend on them
Expand Down
2 changes: 1 addition & 1 deletion go.work
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
go 1.23.0
go 1.23.4

use (
.
Expand Down
2 changes: 1 addition & 1 deletion operator/informer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Informer interface {
// ResourceWatcher describes an object which handles Add/Update/Delete actions for a resource
type ResourceWatcher interface {
Add(context.Context, resource.Object) error
Update(ctx context.Context, old, new resource.Object) error
Update(ctx context.Context, src, tgt resource.Object) error
Delete(context.Context, resource.Object) error
}

Expand Down
38 changes: 19 additions & 19 deletions operator/opinionatedwatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type PatchClient interface {
// OpinionatedWatcher contains unexported fields, and must be created with NewOpinionatedWatcher
type OpinionatedWatcher struct {
AddFunc func(ctx context.Context, object resource.Object) error
UpdateFunc func(ctx context.Context, old resource.Object, new resource.Object) error
UpdateFunc func(ctx context.Context, src resource.Object, tgt resource.Object) error
DeleteFunc func(ctx context.Context, object resource.Object) error
SyncFunc func(ctx context.Context, object resource.Object) error
finalizer string
Expand Down Expand Up @@ -187,40 +187,40 @@ func (o *OpinionatedWatcher) Add(ctx context.Context, object resource.Object) er
// If the new object has a non-nil ObjectMetadata.DeletionTimestamp in its metadata, DeleteFunc will be called,
// and the object's finalizer will be removed to allow kubernetes to hard delete it.
// Otherwise, UpdateFunc is called, provided the update is non-trivial (that is, the metadata.Generation has changed).
func (o *OpinionatedWatcher) Update(ctx context.Context, old resource.Object, new resource.Object) error {
func (o *OpinionatedWatcher) Update(ctx context.Context, src resource.Object, tgt resource.Object) error {
ctx, span := GetTracer().Start(ctx, "OpinionatedWatcher-update")
defer span.End()
// TODO: If old is nil, it _might_ be ok?
if old == nil {
if src == nil {
return fmt.Errorf("old cannot be nil")
}
if new == nil {
if tgt == nil {
return fmt.Errorf("new cannot be nil")
}

logger := logging.FromContext(ctx).With("action", "update", "component", "OpinionatedWatcher", "kind", new.GroupVersionKind().Kind, "namespace", new.GetNamespace(), "name", new.GetName())
logger := logging.FromContext(ctx).With("action", "update", "component", "OpinionatedWatcher", "kind", tgt.GroupVersionKind().Kind, "namespace", tgt.GetNamespace(), "name", tgt.GetName())
logger.Debug("Handling update")

// Only fire off Update if the generation has changed (so skip subresource updates)
if new.GetGeneration() > 0 && old.GetGeneration() == new.GetGeneration() {
if tgt.GetGeneration() > 0 && src.GetGeneration() == tgt.GetGeneration() {
return nil
}

// TODO: finalizers part of object metadata?
oldFinalizers := o.getFinalizers(old)
newFinalizers := o.getFinalizers(new)
if !slices.Contains(newFinalizers, o.finalizer) && new.GetDeletionTimestamp() == nil {
oldFinalizers := o.getFinalizers(src)
newFinalizers := o.getFinalizers(tgt)
if !slices.Contains(newFinalizers, o.finalizer) && tgt.GetDeletionTimestamp() == nil {
// Either the add somehow snuck past us (unlikely), or the original AddFunc call failed, and should be retried.
// Either way, we need to try calling AddFunc
logger.Debug("Missing finalizer, calling Add")
err := o.addFunc(ctx, new)
err := o.addFunc(ctx, tgt)
if err != nil {
span.SetStatus(codes.Error, fmt.Sprintf("watcher add error: %s", err.Error()))
return err
}
// Add the finalizer (which also updates `new` inline)
logger.Debug("Successful call to Add, add the finalizer to the object", "finalizer", o.finalizer)
err = o.addFinalizer(ctx, new, newFinalizers)
err = o.addFinalizer(ctx, tgt, newFinalizers)
if err != nil {
span.SetStatus(codes.Error, fmt.Sprintf("watcher add finalizer error: %s", err.Error()))
return fmt.Errorf("error adding finalizer: %w", err)
Expand All @@ -229,24 +229,24 @@ func (o *OpinionatedWatcher) Update(ctx context.Context, old resource.Object, ne

// Check if the deletion timestamp is non-nil.
// This denotes that the resource was deletes, but has one or more finalizers blocking it from actually deleting.
if new.GetDeletionTimestamp() != nil {
if tgt.GetDeletionTimestamp() != nil {
// If our finalizer is in the list, treat this as a delete.
// Otherwise, drop the event and don't handle it as an update.
if !slices.Contains(newFinalizers, o.finalizer) {
logger.Debug("Update has a DeletionTimestamp, but missing our finalizer, ignoring", "deletionTimestamp", new.GetDeletionTimestamp())
logger.Debug("Update has a DeletionTimestamp, but missing our finalizer, ignoring", "deletionTimestamp", tgt.GetDeletionTimestamp())
return nil
}

// Call the delete handler, then remove the finalizer on success
logger.Debug("Update has a DeletionTimestamp, calling Delete", "deletionTimestamp", new.GetDeletionTimestamp())
err := o.deleteFunc(ctx, new)
logger.Debug("Update has a DeletionTimestamp, calling Delete", "deletionTimestamp", tgt.GetDeletionTimestamp())
err := o.deleteFunc(ctx, tgt)
if err != nil {
span.SetStatus(codes.Error, fmt.Sprintf("watcher delete error: %s", err.Error()))
return err
}

logger.Debug("Delete successful, removing finalizer", "finalizer", o.finalizer, "currentFinalizers", newFinalizers)
err = o.removeFinalizer(ctx, new, newFinalizers)
err = o.removeFinalizer(ctx, tgt, newFinalizers)
if err != nil {
span.SetStatus(codes.Error, fmt.Sprintf("watcher remove finalizer error: %s", err.Error()))
return err
Expand All @@ -260,7 +260,7 @@ func (o *OpinionatedWatcher) Update(ctx context.Context, old resource.Object, ne
return nil
}

err := o.updateFunc(ctx, old, new)
err := o.updateFunc(ctx, src, tgt)
if err != nil {
span.SetStatus(codes.Error, fmt.Sprintf("watcher update error: %s", err.Error()))
return err
Expand Down Expand Up @@ -289,9 +289,9 @@ func (o *OpinionatedWatcher) addFunc(ctx context.Context, object resource.Object
}

// updateFunc is a wrapper for UpdateFunc which makes a nil check to avoid panics
func (o *OpinionatedWatcher) updateFunc(ctx context.Context, old, new resource.Object) error {
func (o *OpinionatedWatcher) updateFunc(ctx context.Context, src, tgt resource.Object) error {
if o.UpdateFunc != nil {
return o.UpdateFunc(ctx, old, new)
return o.UpdateFunc(ctx, src, tgt)
}
// TODO: log?
return nil
Expand Down
4 changes: 2 additions & 2 deletions operator/simplewatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ func (w *SimpleWatcher) Add(ctx context.Context, object resource.Object) error {
}

// Update calls UpdateFunc, if non-nil
func (w *SimpleWatcher) Update(ctx context.Context, old resource.Object, new resource.Object) error {
func (w *SimpleWatcher) Update(ctx context.Context, src resource.Object, tgt resource.Object) error {
if w.UpdateFunc != nil {
return w.UpdateFunc(ctx, old, new)
return w.UpdateFunc(ctx, src, tgt)
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/grafana/grafana-app-sdk/plugin

go 1.23.0
go 1.23.4

retract (
v0.18.4 // Errors in release pipeline didn't allow the binaries to be built for this release, which can break automated workflows that depend on them
Expand Down
4 changes: 2 additions & 2 deletions simple/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ func (w *Watcher) Add(ctx context.Context, object resource.Object) error {
}

// Update calls UpdateFunc, if non-nil
func (w *Watcher) Update(ctx context.Context, old resource.Object, new resource.Object) error {
func (w *Watcher) Update(ctx context.Context, src resource.Object, tgt resource.Object) error {
if w.UpdateFunc != nil {
return w.UpdateFunc(ctx, old, new)
return w.UpdateFunc(ctx, src, tgt)
}
return nil
}
Expand Down

0 comments on commit 05e9e38

Please sign in to comment.