diff --git a/.golangci.yml b/.golangci.yml index 39fec8e2..40b090ce 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,9 +12,10 @@ linters: - copyloopvar - decorder - dogsled - # - dupl # temporarily disabled - has 2 issues + - dupl - dupword - durationcheck + - errcheck - errchkjson - errname - exhaustive @@ -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 @@ -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 @@ -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 @@ -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: @@ -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-* diff --git a/cmd/kubectl-kuttl/main.go b/cmd/kubectl-kuttl/main.go index 31a18c7c..1165d999 100644 --- a/cmd/kubectl-kuttl/main.go +++ b/cmd/kubectl-kuttl/main.go @@ -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 ( diff --git a/internal/assert/assert.go b/internal/assert/assert.go index fca50433..f2e705b8 100644 --- a/internal/assert/assert.go +++ b/internal/assert/assert.go @@ -1,3 +1,4 @@ +// Package assert provides assertion functionality for KUTTL test harness. package assert import ( diff --git a/internal/assert/dummy_test.go b/internal/assert/dummy_test.go index c33c668e..1f4ff6b7 100644 --- a/internal/assert/dummy_test.go +++ b/internal/assert/dummy_test.go @@ -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 } diff --git a/internal/env/env.go b/internal/env/env.go index 4e906f88..a26b508f 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -1,3 +1,4 @@ +// Package env provides environment variable expansion functionality. package env import ( @@ -5,7 +6,7 @@ import ( "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 $$ -> $ diff --git a/internal/expressions/cel.go b/internal/expressions/cel.go index ddf3a9b2..cc663f36 100644 --- a/internal/expressions/cel.go +++ b/internal/expressions/cel.go @@ -1,3 +1,4 @@ +// Package expressions provides Common Expression Language (CEL) evaluation functionality. package expressions import ( @@ -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 diff --git a/internal/expressions/dummy_test.go b/internal/expressions/dummy_test.go index 1b210301..51c32083 100644 --- a/internal/expressions/dummy_test.go +++ b/internal/expressions/dummy_test.go @@ -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 } diff --git a/internal/file/files.go b/internal/file/files.go index f1d206a3..625f0d6f 100644 --- a/internal/file/files.go +++ b/internal/file/files.go @@ -1,3 +1,4 @@ +// Package file provides file manipulation and processing utilities. package file import ( @@ -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{} @@ -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{} diff --git a/internal/file/name.go b/internal/file/name.go index 0a1f2b58..a32bd44a 100644 --- a/internal/file/name.go +++ b/internal/file/name.go @@ -8,6 +8,7 @@ import ( "strings" ) +// Type represents the type of a test file. type Type int const ( @@ -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. @@ -47,6 +49,7 @@ var fileNameRegex = regexp.MustCompile(`^(\d+-)?([^-.]+)(-[^.]+)?((?:\.gotmpl)?\ // fileNamePattern is a human-readable representation of fileNameRegex. const fileNamePattern = "(-)(-)((.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) diff --git a/internal/file/tar.go b/internal/file/tar.go index d053a2ff..fb3ee3f5 100644 --- a/internal/file/tar.go +++ b/internal/file/tar.go @@ -17,7 +17,7 @@ func UntarInPlace(path string) error { if err != nil { return err } - defer file.Close() + defer file.Close() //nolint:errcheck // We are just reading, don't care if closing fails as long as UnTar works. compressed := filepath.Ext(path) == ".tgz" return UnTar(folder, file, compressed) @@ -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 // We are just reading, don't care if closing fails as long as reading works. r = gzr } tr := tar.NewReader(r) @@ -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 + } } } } diff --git a/internal/file/tar_test.go b/internal/file/tar_test.go index 735b8bc8..5a070e9f 100644 --- a/internal/file/tar_test.go +++ b/internal/file/tar_test.go @@ -2,19 +2,27 @@ package file import ( "os" + "path" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestUntarInPlace(t *testing.T) { tarfile := "testdata/tar-test.tgz" - err := UntarInPlace(tarfile) - assert.NoError(t, err) - folder := "testdata/tar-test" - defer os.RemoveAll(folder) + sandbox := t.TempDir() + + testFile := path.Join(sandbox, "tar-test.tgz") + data, err := os.ReadFile(tarfile) + require.NoError(t, err) + require.NoError(t, os.WriteFile(testFile, data, 0600)) + + err = UntarInPlace(testFile) + assert.NoError(t, err) + folder := path.Join(sandbox, "tar-test") fi, err := os.Stat(folder) assert.NoError(t, err) assert.True(t, fi.IsDir()) diff --git a/internal/harness/harness.go b/internal/harness/harness.go index e73f8ee3..91ab510b 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -1,3 +1,4 @@ +// Package harness provides the main test harness functionality for KUTTL. package harness import ( @@ -288,8 +289,6 @@ func (h *Harness) Config() (*rest.Config, error) { return nil, err } - defer f.Close() - return h.config, kubernetes.Kubeconfig(h.config, f) } diff --git a/internal/http/client.go b/internal/http/client.go index 11f9d696..cc77a679 100644 --- a/internal/http/client.go +++ b/internal/http/client.go @@ -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" @@ -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) @@ -66,14 +67,8 @@ func (c *Client) DownloadFile(url, path string) (string, error) { // Download takes a url to download and a filepath to write it to // this will write the response to any url request to a file. func (c *Client) Download(url string, path string) error { - // Get the data - resp, err := c.Get(url) - if err != nil { - return err - } - defer resp.Body.Close() - - _, err = os.Stat(path) + // Check destination path. + _, err := os.Stat(path) if err == nil || os.IsExist(err) { return os.ErrExist } @@ -82,17 +77,46 @@ func (c *Client) Download(url string, path string) error { return err } + // Get the data + resp, err := c.Get(url) //nolint:bodyclose // downloadWithProgress takes ownership of resp.Body and closes it. + if err != nil { + return err + } + // Create the file with .tmp extension, so that we won't overwrite a // file until it's downloaded fully - out, err := os.Create(path + ".tmp") + tmpPath := path + ".tmp" + if err := downloadWithProgress(resp.Body, tmpPath); err != nil { + // Try to clean up garbage if download fails. + rmErr := os.Remove(tmpPath) + // At least warn if unable to clean up for some reason. + if !os.IsNotExist(rmErr) { + fmt.Printf("Warning: failed to remove temporary file %q: %v\n", tmpPath, rmErr) + } + return err + } + // Rename the tmp file back to the original file + return os.Rename(tmpPath, path) +} + +func downloadWithProgress(body io.ReadCloser, path string) (err error) { + defer body.Close() //nolint:errcheck // If the download completed OK, we don't care if closing fails. + + var out *os.File + out, err = os.Create(path) if err != nil { return err } - defer out.Close() + defer func() { + closeErr := out.Close() + if err == nil { + err = fmt.Errorf("failed to close downloaded file %q: %w", path, closeErr) + } + }() // Create our bytes counter and pass it to be used alongside our writer counter := &writeCounter{Name: filepath.Base(path)} - _, err = io.Copy(out, io.TeeReader(resp.Body, counter)) + _, err = io.Copy(out, io.TeeReader(body, counter)) if err != nil { return err } @@ -100,13 +124,7 @@ func (c *Client) Download(url string, path string) error { // The progress use the same line so print a new line once it's finished downloading fmt.Println() - // Rename the tmp file back to the original file - err = os.Rename(path+".tmp", path) - if err != nil { - return err - } - - return nil + return err } type writeCounter struct { diff --git a/internal/http/http.go b/internal/http/http.go index 3090afdd..2fe26736 100644 --- a/internal/http/http.go +++ b/internal/http/http.go @@ -1,4 +1,4 @@ -package http +package http //nolint:revive,nolintlint // apparently nolintlint is confused import ( "bytes" diff --git a/internal/http/http_test.go b/internal/http/http_test.go index bc98dfc9..e3179767 100644 --- a/internal/http/http_test.go +++ b/internal/http/http_test.go @@ -1,4 +1,4 @@ -package http +package http //nolint:revive,nolintlint // apparently nolintlint is confused import ( "testing" diff --git a/internal/kubernetes/client.go b/internal/kubernetes/client.go index ab438a20..fb527561 100644 --- a/internal/kubernetes/client.go +++ b/internal/kubernetes/client.go @@ -1,3 +1,4 @@ +// Package kubernetes provides utilities for working with Kubernetes clients and resources. package kubernetes import ( diff --git a/internal/kubernetes/config.go b/internal/kubernetes/config.go index 9245408b..10fb52dd 100644 --- a/internal/kubernetes/config.go +++ b/internal/kubernetes/config.go @@ -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 { @@ -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 @@ -37,8 +40,9 @@ func BuildConfigWithContext(kubeconfigPath, context string) (*rest.Config, error &clientcmd.ConfigOverrides{CurrentContext: context}).ClientConfig() } -// Kubeconfig converts a rest.Config into a YAML kubeconfig and writes it to w -func Kubeconfig(cfg *rest.Config, w io.Writer) error { +// Kubeconfig converts a rest.Config into a YAML kubeconfig and writes it to w. +// Takes ownership of w and closes it. +func Kubeconfig(cfg *rest.Config, w io.WriteCloser) error { var authProvider *appsv1.AuthProviderConfig var execConfig *appsv1.ExecConfig if cfg.AuthProvider != nil { @@ -65,17 +69,18 @@ func Kubeconfig(cfg *rest.Config, w io.Writer) error { } err := rest.LoadTLSFiles(cfg) if err != nil { + w.Close() //nolint:errcheck // We haven't written anything, we do not care if this fails, just preventing leaks. return err } - return json.NewYAMLSerializer(json.DefaultMetaFactory, nil, nil).Encode(&appsv1.Config{ + err = json.NewYAMLSerializer(json.DefaultMetaFactory, nil, nil).Encode(&appsv1.Config{ CurrentContext: "cluster", Clusters: []appsv1.NamedCluster{ { Name: "cluster", Cluster: appsv1.Cluster{ Server: cfg.Host, - CertificateAuthorityData: cfg.TLSClientConfig.CAData, - InsecureSkipTLSVerify: cfg.TLSClientConfig.Insecure, + CertificateAuthorityData: cfg.CAData, + InsecureSkipTLSVerify: cfg.Insecure, }, }, }, @@ -92,8 +97,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, @@ -106,4 +111,13 @@ func Kubeconfig(cfg *rest.Config, w io.Writer) error { }, }, }, w) + if err != nil { + w.Close() //nolint:errcheck // We do not care if closing failed in addition to writing, just preventing leaks. + return err + } + err = w.Close() + if err != nil { + return fmt.Errorf("error closing newly written kubeconfig file: %w", err) + } + return nil } diff --git a/internal/kubernetes/fake/dummy_test.go b/internal/kubernetes/fake/dummy_test.go index cfd17b38..a7903dfa 100644 --- a/internal/kubernetes/fake/dummy_test.go +++ b/internal/kubernetes/fake/dummy_test.go @@ -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 } diff --git a/internal/kubernetes/retry_client.go b/internal/kubernetes/retry_client.go index 0791e504..adb93f3f 100644 --- a/internal/kubernetes/retry_client.go +++ b/internal/kubernetes/retry_client.go @@ -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...) diff --git a/internal/kubernetes/serialization.go b/internal/kubernetes/serialization.go index 367b6cf3..e3b96dac 100644 --- a/internal/kubernetes/serialization.go +++ b/internal/kubernetes/serialization.go @@ -120,14 +120,14 @@ func convertUnstructured(in client.Object) (client.Object, error) { if group != kuttlGroup { return in, nil } - switch { - case kind == "TestFile": + switch kind { + case "TestFile": converted = &v1beta1.TestFile{} - case kind == "TestStep": + case "TestStep": converted = &v1beta1.TestStep{} - case kind == "TestAssert": + case "TestAssert": converted = &v1beta1.TestAssert{} - case kind == "TestSuite": + case "TestSuite": converted = &v1beta1.TestSuite{} default: return in, nil @@ -194,7 +194,7 @@ func LoadYAMLFromFile(path string) ([]client.Object, error) { if err != nil { return nil, err } - defer opened.Close() + defer opened.Close() //nolint:errcheck // We do not care if closing fails as long as LoadYAML works. return LoadYAML(path, opened) } diff --git a/internal/kubernetes/serialization_test.go b/internal/kubernetes/serialization_test.go index 3acf0182..0916d72c 100644 --- a/internal/kubernetes/serialization_test.go +++ b/internal/kubernetes/serialization_test.go @@ -12,7 +12,6 @@ import ( func TestLoadYAML(t *testing.T) { tmpfile, err := os.CreateTemp(t.TempDir(), "test.yaml") require.NoError(t, err) - defer tmpfile.Close() err = os.WriteFile(tmpfile.Name(), []byte(` apiVersion: v1 @@ -36,9 +35,8 @@ spec: - name: nginx image: nginx:1.7.9 `), 0600) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, tmpfile.Close()) objs, err := LoadYAMLFromFile(tmpfile.Name()) require.NoError(t, err) @@ -147,7 +145,6 @@ func TestPrettyDiff(t *testing.T) { func TestMatchesKind(t *testing.T) { tmpfile, err := os.CreateTemp(t.TempDir(), "test.yaml") require.NoError(t, err) - defer tmpfile.Close() err = os.WriteFile(tmpfile.Name(), []byte(` apiVersion: v1 @@ -164,9 +161,8 @@ kind: CustomResourceDefinition metadata: name: hello `), 0600) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, tmpfile.Close()) objs, err := LoadYAMLFromFile(tmpfile.Name()) require.NoError(t, err) diff --git a/internal/kuttlctl/cmd/assert.go b/internal/kuttlctl/cmd/assert.go index e0d39693..0eea79d2 100644 --- a/internal/kuttlctl/cmd/assert.go +++ b/internal/kuttlctl/cmd/assert.go @@ -1,3 +1,4 @@ +// Package cmd provides command-line interface commands for KUTTL. package cmd import ( diff --git a/internal/kuttlctl/cmd/values_test.go b/internal/kuttlctl/cmd/values_test.go index dfd358a7..e00b95c5 100644 --- a/internal/kuttlctl/cmd/values_test.go +++ b/internal/kuttlctl/cmd/values_test.go @@ -120,14 +120,14 @@ func TestParseVars(t *testing.T) { }, "unicode and special characters": { input: map[string]string{ - "unicode": "测试", + "unicode": "测试", //nolint:gosmopolitan // Unicode test data "emoji": "🚀", "special_chars": "name@domain.com", "with_quotes": `"quoted string"`, "with_backslash": `path\to\file`, }, expected: map[string]any{ - "unicode": "测试", + "unicode": "测试", //nolint:gosmopolitan // Unicode test data "emoji": "🚀", "special_chars": "name@domain.com", "with_quotes": "quoted string", diff --git a/internal/report/report.go b/internal/report/report.go index 37d04f5c..19373286 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -1,3 +1,4 @@ +// Package report provides test result reporting functionality in various formats. package report import ( @@ -229,6 +230,7 @@ func (ts *Testsuite) summarize() time.Time { return end } +// NewTestReporter creates a new test reporter based on the configured granularity. func (ts *Testsuite) NewTestReporter(name string) TestReporter { switch ts.reportGranularity { case "test": diff --git a/internal/step/expression_integration_test.go b/internal/step/expression_integration_test.go index 4ab211e2..daca7b30 100644 --- a/internal/step/expression_integration_test.go +++ b/internal/step/expression_integration_test.go @@ -1,5 +1,6 @@ //go:build integration +// Package step provides integration tests for step expression functionality. package step import ( diff --git a/internal/step/step.go b/internal/step/step.go index 053e5524..337fe379 100644 --- a/internal/step/step.go +++ b/internal/step/step.go @@ -427,6 +427,7 @@ func (s *Step) CheckAssertCommands(ctx context.Context, namespace string, comman return testErrors } +// CheckAssertExpressions validates assertion expressions against the current cluster state. func (s *Step) CheckAssertExpressions(namespace string) []error { client, err := s.Client(false) if err != nil { diff --git a/internal/step/step_integration_test.go b/internal/step/step_integration_test.go index 1c301b62..1ceddbe0 100644 --- a/internal/step/step_integration_test.go +++ b/internal/step/step_integration_test.go @@ -44,6 +44,44 @@ func TestMain(m *testing.M) { } func TestCheckResourceIntegration(t *testing.T) { + // Helper function to create nginx container spec + createNginxContainerSpec := func() map[string]interface{} { + return map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx:1.7.9", + "name": "nginx", + }, + }, + } + } + + // Helper function to create a pod with labels and nginx container + createPodWithLabels := func(name string, labels map[string]string) client.Object { + return kubernetes.WithSpec(t, kubernetes.WithLabels(t, kubernetes.NewPod(name, ""), labels), createNginxContainerSpec()) + } + + // Common expected pod object + expectedNginxPod := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "app": "nginx", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx:1.7.9", + "name": "nginx", + }, + }, + }, + }, + } + for _, test := range []struct { testName string actual []client.Object @@ -53,90 +91,18 @@ func TestCheckResourceIntegration(t *testing.T) { { testName: "match object by labels, first in list matches", actual: []client.Object{ - kubernetes.WithSpec(t, kubernetes.WithLabels(t, kubernetes.NewPod("labels-match-pod", ""), map[string]string{ - "app": "nginx", - }), map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx:1.7.9", - "name": "nginx", - }, - }, - }), - kubernetes.WithSpec(t, kubernetes.WithLabels(t, kubernetes.NewPod("bb", ""), map[string]string{ - "app": "not-match", - }), map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx:1.7.9", - "name": "nginx", - }, - }, - }), - }, - expected: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Pod", - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "app": "nginx", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx:1.7.9", - "name": "nginx", - }, - }, - }, - }, + createPodWithLabels("labels-match-pod", map[string]string{"app": "nginx"}), + createPodWithLabels("bb", map[string]string{"app": "not-match"}), }, + expected: expectedNginxPod, }, { testName: "match object by labels, last in list matches", actual: []client.Object{ - kubernetes.WithSpec(t, kubernetes.WithLabels(t, kubernetes.NewPod("last-in-list", ""), map[string]string{ - "app": "not-match", - }), map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx:1.7.9", - "name": "nginx", - }, - }, - }), - kubernetes.WithSpec(t, kubernetes.WithLabels(t, kubernetes.NewPod("bb", ""), map[string]string{ - "app": "nginx", - }), map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx:1.7.9", - "name": "nginx", - }, - }, - }), - }, - expected: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Pod", - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "app": "nginx", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx:1.7.9", - "name": "nginx", - }, - }, - }, - }, + createPodWithLabels("last-in-list", map[string]string{"app": "not-match"}), + createPodWithLabels("bb", map[string]string{"app": "nginx"}), }, + expected: expectedNginxPod, }, { testName: "match object by labels, does not exist", diff --git a/internal/template/env.go b/internal/template/env.go index 0387a213..fd115c7a 100644 --- a/internal/template/env.go +++ b/internal/template/env.go @@ -1,3 +1,4 @@ +// Package template provides template processing functionality. package template import ( @@ -18,6 +19,7 @@ type Env struct { // Please keep docs/testing/templating.md in sync. } +// Clone creates a deep copy of the template environment. func (e Env) Clone() (Env, error) { buf := bytes.Buffer{} if err := json.NewEncoder(&buf).Encode(e); err != nil { @@ -30,6 +32,7 @@ func (e Env) Clone() (Env, error) { return clone, nil } +// LoadAndExpand loads a template file and expands it with the provided environment variables. func LoadAndExpand(fileName string, env Env) (io.Reader, error) { tpl, err := template.ParseFiles(fileName) if err != nil { diff --git a/internal/testcase/case.go b/internal/testcase/case.go index 8ad053d6..08e67844 100644 --- a/internal/testcase/case.go +++ b/internal/testcase/case.go @@ -1,3 +1,4 @@ +// Package testcase provides test case execution functionality for KUTTL. package testcase import ( @@ -85,6 +86,7 @@ func WithRunLabels(runLabels labels.Set) CaseOption { } } +// WithTemplateVars returns a CaseOption that sets template variables for the test case. func WithTemplateVars(vars map[string]any) CaseOption { return func(c *Case) { c.templateEnv = template.Env{Vars: vars} @@ -201,6 +203,7 @@ func (c *Case) deleteNamespace(cl clientWithKubeConfig) error { return cl.Wrapf(err, "waiting for namespace %q to be deleted timed out", c.ns.name) } +// T represents a testing context interface with essential methods for test execution. type T interface { Context() context.Context Cleanup(f func()) diff --git a/internal/utils/commands.go b/internal/utils/commands.go index a5fe42bb..d6420921 100644 --- a/internal/utils/commands.go +++ b/internal/utils/commands.go @@ -1,4 +1,5 @@ -package utils +// Package utils provides utility functions for commands, logging, and common operations. +package utils //nolint:revive,nolintlint // apparently nolintlint is confused // Contains functions helpful for running commands. diff --git a/internal/utils/commands_test.go b/internal/utils/commands_test.go index 533889d4..7cfbc293 100644 --- a/internal/utils/commands_test.go +++ b/internal/utils/commands_test.go @@ -1,4 +1,4 @@ -package utils +package utils //nolint:revive,nolintlint // apparently nolintlint is confused import ( "bytes" diff --git a/internal/utils/docker.go b/internal/utils/docker.go index 2d38bfba..f2871ac1 100644 --- a/internal/utils/docker.go +++ b/internal/utils/docker.go @@ -1,4 +1,4 @@ -package utils +package utils //nolint:revive,nolintlint // apparently nolintlint is confused import ( "context" diff --git a/internal/utils/events/dummy_test.go b/internal/utils/events/dummy_test.go index 77c195bb..9f62b9fb 100644 --- a/internal/utils/events/dummy_test.go +++ b/internal/utils/events/dummy_test.go @@ -1,9 +1,10 @@ +// Package events provides event processing utilities. package events 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 } diff --git a/internal/utils/events/events.go b/internal/utils/events/events.go index 100f3b53..0f911296 100644 --- a/internal/utils/events/events.go +++ b/internal/utils/events/events.go @@ -20,10 +20,10 @@ func (o byFirstTimestamp) Len() int { return len(o) } func (o byFirstTimestamp) Swap(i, j int) { o[i], o[j] = o[j], o[i] } func (o byFirstTimestamp) Less(i, j int) bool { - if o[i].ObjectMeta.CreationTimestamp.Equal(&o[j].ObjectMeta.CreationTimestamp) { + if o[i].CreationTimestamp.Equal(&o[j].CreationTimestamp) { return o[i].Name < o[j].Name } - return o[i].ObjectMeta.CreationTimestamp.Before(&o[j].ObjectMeta.CreationTimestamp) + return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp) } // byFirstTimestampV1 sorts a slice of eventsv1 by first timestamp, using their involvedObject's name as a tie breaker. @@ -33,10 +33,10 @@ func (o byFirstTimestampV1) Len() int { return len(o) } func (o byFirstTimestampV1) Swap(i, j int) { o[i], o[j] = o[j], o[i] } func (o byFirstTimestampV1) Less(i, j int) bool { - if o[i].ObjectMeta.CreationTimestamp.Equal(&o[j].ObjectMeta.CreationTimestamp) { + if o[i].CreationTimestamp.Equal(&o[j].CreationTimestamp) { return o[i].Name < o[j].Name } - return o[i].ObjectMeta.CreationTimestamp.Before(&o[j].ObjectMeta.CreationTimestamp) + return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp) } // byFirstTimestampCoreV1 sorts a slice of corev1 by first timestamp, using their involvedObject's name as a tie breaker. @@ -46,10 +46,10 @@ func (o byFirstTimestampCoreV1) Len() int { return len(o) } func (o byFirstTimestampCoreV1) Swap(i, j int) { o[i], o[j] = o[j], o[i] } func (o byFirstTimestampCoreV1) Less(i, j int) bool { - if o[i].ObjectMeta.CreationTimestamp.Equal(&o[j].ObjectMeta.CreationTimestamp) { + if o[i].CreationTimestamp.Equal(&o[j].CreationTimestamp) { return o[i].Name < o[j].Name } - return o[i].ObjectMeta.CreationTimestamp.Before(&o[j].ObjectMeta.CreationTimestamp) + return o[i].CreationTimestamp.Before(&o[j].CreationTimestamp) } // CollectAndLog retrieves event resources for a given namespace and logs them with the logger. @@ -124,7 +124,7 @@ func printEventsBeta1(events []eventsbeta1.Event, logger testutils.Logger) { for _, e := range events { // time type regarding action reason note reportingController related logger.Logf("%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s", - e.ObjectMeta.CreationTimestamp, + e.CreationTimestamp, e.Type, shortString(&e.Regarding), e.Action, @@ -139,7 +139,7 @@ func printEventsV1(events []eventsv1.Event, logger testutils.Logger) { for _, e := range events { // time type regarding action reason note reportingController related logger.Logf("%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s", - e.ObjectMeta.CreationTimestamp, + e.CreationTimestamp, e.Type, shortString(&e.Regarding), e.Action, @@ -154,7 +154,7 @@ func printEventsCoreV1(events []corev1.Event, logger testutils.Logger) { for _, e := range events { // time type regarding action reason note reportingController related logger.Logf("%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s", - e.ObjectMeta.CreationTimestamp, + e.CreationTimestamp, e.Type, shortString(&e.InvolvedObject), e.Action, diff --git a/internal/utils/files/files.go b/internal/utils/files/files.go index 6a919c60..02265186 100644 --- a/internal/utils/files/files.go +++ b/internal/utils/files/files.go @@ -1,3 +1,4 @@ +// Package files provides file utility functions for KUTTL. package files import ( diff --git a/internal/utils/logger.go b/internal/utils/logger.go index 17b1603e..78f7f1f1 100644 --- a/internal/utils/logger.go +++ b/internal/utils/logger.go @@ -1,4 +1,4 @@ -package utils +package utils //nolint:revive,nolintlint // apparently nolintlint is confused import ( "bytes" @@ -67,6 +67,7 @@ func (t *TestLogger) Write(p []byte) (n int, err error) { return len(p), nil } +// Flush outputs any buffered log messages to the underlying test logger. func (t *TestLogger) Flush() { if len(t.buffer) != 0 { t.Log(string(t.buffer)) diff --git a/internal/utils/subset.go b/internal/utils/subset.go index 610f3cd9..7968aa5e 100644 --- a/internal/utils/subset.go +++ b/internal/utils/subset.go @@ -1,4 +1,4 @@ -package utils +package utils //nolint:revive,nolintlint // apparently nolintlint is confused import ( "fmt" diff --git a/internal/utils/subset_test.go b/internal/utils/subset_test.go index 1b6ddfb7..15ec1017 100644 --- a/internal/utils/subset_test.go +++ b/internal/utils/subset_test.go @@ -1,4 +1,4 @@ -package utils +package utils //nolint:revive,nolintlint // apparently nolintlint is confused import ( "testing" diff --git a/internal/utils/testing.go b/internal/utils/testing.go index e9b2b6e4..1cc18235 100644 --- a/internal/utils/testing.go +++ b/internal/utils/testing.go @@ -1,4 +1,4 @@ -package utils +package utils //nolint:revive,nolintlint // apparently nolintlint is confused import ( "flag" diff --git a/internal/version/base.go b/internal/version/base.go index 2f0c6f10..3a5ec763 100644 --- a/internal/version/base.go +++ b/internal/version/base.go @@ -1,3 +1,4 @@ +// Package version provides version information for the application. package version // Base version information. diff --git a/pkg/apis/dummy_test.go b/pkg/apis/dummy_test.go index dc745322..181c297f 100644 --- a/pkg/apis/dummy_test.go +++ b/pkg/apis/dummy_test.go @@ -3,7 +3,7 @@ package apis 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 } diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index 2ff215cb..61c1417b 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -17,6 +17,7 @@ var ( errRefNotSpecified = errors.New("ref not specified") ) +// BuildResourceReference constructs a NamespacedName and unstructured resource from the TestResourceRef. func (t *TestResourceRef) BuildResourceReference() (namespacedName types.NamespacedName, referencedResource *unstructured.Unstructured) { referencedResource = &unstructured.Unstructured{} apiVersionSplit := strings.Split(t.APIVersion, "/") @@ -37,6 +38,7 @@ func (t *TestResourceRef) BuildResourceReference() (namespacedName types.Namespa return } +// Validate checks that all required fields in TestResourceRef are properly set. func (t *TestResourceRef) Validate() error { apiVersionSplit := strings.Split(t.APIVersion, "/") switch { diff --git a/pkg/apis/testharness/v1beta1/test_types.go b/pkg/apis/testharness/v1beta1/test_types.go index 285dc0f3..2a246481 100644 --- a/pkg/apis/testharness/v1beta1/test_types.go +++ b/pkg/apis/testharness/v1beta1/test_types.go @@ -6,10 +6,13 @@ import ( "k8s.io/client-go/rest" ) +// KubeconfigLoadingEager specifies that kubeconfig should be loaded eagerly. const KubeconfigLoadingEager = "Eager" + +// KubeconfigLoadingLazy specifies that kubeconfig should be loaded lazily. const KubeconfigLoadingLazy = "Lazy" -// Create embedded struct to implement custom DeepCopyInto method +// RestConfig embeds rest.Config to implement custom DeepCopyInto method. type RestConfig struct { RC *rest.Config } @@ -236,6 +239,7 @@ type TestCollector struct { Cmd string `json:"command,omitempty"` } +// TestResourceRef defines a reference to a Kubernetes resource for testing. type TestResourceRef struct { APIVersion string `json:"apiVersion,omitempty"` Kind string `json:"kind,omitempty"` @@ -244,6 +248,7 @@ type TestResourceRef struct { Ref string `json:"ref,omitempty"` } +// Assertion defines a test assertion using CEL expressions. type Assertion struct { CELExpression string `json:"celExpr,omitempty"` } @@ -251,6 +256,7 @@ type Assertion struct { // DefaultKINDContext defines the default kind context to use. const DefaultKINDContext = "kind" +// DeepCopyInto creates a deep copy of the RestConfig. func (in *RestConfig) DeepCopyInto(out *RestConfig) { out.RC = rest.CopyConfig(in.RC) } diff --git a/pkg/test/harness.go b/pkg/test/harness.go index 7a193ee8..d97ade47 100644 --- a/pkg/test/harness.go +++ b/pkg/test/harness.go @@ -1,3 +1,4 @@ +// Package test provides a public API for KUTTL test harness functionality. package test import "github.com/kudobuilder/kuttl/internal/harness" @@ -5,4 +6,5 @@ import "github.com/kudobuilder/kuttl/internal/harness" // This type alias is here to avoid breaking the only apparent active user of kuttl Go API, // https://github.com/kube-green/kube-green/blob/main/tests/integration/kuttl_test.go +// Harness provides a type alias for harness.Harness to maintain backward compatibility. type Harness = harness.Harness