feat(lint) : add golangci-lint

.travis.yml

@@ -50,6 +50,8 @@ install:
   -
 
 script:
+- curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.21.0
+- ./scripts/verify.sh
 # The golden_test.sh check if the the testdata is updated according to the current changes
 # To update the testdata use the Makefile targets `make generate-setup` then `make generate-testdata`
 - ./golden_test.sh

CONTRIBUTING.md

@@ -39,6 +39,8 @@ $ git clone [email protected]:<user>/kubebuilder.git $GOPATH/src/sigs.k8s.io/kubebu
 1. Run the script `make generate` to update/generate the mock data used in the e2e test in `$GOPATH/src/sigs.k8s.io/kubebuilder/testdata/`
 
 **IMPORTANT:** The `make generate` is very helpful. By using it, you can check if good part of the commands still working successfully after the changes. Also, note that its usage is a pre-requirement to submit a PR.
+
+**NOTE** To run `make lint` is required to install `golangci-lint` locally it. More info: https://github.com/golangci/golangci-lint#install
 
 ## Where the CI Tests are configured?
 

Makefile

@@ -62,6 +62,10 @@ generate-setup: ## Current workarround to generate the testdata with the correct
 	- rm -rf $(CONTROLLER_GEN_BIN_PATH)
 	- GO111MODULE=on go get sigs.k8s.io/controller-tools/cmd/[email protected]
 
+.PHONY: lint
+lint: ## Run code lint checks
+	./scripts/verify.sh
+
 ##@ Tests
 
 .PHONY: test

cmd/init_project.go

@@ -96,8 +96,13 @@ func (o *projectOptions) bindCmdlineFlags(cmd *cobra.Command) {
 	cmd.Flags().BoolVar(&o.dep, "dep", true, "if specified, determines whether dep will be used.")
 	o.depFlag = cmd.Flag("dep")
 	cmd.Flags().StringArrayVar(&o.depArgs, "depArgs", nil, "Additional arguments for dep")
-	cmd.Flags().MarkDeprecated("dep", "use the fetch-deps flag instead")
-	cmd.Flags().MarkDeprecated("depArgs", "will be removed with version 1 scaffolding")
+
+	if err := cmd.Flags().MarkDeprecated("dep", "use the fetch-deps flag instead"); err != nil {
+		log.Printf("error to mark dep flag as deprecated: %v", err)
+	}
+	if err := cmd.Flags().MarkDeprecated("depArgs", "will be removed with version 1 scaffolding"); err != nil {
+		log.Printf("error to mark dep flag as deprecated: %v", err)
+	}
 
 	// boilerplate args
 	cmd.Flags().StringVar(&o.boilerplate.Path, "path", "", "path for boilerplate")

cmd/main.go

@@ -178,7 +178,9 @@ After the scaffold is written, api will run make on the project.
 `,
 
 		Run: func(cmd *cobra.Command, args []string) {
-			cmd.Help()
+			if err := cmd.Help(); err != nil {
+				log.Fatalf("failed to call the help: %v", err)
+			}
 		},
 	}
 }

cmd/util/validations.go

@@ -12,8 +12,9 @@ import (
 const (
 	qnameCharFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
 	// The value is 56 because it will be contact with "-system" = 63
-    qualifiedNameMaxLength int = 56
+	qualifiedNameMaxLength int = 56
 )
+
 var qualifiedNameRegexp = regexp.MustCompile("^" + qnameCharFmt + "$")
 
 //IsValidName used to check the name of the project

common.sh

@@ -17,6 +17,17 @@ set -o errexit
 set -o nounset
 set -o pipefail
 
+# check if modules are enabled
+(go mod edit -json &>/dev/null)
+MODULES_ENABLED=$?
+
+MOD_OPT=""
+MODULES_OPT=${MODULES_OPT:-""}
+if [[ -n "${MODULES_OPT}" && $MODULES_ENABLED ]]; then
+    MOD_OPT="-mod=${MODULES_OPT}"
+fi
+
+
 # Enable tracing in this script off by setting the TRACE variable in your
 # environment to any value:
 #
@@ -112,14 +123,13 @@ function prepare_staging_dir {
 
 # fetch k8s API gen tools and make it available under kb_root_dir/bin.
 function fetch_tools {
-  if [ -n "$SKIP_FETCH_TOOLS" ]; then
-    return 0
+  if [ -z "$SKIP_FETCH_TOOLS" ]; then
+    fetch_kb_tools
   fi
-  fetch_kb_tools
 }
 
 function fetch_kb_tools {
-  header_text "fetching tools"
+  header_text "fetching kb tools"
   kb_tools_archive_name="kubebuilder-tools-$k8s_version-$goos-$goarch.tar.gz"
   kb_tools_download_url="https://storage.googleapis.com/kubebuilder-tools/$kb_tools_archive_name"
 

pkg/scaffold/api.go

@@ -37,8 +37,6 @@ import (
 // API contains configuration for generating scaffolding for Go type
 // representing the API and controller that implements the behavior for the API.
 type API struct {
-	scaffold *Scaffold
-
 	// Plugins is the list of plugins we should allow to transform our generated scaffolding
 	Plugins []Plugin
 
@@ -275,7 +273,7 @@ func (api *API) scaffoldV2() error {
 // being created belongs to existing group.
 func (api *API) validateResourceGroup(r *resource.Resource) error {
 	for _, existingGroup := range api.project.ResourceGroups() {
-		if strings.ToLower(r.Group) != strings.ToLower(existingGroup) {
+		if !strings.EqualFold(r.Group, existingGroup) {
 			return fmt.Errorf("group '%s' is not same as existing group '%s'. Multiple groups are not supported yet.", r.Group, existingGroup)
 		}
 	}

pkg/scaffold/resource/resource.go

@@ -68,7 +68,7 @@ func (r *Resource) Validate() error {
 		return fmt.Errorf("group name is invalid: (%v)", err)
 	}
 	// Check if the version is a valid value
-	versionMatch := regexp.MustCompile("^v\\d+(alpha\\d+|beta\\d+)?$")
+	versionMatch := regexp.MustCompile(`^v\d+(alpha\d+|beta\d+)?$`)
 	if !versionMatch.MatchString(r.Version) {
 		return fmt.Errorf(
 			"version must match ^v\\d+(alpha\\d+|beta\\d+)?$ (was %s)", r.Version)
@@ -85,7 +85,7 @@ func (r *Resource) Validate() error {
 	}
 	// Replace the caracter "-" for "" to allow scaffold the go imports
 	r.GroupImportSafe = strings.Replace(r.Group, "-", "", -1)
-    r.GroupImportSafe = strings.Replace(r.GroupImportSafe, ".", "", -1)
+	r.GroupImportSafe = strings.Replace(r.GroupImportSafe, ".", "", -1)
 	return nil
 }
 

pkg/scaffold/util/util.go

@@ -60,7 +60,7 @@ func GetResourceInfo(r *resource.Resource, repo, domain string) (resourcePackage
 			// apiextensions.k8s.io is in k8s.io/apiextensions-apiserver/pkg/apis/apiextensions
 			// metrics.k8s.io is in k8s.io/metrics/pkg/apis/metrics
 			resourcePackage := path.Join("k8s.io", "api", r.Group)
-			groupDomain = r.Group
+			groupDomain := r.Group
 			if domain != "" {
 				groupDomain = r.Group + "." + domain
 			}

pkg/scaffold/v1/webhook/admissionbuilder.go

@@ -48,7 +48,7 @@ type AdmissionWebhookBuilder struct {
 
 // GetInput implements input.File
 func (a *AdmissionWebhookBuilder) GetInput() (input.Input, error) {
-	a.ResourcePackage, _ = getResourceInfo(coreGroups, a.Resource, a.Input)
+	a.ResourcePackage = getResourceInfo(coreGroups, a.Resource, a.Input)
 
 	if a.Type == "mutating" {
 		a.Mutating = true

pkg/scaffold/v1/webhook/admissionhandler.go

@@ -48,7 +48,7 @@ type AdmissionHandler struct {
 
 // GetInput implements input.File
 func (a *AdmissionHandler) GetInput() (input.Input, error) {
-	a.ResourcePackage, _ = getResourceInfo(coreGroups, a.Resource, a.Input)
+	a.ResourcePackage = getResourceInfo(coreGroups, a.Resource, a.Input)
 	a.Type = strings.ToLower(a.Type)
 	if a.Type == "mutating" {
 		a.Mutate = true

pkg/scaffold/v1/webhook/util.go

@@ -49,19 +49,15 @@ func builderName(config Config, resource string) string {
 	return fmt.Sprintf("%s-%s-%s", config.Type, opsStr, resource)
 }
 
-func getResourceInfo(coreGroups map[string]string, r *resource.Resource, in input.Input) (resourcePackage, groupDomain string) {
+func getResourceInfo(coreGroups map[string]string, r *resource.Resource, in input.Input) (resourcePackage string) {
 	resourcePath := filepath.Join("pkg", "apis", r.Group, r.Version,
 		fmt.Sprintf("%s_types.go", strings.ToLower(r.Kind)))
 	if _, err := os.Stat(resourcePath); os.IsNotExist(err) {
-		if domain, found := coreGroups[r.Group]; found {
+		if _, found := coreGroups[r.Group]; found {
 			resourcePackage := path.Join("k8s.io", "api")
-			groupDomain = r.Group
-			if domain != "" {
-				groupDomain = r.Group + "." + domain
-			}
-			return resourcePackage, groupDomain
+			return resourcePackage
 		}
 		// TODO: need to support '--resource-pkg-path' flag for specifying resourcePath
 	}
-	return path.Join(in.Repo, "pkg", "apis"), r.Group + "." + in.Domain
+	return path.Join(in.Repo, "pkg", "apis")
 }

pkg/scaffold/v1/webhook/webhook_test.go

@@ -73,13 +73,13 @@ var _ = Describe("Webhook", func() {
 				{
 					file: filepath.Join("pkg", "webhook", "add_default_server.go"),
 					instance: &AddServer{
-						Config:   in.Config,
+						Config: in.Config,
 					},
 				},
 				{
 					file: filepath.Join("pkg", "webhook", "default_server", "server.go"),
 					instance: &Server{
-						Config:   in.Config,
+						Config: in.Config,
 					},
 				},
 				{
@@ -112,17 +112,17 @@ var _ = Describe("Webhook", func() {
 						strings.ToLower(in.Kind), strings.ToLower(in.Type),
 						fmt.Sprintf("%s_webhook.go", strings.Join(in.Operations, "_"))),
 					instance: &AdmissionWebhookBuilder{
-						Resource:    &in.Resource,
-						Config:      in.Config,
+						Resource: &in.Resource,
+						Config:   in.Config,
 					},
 				},
 				{
 					file: filepath.Join("pkg", "webhook", "default_server",
 						strings.ToLower(in.Kind), strings.ToLower(in.Type),
 						fmt.Sprintf("%s_%s_handler.go", strings.ToLower(in.Kind), strings.Join(in.Operations, "_"))),
 					instance: &AdmissionHandler{
-						Resource:    &in.Resource,
-						Config:      in.Config,
+						Resource: &in.Resource,
+						Config:   in.Config,
 					},
 				},
 			}

pkg/scaffold/v2/internal/string_utils_test.go

@@ -25,7 +25,6 @@ type insertStrTest struct {
 	input         string
 	markerNValues map[string][]string
 	expected      string
-	got           string
 }
 
 func TestInsertStrBelowMarker(t *testing.T) {

scripts/verify.sh

@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+
+#  Copyright 2018 The Kubernetes Authors.
+#
+#  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.
+
+set -e
+
+source ./common.sh
+
+header_text "running go vet"
+go vet ${MOD_OPT} ../...
+
+header_text "running go fmt"
+go fmt ${MOD_OPT} ../...
+
+header_text "running golangci-lint"
+cd .. # To go to the root of the project
+golangci-lint run --disable-all \
+    --deadline 5m \
+    --enable=misspell \
+    --enable=structcheck \
+    --enable=deadcode \
+    --enable=errcheck \
+    --enable=varcheck \
+    --enable=unparam \
+    --enable=ineffassign \
+    --enable=nakedret \
+    --enable=interfacer \
+    --enable=misspell \
+    --enable=dupl
+
+##todo(camilamacedo86): The following checks requires fixes in the code
+# --enable=golint
+# --enable=gocyclo
+# --enable=goimports
+# --enable=lll
+# --enable=goconst
+# --enable=gosec
+# --enable=maligned
+
+GO111MODULES=on go list -mod=readonly ./...

test/e2e/utils/kubectl.go

@@ -44,7 +44,7 @@ func (k *Kubectl) CommandWithInput(stdinInput string, cmdOptions ...string) (str
 	}
 	go func() {
 		defer stdin.Close()
-		io.WriteString(stdin, stdinInput)
+		_, _ = io.WriteString(stdin, stdinInput)
 	}()
 	output, err := k.Run(cmd)
 	return string(output), err

test/e2e/v2/e2e_suite.go

@@ -332,7 +332,7 @@ func implementWebhooks(filename string) error {
 }
 
 func ensureExistAndReplace(input, match, replace string) (string, error) {
-	if strings.Index(input, match) == -1 {
+	if !strings.Contains(input, match) {
 		return "", fmt.Errorf("can't find %q", match)
 	}
 	return strings.Replace(input, match, replace, -1), nil