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

Enhanced Linting #813

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
202 changes: 202 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
linters:
disable-all: true
enable:
- asasalint
- asciicheck
- bodyclose
- containedctx
- copyloopvar
- decorder
- dogsled
- errcheck
- errchkjson
- execinquery
# - gci
- goconst
# - gocritic
- gocyclo
- godot
- gofmt
- goimports
- goprintffuncname
# - gosec
- gosimple
- govet
# - importas
- ineffassign
- misspell
- nakedret
- nilerr
- noctx
- nolintlint
- nosprintfhostport
- prealloc
- predeclared
- reassign
# - revive
- rowserrcheck
- staticcheck
# - stylecheck
# - thelper
- typecheck
- unconvert
- unparam
- unused
- usestdlibvars
- whitespace

linters-settings:
gocyclo:
min-complexity: 20
godot:
# declarations - for top level declaration comments (default);
# toplevel - for top level comments;
# all - for all comments.
scope: toplevel
exclude:
- '^ \+.*'
- '^ ANCHOR.*'
gci:
sections:
- standard
- default
- prefix(github.com/IBM)
- prefix(k8s.io)
- prefix(sigs.k8s.io)
- blank
- dot
importas:
no-unaliased: true
alias:
# Kubernetes
- pkg: k8s.io/api/core/v1
alias: corev1
- pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
alias: apiextensionsv1
- pkg: k8s.io/apimachinery/pkg/apis/meta/v1
alias: metav1
- pkg: k8s.io/apimachinery/pkg/api/errors
alias: apierrors
- pkg: k8s.io/apimachinery/pkg/util/errors
alias: kerrors
# Controller Runtime
- pkg: sigs.k8s.io/controller-runtime
alias: ctrl
nolintlint:
allow-unused: false
# allow-leading-space: false
require-specific: true
gosec:
excludes:
- G307 # Deferring unsafe method "Close" on type "\*os.File"
- G108 # Profiling endpoint is automatically exposed on /debug/pprof
gocritic:
enabled-tags:
- experimental
disabled-checks:
- appendAssign
- dupImport # https://github.com/go-critic/go-critic/issues/845
- evalOrder
- ifElseChain
- octalLiteral
- regexpSimplify
- sloppyReassign
- truncateCmp
- typeDefFirst
- unnamedResult
- unnecessaryDefer
- whyNoLint
- wrapperFunc
unused:
go: "1.22"
issues:
exclude-files:
- "zz_generated.*\\.go$"
max-same-issues: 0
max-issues-per-linter: 0
# We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
# changes in PRs and avoid nitpicking.
exclude-use-default: false
exclude-rules:
- linters:
- gci
path: _test\.go
- linters:
- revive
text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
- linters:
- errcheck
text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
# Exclude some packages or code to require comments, for example test code, or fake clients.
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
source: (func|type).*Fake.*
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
path: fake_\.go
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
path: "(framework|e2e)/.*.go"
# Disable unparam "always receives" which might not be really
# useful when building libraries.
- linters:
- unparam
text: always receives
# Dot imports for gomega or ginkgo are allowed
# within test files.
- path: _test\.go
text: should not use dot imports
- path: (framework|e2e)/.*.go
text: should not use dot imports
- path: _test\.go
text: cyclomatic complexity
# Append should be able to assign to a different var/slice.
- linters:
- gocritic
text: "appendAssign: append result not assigned to the same slice"
# Disable linters for conversion
- linters:
- staticcheck
text: "SA1019: in.(.+) is deprecated"
path: .*(api|types)\/.*\/.*conversion.*\.go$
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
path: .*(api|types)\/.*\/.*conversion.*\.go$
- linters:
- revive
text: "var-naming: don't use underscores in Go names;"
path: .*(api|types)\/.*\/.*conversion.*\.go$
- linters:
- revive
text: "receiver-naming: receiver name"
path: .*(api|types)\/.*\/.*conversion.*\.go$
- linters:
- stylecheck
text: "ST1003: should not use underscores in Go names;"
path: .*(api|types)\/.*\/.*conversion.*\.go$
- linters:
- stylecheck
text: "ST1016: methods on the same type should have the same receiver name"
path: .*(api|types)\/.*\/.*conversion.*\.go$
# hack/tools
- linters:
- typecheck
text: import (".+") is a program, not an importable package
path: ^tools\.go$
# We don't care about defer in for loops in test files.
- linters:
- gocritic
text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
path: _test\.go

run:
timeout: 10m
build-tags:
- tools
- e2e
allow-parallel-runners: true
go: "1.22"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Let's add a new line at the end of the file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this point to 1.23 as what is used in the project?
Maintaining go version strings at multiple places during upgrades is just a concern.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the go version.

4 changes: 2 additions & 2 deletions adhoc-controllers/controllers/nodeupdate_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog/v2"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/cloud"
)

// NodeUpdateReconciler reconciles a NodeUpdate object
// NodeUpdateReconciler reconciles a NodeUpdate object.
type NodeUpdateReconciler struct {
Client client.Client
Scheme *runtime.Scheme
Expand All @@ -41,7 +42,6 @@ type NodeUpdateReconciler struct {
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
func (r *NodeUpdateReconciler) Reconcile(_ context.Context, req ctrl.Request) (ctrl.Result, error) {

// Fetch the Node instance
node := corev1.Node{}
err := r.Client.Get(context.Background(), req.NamespacedName, &node)
Expand Down
11 changes: 6 additions & 5 deletions adhoc-controllers/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,22 @@ import (
"flag"
"os"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
_ "k8s.io/client-go/plugin/pkg/client/auth"

"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"sigs.k8s.io/ibm-powervs-block-csi-driver/adhoc-controllers/controllers"

//+kubebuilder:scaffold:imports

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
_ "k8s.io/client-go/plugin/pkg/client/auth"
)

var (
Expand Down
6 changes: 3 additions & 3 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ limitations under the License.

package main

//DONE
// DONE.

import (
"flag"

"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver"

"k8s.io/klog/v2"

"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver"
)

func main() {
Expand Down
16 changes: 8 additions & 8 deletions cmd/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ limitations under the License.

package main

//DONE
// DONE.

import (
"flag"
"os"
"strings"

"k8s.io/klog/v2"

"sigs.k8s.io/ibm-powervs-block-csi-driver/cmd/options"
"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver"

"k8s.io/klog/v2"
)

// Options is the combined set of options for all operating modes.
Expand All @@ -38,7 +38,7 @@ type Options struct {
*options.NodeOptions
}

// used for testing
// used for testing.
var osExit = os.Exit

// GetOptions parses the command line options and returns a struct that contains
Expand All @@ -63,7 +63,7 @@ func GetOptions(fs *flag.FlagSet) *Options {

switch {
case cmd == string(driver.ControllerMode):
//controllerOptions.AddFlags(fs)
// controllerOptions.AddFlags(fs)
args = os.Args[2:]
mode = driver.ControllerMode

Expand All @@ -73,12 +73,12 @@ func GetOptions(fs *flag.FlagSet) *Options {
mode = driver.NodeMode

case cmd == string(driver.AllMode):
//controllerOptions.AddFlags(fs)
// controllerOptions.AddFlags(fs)
nodeOptions.AddFlags(fs)
args = os.Args[2:]

case strings.HasPrefix(cmd, "-"):
//controllerOptions.AddFlags(fs)
// controllerOptions.AddFlags(fs)
nodeOptions.AddFlags(fs)
args = os.Args[1:]

Expand All @@ -95,7 +95,7 @@ func GetOptions(fs *flag.FlagSet) *Options {
if *version {
info, err := driver.GetVersionJSON()
if err != nil {
klog.Fatalf("error while retriving the CSI driver version. err: %v", err)
klog.Fatalf("error while retrieving the CSI driver version. err: %v", err)
}
klog.Info(info)
osExit(0)
Expand Down
38 changes: 19 additions & 19 deletions cmd/options/controller_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,23 @@ limitations under the License.

package options

//DONE

//// ControllerOptions contains options and configuration settings for the controller service.
//type ControllerOptions struct {
// // ExtraTags is a map of tags that will be attached to each dynamically provisioned
// // resource.
// ExtraTags map[string]string
// //// ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned
// //// volume.
// //// DEPRECATED: Use ExtraTags instead.
// //ExtraVolumeTags map[string]string
// // ID of the kubernetes cluster.
// KubernetesClusterID string
//}
/*
// ControllerOptions contains options and configuration settings for the controller service.
type ControllerOptions struct {
// ExtraTags is a map of tags that will be attached to each dynamically provisioned
// resource.
ExtraTags map[string]string
//// ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned
//// volume.
//// DEPRECATED: Use ExtraTags instead.
//ExtraVolumeTags map[string]string
// ID of the kubernetes cluster.
KubernetesClusterID string
}

//func (s *ControllerOptions) AddFlags(fs *flag.FlagSet) {
// //fs.Var(cliflag.NewMapStringString(&s.ExtraTags), "extra-tags", "Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
// //fs.Var(cliflag.NewMapStringString(&s.ExtraVolumeTags), "extra-volume-tags", "DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
// //fs.StringVar(&s.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).")
//}
func (s *ControllerOptions) AddFlags(fs *flag.FlagSet) {
fs.Var(cliflag.NewMapStringString(&s.ExtraTags), "extra-tags", "Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
fs.Var(cliflag.NewMapStringString(&s.ExtraVolumeTags), "extra-volume-tags", "DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
fs.StringVar(&s.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).")
}.
*/
2 changes: 1 addition & 1 deletion cmd/options/node_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

package options

//DONE
// DONE.

import (
"flag"
Expand Down
2 changes: 1 addition & 1 deletion cmd/options/server_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

package options

//DONE
// DONE.

import (
"flag"
Expand Down
Loading