Skip to content
Open
Show file tree
Hide file tree
Changes from 13 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
69 changes: 35 additions & 34 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ linters:
- copyloopvar
- decorder
- dogsled
# - dupl # temporarily disabled - has 2 issues
- dupl
- dupword
- durationcheck
- errcheck
- errchkjson
- errname
- exhaustive
Expand All @@ -32,14 +33,14 @@ linters:
- gomoddirectives
- gomodguard
- goprintffuncname
# - gosmopolitan # temporarily disabled - has 2 issues
- gosmopolitan
- govet
- grouper
- iface
- importas
- ineffassign
- interfacebloat
# - lll # temporarily disabled - has 50 issues
- lll
- loggercheck
- makezero
- mirror
Expand All @@ -48,18 +49,19 @@ linters:
- nilerr
- nilnesserr
- noctx
# - nolintlint # temporarily disabled - has 4 issues
- nolintlint
- nosprintfhostport
- predeclared
- promlinter
- protogetter
- reassign
- recvcheck
# - revive # temporarily disabled - has 38 issues
- revive
- rowserrcheck
- sloglint
- spancheck
- sqlclosecheck
- staticcheck
- tagliatelle
- testableexamples
- tparallel
Expand All @@ -76,8 +78,6 @@ linters:
# These linters are disabled ONLY because fixing/investigating their complaints initially was too overwhelming.
# We SHOULD try and evaluate each of them eventually and either fix their reports
# or comment here on why we decided to not listen to them.
- errcheck # temporarily disabled - has 11 issues
- staticcheck # temporarily disabled - has 7 issues
- cyclop
- depguard
- err113
Expand Down Expand Up @@ -109,6 +109,32 @@ linters:
- thelper
- varnamelen
- wrapcheck
settings:
errcheck:
check-type-assertions: true
check-blank: true
lll:
line-length: 250
dupl:
threshold: 400
govet:
settings:
printf:
funcs:
- (github.com/kudobuilder/kuttl/internal/utils.Logger).Logf
- (github.com/kudobuilder/kuttl/internal/utils.TestLogger).Logf
- (github.com/kudobuilder/kuttl/internal/case.clientWithKubeConfig).Logf
- (github.com/kudobuilder/kuttl/internal/case.clientWithKubeConfig).Wrapf
# TODO: fix the `context` usage in step package and enable these linters.
usetesting:
context-background: false
context-todo: false
exclusions:
paths:
- hack
- dist
- keps
- kind-*

formatters:
enable:
Expand All @@ -117,31 +143,6 @@ formatters:
settings:
goimports:
# Don't use 'github.com/kudobuilder/kuttl', it'll result in unreliable output!
local-prefixes: github.com/kudobuilder
local-prefixes:
- github.com/kudobuilder

linters-settings:
errcheck:
check-type-assertions: true
check-blank: true
lll:
line-length: 250
dupl:
threshold: 400
govet:
settings:
printf:
funcs:
- (github.com/kudobuilder/kuttl/internal/utils.Logger).Logf
- (github.com/kudobuilder/kuttl/internal/utils.TestLogger).Logf
- (github.com/kudobuilder/kuttl/internal/case.clientWithKubeConfig).Logf
- (github.com/kudobuilder/kuttl/internal/case.clientWithKubeConfig).Wrapf
# TODO: fix the `context` usage in step package and enable these linters.
usetesting:
context-background: false
context-todo: false
issues:
exclude-dirs:
- hack
- dist
- keps
- kind-*
1 change: 1 addition & 0 deletions cmd/kubectl-kuttl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// Package main provides the kubectl-kuttl command-line interface.
package main

import (
Expand Down
1 change: 1 addition & 0 deletions internal/assert/assert.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package assert provides assertion functionality for KUTTL test harness.
package assert

import (
Expand Down
2 changes: 1 addition & 1 deletion internal/assert/dummy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package assert
import "testing"

// TestDummy is a no-op test to satisfy coverage tools
func TestDummy(t *testing.T) {
func TestDummy(_ *testing.T) {
// This test exists only to ensure the package is recognized by go test
// and to avoid "go: no such tool 'covdata'" warnings
}
3 changes: 2 additions & 1 deletion internal/env/env.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Package env provides environment variable expansion functionality.
package env

import (
"os"
"strings"
)

// Expand provides OS expansion of defined ENV VARs inside args to commands. The expansion is limited to what is defined on the OS
// ExpandWithMap provides OS expansion of defined ENV VARs inside args to commands. The expansion is limited to what is defined on the OS
// and the variables passed into to the env parameter. To escape a dollar sign, pass in two dollar signs.
func ExpandWithMap(c string, env map[string]string) string {
// expand $$ -> $
Expand Down
2 changes: 2 additions & 0 deletions internal/expressions/cel.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package expressions provides Common Expression Language (CEL) evaluation functionality.
package expressions

import (
Expand Down Expand Up @@ -86,6 +87,7 @@ func RunAssertExpressions(
return errs
}

// LoadPrograms loads and compiles CEL programs from test assertions.
func LoadPrograms(testAssert *harness.TestAssert) (map[string]cel.Program, error) {
var errs []error
var assertions []*harness.Assertion
Expand Down
2 changes: 1 addition & 1 deletion internal/expressions/dummy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package expressions
import "testing"

// TestDummy is a no-op test to satisfy coverage tools
func TestDummy(t *testing.T) {
func TestDummy(_ *testing.T) {
// This test exists only to ensure the package is recognized by go test
// and to avoid "go: no such tool 'covdata'" warnings
}
5 changes: 3 additions & 2 deletions internal/file/files.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package file provides file manipulation and processing utilities.
package file

import (
Expand All @@ -11,7 +12,7 @@ import (
"github.com/kudobuilder/kuttl/internal/kubernetes"
)

// from a list of paths, returns an array of runtime objects
// ToObjects from a list of paths, returns an array of runtime objects.
func ToObjects(paths []string) ([]client.Object, error) {
apply := []client.Object{}

Expand All @@ -26,7 +27,7 @@ func ToObjects(paths []string) ([]client.Object, error) {
return apply, nil
}

// From a file or dir path returns an array of flat file paths
// FromPath from a file or dir path returns an array of flat file paths.
// pattern is a filepath.Match pattern to limit files to a pattern
func FromPath(path, pattern string) ([]string, error) {
files := []string{}
Expand Down
3 changes: 3 additions & 0 deletions internal/file/name.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
)

// Type represents the type of a test file.
type Type int

const (
Expand All @@ -21,6 +22,7 @@ const (
TypeError
)

// Info contains parsed information about a test file name.
type Info struct {
Type Type
// Error is set for TypeUnknown objects and describes the reason.
Expand All @@ -47,6 +49,7 @@ var fileNameRegex = regexp.MustCompile(`^(\d+-)?([^-.]+)(-[^.]+)?((?:\.gotmpl)?\
// fileNamePattern is a human-readable representation of fileNameRegex.
const fileNamePattern = "(<number>-)<name>(-<name>)((.gotmpl).yaml)"

// Parse parses a file name and returns information about its type and structure.
func Parse(fullName string) Info {
name := filepath.Base(fullName)
matches := fileNameRegex.FindStringSubmatch(name)
Expand Down
8 changes: 5 additions & 3 deletions internal/file/tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func UntarInPlace(path string) error {
if err != nil {
return err
}
defer file.Close()
defer file.Close() //nolint:errcheck

compressed := filepath.Ext(path) == ".tgz"
return UnTar(folder, file, compressed)
Expand All @@ -33,7 +33,7 @@ func UnTar(dest string, r io.Reader, compressed bool) (err error) {
if err != nil {
return err
}
defer gzr.Close()
defer gzr.Close() //nolint:errcheck
r = gzr
}
tr := tar.NewReader(r)
Expand Down Expand Up @@ -76,7 +76,9 @@ func UnTar(dest string, r io.Reader, compressed bool) (err error) {
return err
}

f.Close()
if err := f.Close(); err != nil {
return err
}
}
}
}
2 changes: 1 addition & 1 deletion internal/file/tar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func TestUntarInPlace(t *testing.T) {
assert.NoError(t, err)

folder := "testdata/tar-test"
defer os.RemoveAll(folder)
defer os.RemoveAll(folder) //nolint:errcheck

fi, err := os.Stat(folder)
assert.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion internal/harness/harness.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package harness provides the main test harness functionality for KUTTL.
package harness

import (
Expand Down Expand Up @@ -288,7 +289,7 @@ func (h *Harness) Config() (*rest.Config, error) {
return nil, err
}

defer f.Close()
defer f.Close() //nolint:errcheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be important to check for errors here? error on closing here could indicate that the config was not successfully written, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Excellent point, I will change the code to propagate errors from Close on writable filehandles.

Copy link
Member Author

Choose a reason for hiding this comment

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

I took a proper look at the Close() calls. It would be great if you could review the last commit @mclasmeier 🙏🏻


return h.config, kubernetes.Kubeconfig(h.config, f)
}
Expand Down
9 changes: 5 additions & 4 deletions internal/http/client.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package http
// Package http provides HTTP client functionality for downloading files and making requests.
package http //nolint:revive,nolintlint // apparently nolintlint is confused

import (
"bytes"
Expand Down Expand Up @@ -33,7 +34,7 @@ func (c *Client) Get(url string) (*http.Response, error) {
return resp, err
}

// Get performs HTTP get, retrieves the contents and returns a bytes.Buffer of the entire contents
// GetByteBuffer performs HTTP get, retrieves the contents and returns a bytes.Buffer of the entire contents
// this could be dangerous against an extremely large file
func (c *Client) GetByteBuffer(url string) (*bytes.Buffer, error) {
buf := bytes.NewBuffer(nil)
Expand Down Expand Up @@ -71,7 +72,7 @@ func (c *Client) Download(url string, path string) error {
if err != nil {
return err
}
defer resp.Body.Close()
defer resp.Body.Close() //nolint:errcheck

_, err = os.Stat(path)
if err == nil || os.IsExist(err) {
Expand All @@ -88,7 +89,7 @@ func (c *Client) Download(url string, path string) error {
if err != nil {
return err
}
defer out.Close()
defer out.Close() //nolint:errcheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as before, is ignoring err here safe?


// Create our bytes counter and pass it to be used alongside our writer
counter := &writeCounter{Name: filepath.Base(path)}
Expand Down
2 changes: 1 addition & 1 deletion internal/http/http.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package http
package http //nolint:revive,nolintlint // apparently nolintlint is confused

import (
"bytes"
Expand Down
2 changes: 1 addition & 1 deletion internal/http/http_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package http
package http //nolint:revive,nolintlint // apparently nolintlint is confused

import (
"testing"
Expand Down
1 change: 1 addition & 0 deletions internal/kubernetes/client.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package kubernetes provides utilities for working with Kubernetes clients and resources.
package kubernetes

import (
Expand Down
11 changes: 7 additions & 4 deletions internal/kubernetes/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/config"
)

// ImpersonateAs holds the username to impersonate in Kubernetes API calls.
var ImpersonateAs = ""

// GetConfig returns a Kubernetes REST configuration with optional user impersonation.
func GetConfig() (*rest.Config, error) {
cfg, err := config.GetConfig()
if err != nil {
Expand All @@ -26,6 +28,7 @@ func GetConfig() (*rest.Config, error) {
return cfg, nil
}

// BuildConfigWithContext creates a Kubernetes REST configuration from a kubeconfig file and context.
func BuildConfigWithContext(kubeconfigPath, context string) (*rest.Config, error) {
if context == "" {
// Use default context
Expand Down Expand Up @@ -74,8 +77,8 @@ func Kubeconfig(cfg *rest.Config, w io.Writer) error {
Name: "cluster",
Cluster: appsv1.Cluster{
Server: cfg.Host,
CertificateAuthorityData: cfg.TLSClientConfig.CAData,
InsecureSkipTLSVerify: cfg.TLSClientConfig.Insecure,
CertificateAuthorityData: cfg.CAData,
InsecureSkipTLSVerify: cfg.Insecure,
},
},
},
Expand All @@ -92,8 +95,8 @@ func Kubeconfig(cfg *rest.Config, w io.Writer) error {
{
Name: "user",
AuthInfo: appsv1.AuthInfo{
ClientCertificateData: cfg.TLSClientConfig.CertData,
ClientKeyData: cfg.TLSClientConfig.KeyData,
ClientCertificateData: cfg.CertData,
ClientKeyData: cfg.KeyData,
Token: cfg.BearerToken,
Username: cfg.Username,
Password: cfg.Password,
Expand Down
3 changes: 2 additions & 1 deletion internal/kubernetes/fake/dummy_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Package fake provides fake Kubernetes client implementations for testing.
package fake

import "testing"

// TestDummy is a no-op test to satisfy coverage tools
func TestDummy(t *testing.T) {
func TestDummy(_ *testing.T) {
// This test exists only to ensure the package is recognized by go test
// and to avoid "go: no such tool 'covdata'" warnings
}
1 change: 1 addition & 0 deletions internal/kubernetes/retry_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ func (r *RetryClient) Patch(ctx context.Context, obj client.Object, patch client
}, isJSONSyntaxError)
}

// Apply applies a Kubernetes configuration with automatic retry on transient failures.
func (r *RetryClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error {
return retry(ctx, func(ctx context.Context) error {
return r.Client.Apply(ctx, obj, opts...)
Expand Down
Loading
Loading