Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update go linting + update dependencies for controller #137

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions workspaces/backend/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,17 @@
/bin
cover.out
# Binaries for programs and plugins
*.exe
*.exe~
*.dll
*.so
*.dylib
bin/*
Dockerfile.cross

# Test binary, built with `go test -c`
*.test

# Output of the go coverage tool, specifically when used with LiteIDE
*.out

# Go workspace file
go.work
75 changes: 70 additions & 5 deletions workspaces/backend/.golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,103 @@ run:
allow-parallel-runners: true

issues:
# don't skip warning about doc comments
# don't exclude the default set of lint
# disable the default set of exclude rules
exclude-use-default: false
# restore some of the defaults
# (fill in the rest as needed)

exclude-rules:
# disable some linters for api packages
- path: "api/*"
linters:
- lll

# disable some linters for internal packages
- path: "internal/*"
linters:
- dupl
- lll

# exclude some linters for the test directory and test files
- path: test/.*|.*_test\.go
linters:
- dupl
- errcheck
- goconst
- gocyclo
- gosec

linters:
disable-all: true
enable:
- asciicheck
- bodyclose
- copyloopvar
- dogsled
- dupl
- errcheck
- exportloopref
- errorlint
- exhaustive
- ginkgolinter
- goconst
- gocritic
- gocyclo
- gofmt
- goheader
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- ineffassign
- lll
- misspell
- nakedret
- nolintlint
- prealloc
- revive
- staticcheck
- typecheck
- unconvert
- unparam
- unused

linters-settings:
gocritic:
enabled-tags:
- diagnostic
- experimental
- opinionated
- performance
- style
disabled-checks:
- assignOp
- filepathJoin
- paramTypeCombine
- rangeValCopy
- unnamedResult
- whyNoLint

goimports:
local-prefixes: github.com/kubeflow/notebooks/workspaces/backend

goheader:
template: |-
Copyright 2024.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

misspell:
locale: US

revive:
rules:
- name: comment-spacings
2 changes: 1 addition & 1 deletion workspaces/backend/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Use the golang image to build the application
FROM golang:1.22.2 AS builder
FROM golang:1.22 AS builder
ARG TARGETOS
ARG TARGETARCH

Expand Down
80 changes: 56 additions & 24 deletions workspaces/backend/Makefile
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
# CONTAINER_TOOL defines the container tool to be used for building images.
# Be aware that the target commands are only tested with Docker which is
# scaffolded by default. However, you might want to replace it to use other
# tools. (i.e. podman)
CONTAINER_TOOL ?= docker
# Image URL to use all building/pushing image targets
IMG ?= nbv2-backend:latest
# Backend default port
PORT ?= 4000
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
ENVTEST_K8S_VERSION = 1.29.0
ENVTEST_K8S_VERSION = 1.31.0

# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
ifeq (,$(shell go env GOBIN))
Expand All @@ -17,6 +10,15 @@ else
GOBIN=$(shell go env GOBIN)
endif

# Backend default port
PORT ?= 4000

# CONTAINER_TOOL defines the container tool to be used for building images.
# Be aware that the target commands are only tested with Docker which is
# scaffolded by default. However, you might want to replace it to use other
# tools. (i.e. podman)
CONTAINER_TOOL ?= docker

# Setting SHELL to bash allows bash commands to be executed by recipes.
# Options are set to exit when a recipe line exits non-zero or a piped command fails.
SHELL = /usr/bin/env bash -o pipefail
Expand All @@ -27,6 +29,17 @@ all: build

##@ General

# The help target prints out all targets with their descriptions organized
# beneath their categories. The categories are represented by '##@' and the
# target descriptions by '##'. The awk command is responsible for reading the
# entire set of makefiles included in this invocation, looking for lines of the
# file as xyz: ## something, and then pretty-format the target and help. Then,
# if there's a line with ##@ something, that gets pretty-printed as a category.
# More info on the usage of ANSI control characters for terminal formatting:
# https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_parameters
# More info on the awk command:
# http://linuxcommand.org/lc3_adv_awk.php

.PHONY: help
help: ## Display this help.
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
Expand All @@ -43,11 +56,10 @@ vet: ## Run go vet against code.

.PHONY: test
test: fmt vet envtest ## Run tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \
go test ./... -coverprofile cover.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out

.PHONY: lint
lint: golangci-lint ## Run golangci-lint linter & yamllint
lint: golangci-lint ## Run golangci-lint linter
$(GOLANGCI_LINT) run

.PHONY: lint-fix
Expand All @@ -57,13 +69,16 @@ lint-fix: golangci-lint ## Run golangci-lint linter and perform fixes
##@ Build

.PHONY: build
build: fmt vet envtest ## Build backend binary.
build: fmt vet ## Build backend binary.
go build -o bin/backend cmd/main.go

.PHONY: run
run: fmt vet ## Run a backend from your host.
go run ./cmd/main.go --port=$(PORT)
go run ./cmd/main.go --port=$(PORT)

# If you wish to build the manager image targeting other platforms you can use the --platform flag.
# (i.e. docker build --platform linux/arm64). However, you must enable docker buildKit for it.
# More info: https://docs.docker.com/develop/develop-images/build_enhancements/
.PHONY: docker-build
docker-build: ## Build docker image with the backend.
$(CONTAINER_TOOL) build -t ${IMG} .
Expand All @@ -72,6 +87,23 @@ docker-build: ## Build docker image with the backend.
docker-push: ## Push docker image with the backend.
$(CONTAINER_TOOL) push ${IMG}

# PLATFORMS defines the target platforms for the manager image be built to provide support to multiple
# architectures. (i.e. make docker-buildx IMG=myregistry/mypoperator:0.0.1). To use this option you need to:
# - be able to use docker buildx. More info: https://docs.docker.com/build/buildx/
# - have enabled BuildKit. More info: https://docs.docker.com/develop/develop-images/build_enhancements/
# - be able to push the image to your registry (i.e. if you do not set a valid value via IMG=<myregistry/image:<tag>> then the export will fail)
# To adequately provide solutions that are compatible with multiple platforms, you should consider using this option.
PLATFORMS ?= linux/arm64,linux/amd64,linux/s390x,linux/ppc64le
.PHONY: docker-buildx
docker-buildx: ## Build and push docker image for the manager for cross-platform support
# copy existing Dockerfile and insert --platform=${BUILDPLATFORM} into Dockerfile.cross, and preserve the original Dockerfile
sed -e '1 s/\(^FROM\)/FROM --platform=\$$\{BUILDPLATFORM\}/; t' -e ' 1,// s//FROM --platform=\$$\{BUILDPLATFORM\}/' Dockerfile > Dockerfile.cross
- $(CONTAINER_TOOL) buildx create --name project-v3-builder
$(CONTAINER_TOOL) buildx use project-v3-builder
- $(CONTAINER_TOOL) buildx build --push --platform=$(PLATFORMS) --tag ${IMG} -f Dockerfile.cross .
- $(CONTAINER_TOOL) buildx rm project-v3-builder
rm Dockerfile.cross

##@ Dependencies

## Location to install dependencies to
Expand All @@ -81,13 +113,12 @@ $(LOCALBIN):

## Tool Binaries
KUBECTL ?= kubectl
ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION)
GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
ENVTEST ?= $(LOCALBIN)/setup-envtest
GOLANGCI_LINT = $(LOCALBIN)/golangci-lint

## Tool Versions
KUSTOMIZE_VERSION ?= v5.3.0
ENVTEST_VERSION ?= release-0.17
GOLANGCI_LINT_VERSION ?= v1.57.2
ENVTEST_VERSION ?= release-0.19
GOLANGCI_LINT_VERSION ?= v1.61.0

.PHONY: envtest
envtest: $(ENVTEST) ## Download setup-envtest locally if necessary.
Expand All @@ -97,19 +128,20 @@ $(ENVTEST): $(LOCALBIN)
.PHONY: golangci-lint
golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
$(GOLANGCI_LINT): $(LOCALBIN)
$(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,${GOLANGCI_LINT_VERSION})

$(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION))

# go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist
# $1 - target path with name of binary (ideally with version)
# $1 - target path with name of binary
# $2 - package url which can be installed
# $3 - specific version of package
define go-install-tool
@[ -f $(1) ] || { \
@[ -f "$(1)-$(3)" ] || { \
set -e; \
package=$(2)@$(3) ;\
echo "Downloading $${package}" ;\
rm -f $(1) || true ;\
GOBIN=$(LOCALBIN) go install $${package} ;\
mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\
}
mv $(1) $(1)-$(3) ;\
} ;\
ln -sf $(1)-$(3) $(1)
endef
6 changes: 3 additions & 3 deletions workspaces/backend/api/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const (
WorkspaceKindNamePathParam = "name"
WorkspaceKindsByNamePath = AllWorkspaceKindsPath + "/:" + WorkspaceNamePathParam

//namespaces
// namespaces
AllNamespacesPath = PathPrefix + "/namespaces"
)

Expand All @@ -59,12 +59,12 @@ type App struct {
}

// NewApp creates a new instance of the app
func NewApp(cfg config.EnvConfig, logger *slog.Logger, client client.Client, scheme *runtime.Scheme) (*App, error) {
func NewApp(cfg config.EnvConfig, logger *slog.Logger, cl client.Client, scheme *runtime.Scheme) (*App, error) {

app := &App{
Config: cfg,
logger: logger,
repositories: repositories.NewRepositories(client),
repositories: repositories.NewRepositories(cl),
Scheme: scheme,
}
return app, nil
Expand Down
12 changes: 6 additions & 6 deletions workspaces/backend/api/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (a *App) LogError(r *http.Request, err error) {
a.logger.Error(err.Error(), "method", method, "uri", uri)
}

// nolint:unused
//nolint:unused
func (a *App) badRequestResponse(w http.ResponseWriter, r *http.Request, err error) {
httpError := &HTTPError{
StatusCode: http.StatusBadRequest,
Expand All @@ -58,13 +58,13 @@ func (a *App) badRequestResponse(w http.ResponseWriter, r *http.Request, err err
a.errorResponse(w, r, httpError)
}

func (a *App) errorResponse(w http.ResponseWriter, r *http.Request, error *HTTPError) {
env := ErrorEnvelope{Error: error}
func (a *App) errorResponse(w http.ResponseWriter, r *http.Request, httpError *HTTPError) {
env := ErrorEnvelope{Error: httpError}

err := a.WriteJSON(w, error.StatusCode, env, nil)
err := a.WriteJSON(w, httpError.StatusCode, env, nil)
if err != nil {
a.LogError(r, err)
w.WriteHeader(error.StatusCode)
w.WriteHeader(httpError.StatusCode)
}
}

Expand Down Expand Up @@ -103,7 +103,7 @@ func (a *App) methodNotAllowedResponse(w http.ResponseWriter, r *http.Request) {
a.errorResponse(w, r, httpError)
}

// nolint:unused
//nolint:unused
func (a *App) failedValidationResponse(w http.ResponseWriter, r *http.Request, errors map[string]string) {
message, err := json.Marshal(errors)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions workspaces/backend/api/healthcheck__handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ func TestHealthCheckHandler(t *testing.T) {
}

rr := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, HealthCheckPath, nil)
req, err := http.NewRequest(http.MethodGet, HealthCheckPath, http.NoBody)
if err != nil {
t.Fatal(err)
}

app.HealthcheckHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close() // nolint: errcheck
defer rs.Body.Close()

body, err := io.ReadAll(rs.Body)
if err != nil {
Expand Down
Loading