Fix Integration tests (#2425)

* fix(ci): Bump golangci-lint to 1.51.1

* chore(lint): fix gofmt and goimport issues

* chore(lint): fix linter issues

- Adapted error comparison according to linter recommendation
- Disabled noctx linting for http request where canceling makes no sense
- Disabled nilerror linting where nil error is returned on purpose
- Disabled makezero linter where slice is explicitly deepcopied

* chore(ci): Update go version in tests workflows

* fix(ci): Allow boilerplate years from 2000-2099

Previously the regex only allowed the copyright notice to contain the
years 2018,2019,2020,2021, or 2022. This commit widens to regex to
20\d\d allowing any year in the range [2000-2099]

* feat(ci): Replace minikube with k3s for intregration tests

The existing setup for minikube is very complicated, replicating most of
the setup steps for a full kubernetes cluster in an only partially
supported minikube configuration (driver=none). Furthermore the existing
setup has been broken for sometime, likely, at least in part due to the
changes to CNI and CRI in recent kubernetes versions.

Since what we actually need is only a running Kubernetes cluster on the
node and access to a registry on localhost:5000, we can switch the
extremely complicated minikube setup for a lightweight cluster using
k3s. Minikube came with a default addon for running a registry on every
node, but the same is not the case for k3s, instead we make use of the
package helm controller and its HelmChart CR to deploy twuni/docker-registry.helm
and expose it on localhost using the integrated LoadBalancer controller.

* fix(test-684): pin base container version

The dockerfile for the regression test connected to issue 684 used a
rolling tag as base image, making it flaky and fail since it was
introduced.

This commit pins the base image to the digest of bionic-20200219, which,
based on the date of the commit that introduced to the dockerfile would
be the most newest ubuntu build and likely what the "rolling" tag
resolved to back then. Since this also an image from the pre-oci days of
ubuntu, this circumvents a bug in container-diff as well
(GoogleContainerTools/container-diff#389)

.github/workflows/integration-tests.yaml

@@ -27,9 +27,9 @@ jobs:
     steps:
     - uses: actions/setup-go@b22fbbc2921299758641fab08929b4ac52b32923 # v3
       with:
-        go-version: 1.17
+        go-version: '1.20'
     - uses: actions/checkout@b0e28b5ac45a892f91e7d036f8200cf5ed489415 # v3
     - uses: docker/setup-buildx-action@dc7b9719a96d48369863986a06765841d7ea23f6 # v1
 
-    - run: make install-container-diff minikube-setup
+    - run: make install-container-diff k3s-setup
     - run: make ${{ matrix.make-target }}

.github/workflows/unit-tests.yaml

@@ -15,7 +15,7 @@ jobs:
     steps:
     - uses: actions/setup-go@b22fbbc2921299758641fab08929b4ac52b32923 # v3
       with:
-        go-version: 1.17
+        go-version: '1.20'
     - uses: actions/checkout@b0e28b5ac45a892f91e7d036f8200cf5ed489415 # v3
 
     - run: make test

Makefile

@@ -58,9 +58,9 @@ install-container-diff:
 	@ curl -LO https://github.com/GoogleContainerTools/container-diff/releases/download/v0.17.0/container-diff-linux-amd64 && \
 		chmod +x container-diff-linux-amd64 && sudo mv container-diff-linux-amd64 /usr/local/bin/container-diff
 
-.PHONY: minikube-setup
-minikube-setup:
-	@ ./scripts/minikube-setup.sh
+.PHONY: k3s-setup
+k3s-setup:
+	@ ./scripts/k3s-setup.sh
 
 .PHONY: test
 test: out/executor

cmd/executor/cmd/root.go

@@ -446,7 +446,7 @@ func exit(err error) {
 	exitWithCode(err, 1)
 }
 
-//exits with the given error and exit code
+// exits with the given error and exit code
 func exitWithCode(err error, exitCode int) {
 	fmt.Println(err)
 	os.Exit(exitCode)

hack/boilerplate/boilerplate.py

@@ -95,7 +95,7 @@ def file_passes(filename, refs, regexs):
         if p.search(d):
             return False
 
-    # Replace all occurrences of the regex "2017|2016|2015|2014" with "YEAR"
+    # Replace all occurrences of the date regex with "YEAR"
     p = regexs["date"]
     for i, d in enumerate(data):
         (data[i], found) = p.subn('YEAR', d)
@@ -146,8 +146,8 @@ def get_regexs():
     regexs = {}
     # Search for "YEAR" which exists in the boilerplate, but shouldn't in the real thing
     regexs["year"] = re.compile( 'YEAR' )
-    # dates can be 2018, 2019, 2020 company holder names can be anything
-    regexs["date"] = re.compile( '(2018|2019|2020|2021|2022)' )
+    # dates can be any year [2000-2099] company holder names can be anything
+    regexs["date"] = re.compile( '(20\d\d)' )
     # strip // go:build \n\n build constraints
     regexs["go_build_constraints_go"] = re.compile(r"^(//go\:build.*)+\n", re.MULTILINE)
     # strip // +build \n\n build constraints

hack/linter.sh

@@ -21,7 +21,7 @@ BIN=${DIR}/bin
 
 if [ ! -x "${BIN}/golangci-lint" ]; then
 	echo "Installing GolangCI-Lint"
-	"${DIR}/install_golint.sh" -b "${BIN}" v1.23.7
+	"${DIR}/install_golint.sh" -b "${BIN}" v1.51.1
 fi
 
 "${BIN}/golangci-lint" run

integration/dockerfiles/Dockerfile_test_issue_684

@@ -1,3 +1,4 @@
-FROM ubuntu:rolling as builder
+# ubuntu:bionic-20200219
+FROM ubuntu@sha256:04d48df82c938587820d7b6006f5071dbbffceb7ca01d2814f81857c631d44df as builder
 
 RUN apt-get update && apt-get -y upgrade && apt-get -y install lib32stdc++6 wget

integration/integration_test.go

@@ -282,15 +282,17 @@ func testGitBuildcontextHelper(t *testing.T, repo string) {
 
 // TestGitBuildcontext explicitly names the main branch
 // Example:
-//   git://github.com/myuser/repo#refs/heads/main
+//
+//	git://github.com/myuser/repo#refs/heads/main
 func TestGitBuildcontext(t *testing.T) {
 	repo := getGitRepo(false)
 	testGitBuildcontextHelper(t, repo)
 }
 
 // TestGitBuildcontextNoRef builds without any commit / branch reference
 // Example:
-//   git://github.com/myuser/repo
+//
+//	git://github.com/myuser/repo
 func TestGitBuildcontextNoRef(t *testing.T) {
 	t.Skip("Docker's behavior is to assume a 'master' branch, which the Kaniko repo doesn't have")
 	_, _, url := getBranchCommitAndURL()
@@ -299,7 +301,8 @@ func TestGitBuildcontextNoRef(t *testing.T) {
 
 // TestGitBuildcontextExplicitCommit uses an explicit commit hash instead of named reference
 // Example:
-//   git://github.com/myuser/repo#b873088c4a7b60bb7e216289c58da945d0d771b6
+//
+//	git://github.com/myuser/repo#b873088c4a7b60bb7e216289c58da945d0d771b6
 func TestGitBuildcontextExplicitCommit(t *testing.T) {
 	repo := getGitRepo(true)
 	testGitBuildcontextHelper(t, repo)
@@ -711,18 +714,18 @@ func TestExitCodePropagation(t *testing.T) {
 }
 
 type fileDiff struct {
-	Name string
-	Size int
+	Name string `json:"Name"`
+	Size int    `json:"Size"`
 }
 
 type fileDiffResult struct {
-	Adds []fileDiff
-	Dels []fileDiff
+	Adds []fileDiff `json:"Adds"`
+	Dels []fileDiff `json:"Dels"`
 }
 
 type metaDiffResult struct {
-	Adds []string
-	Dels []string
+	Adds []string `json:"Adds"`
+	Dels []string `json:"Dels"`
 }
 
 type diffOutput struct {

pkg/buildcontext/https.go

@@ -49,7 +49,7 @@ func (h *HTTPSTar) UnpackTarFromBuildContext() (directory string, err error) {
 
 	// Download tar file from remote https server
 	// and save it into the target tar file
-	resp, err := http.Get(h.context)
+	resp, err := http.Get(h.context) //nolint:noctx
 	if err != nil {
 		return
 	}

pkg/cache/errors.go

@@ -16,15 +16,13 @@ limitations under the License.
 
 package cache
 
+import "errors"
+
 // IsAlreadyCached returns true if the supplied error is of the type AlreadyCachedErr
 // otherwise it returns false.
 func IsAlreadyCached(err error) bool {
-	switch err.(type) {
-	case AlreadyCachedErr:
-		return true
-	}
-
-	return false
+	var e AlreadyCachedErr
+	return errors.As(err, &e)
 }
 
 // AlreadyCachedErr is returned when the Docker image requested for caching is already
@@ -39,13 +37,9 @@ func (a AlreadyCachedErr) Error() string {
 
 // IsNotFound returns true if the supplied error is of the type NotFoundErr
 // otherwise it returns false.
-func IsNotFound(e error) bool {
-	switch e.(type) {
-	case NotFoundErr:
-		return true
-	}
-
-	return false
+func IsNotFound(err error) bool {
+	var e NotFoundErr
+	return errors.As(err, &e)
 }
 
 // NotFoundErr is returned when the requested Docker image is not present in the cache.
@@ -59,13 +53,9 @@ func (e NotFoundErr) Error() string {
 
 // IsExpired returns true if the supplied error is of the type ExpiredErr
 // otherwise it returns false.
-func IsExpired(e error) bool {
-	switch e.(type) {
-	case ExpiredErr:
-		return true
-	}
-
-	return false
+func IsExpired(err error) bool {
+	var e ExpiredErr
+	return errors.As(err, &e)
 }
 
 // ExpiredErr is returned when the requested Docker image is present in the cache, but is

pkg/commands/add.go

@@ -38,12 +38,12 @@ type AddCommand struct {
 
 // ExecuteCommand executes the ADD command
 // Special stuff about ADD:
-// 	1. If <src> is a remote file URL:
-// 		- destination will have permissions of 0600
-// 		- If remote file has HTTP Last-Modified header, we set the mtime of the file to that timestamp
-// 		- If dest doesn't end with a slash, the filepath is inferred to be <dest>/<filename>
-// 	2. If <src> is a local tar archive:
-// 		- it is unpacked at the dest, as 'tar -x' would
+//  1. If <src> is a remote file URL:
+//     - destination will have permissions of 0600
+//     - If remote file has HTTP Last-Modified header, we set the mtime of the file to that timestamp
+//     - If dest doesn't end with a slash, the filepath is inferred to be <dest>/<filename>
+//  2. If <src> is a local tar archive:
+//     - it is unpacked at the dest, as 'tar -x' would
 func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 

pkg/commands/cmd_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/copy_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (
@@ -374,7 +375,7 @@ func Test_resolveIfSymlink(t *testing.T) {
 	for i, c := range cases {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 			res, e := resolveIfSymlink(c.destPath)
-			if e != c.err {
+			if !errors.Is(e, c.err) {
 				t.Errorf("%s: expected %v but got %v", c.destPath, c.err, e)
 			}
 

pkg/commands/entrypoint_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/env_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/onbuild.go

@@ -28,7 +28,7 @@ type OnBuildCommand struct {
 	cmd *instructions.OnbuildCommand
 }
 
-//ExecuteCommand adds the specified expression in Onbuild to the config
+// ExecuteCommand adds the specified expression in Onbuild to the config
 func (o *OnBuildCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	logrus.Info("Cmd: ONBUILD")
 	logrus.Infof("Args: %s", o.cmd.Expression)

pkg/commands/run_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/shell_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/stopsignal_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/user_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/volume.go

@@ -54,7 +54,7 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
 		if _, err := os.Stat(volume); os.IsNotExist(err) {
 			logrus.Infof("Creating directory %s", volume)
 			if err := os.MkdirAll(volume, 0755); err != nil {
-				return fmt.Errorf("could not create directory for volume %s: %s", volume, err)
+				return fmt.Errorf("could not create directory for volume %s: %w", volume, err)
 			}
 		}
 	}

pkg/commands/volume_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/commands/workdir_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package commands
 
 import (

pkg/dockerfile/buildargs.go

@@ -55,7 +55,8 @@ func (b *BuildArgs) ReplacementEnvs(envs []string) []string {
 	resultEnv := make([]string, len(envs))
 	copy(resultEnv, envs)
 	filtered := b.FilterAllowed(envs)
-	return append(resultEnv, filtered...)
+	// Disable makezero linter, since the previous make is paired with a same sized copy
+	return append(resultEnv, filtered...) //nolint:makezero
 }
 
 // AddMetaArgs adds the supplied args map to b's allowedMetaArgs

pkg/dockerfile/dockerfile.go

@@ -40,7 +40,7 @@ func ParseStages(opts *config.KanikoOptions) ([]instructions.Stage, []instructio
 	var d []uint8
 	match, _ := regexp.MatchString("^https?://", opts.DockerfilePath)
 	if match {
-		response, e := http.Get(opts.DockerfilePath)
+		response, e := http.Get(opts.DockerfilePath) //nolint:noctx
 		if e != nil {
 			return nil, nil, e
 		}

pkg/executor/copy_multistage_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package executor
 
 import (

pkg/executor/push_test.go

@@ -127,7 +127,7 @@ func TestHeaderAdded(t *testing.T) {
 				os.Setenv("UPSTREAM_CLIENT_TYPE", test.upstream)
 				defer func() { os.Unsetenv("UPSTREAM_CLIENT_TYPE") }()
 			}
-			req, err := http.NewRequest("GET", "dummy", nil)
+			req, err := http.NewRequest("GET", "dummy", nil) //nolint:noctx
 			if err != nil {
 				t.Fatalf("culd not create a req due to %s", err)
 			}

pkg/snapshot/layered_map_test.go

@@ -13,6 +13,7 @@ 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.
 */
+
 package snapshot
 
 import (

pkg/snapshot/snapshot.go

@@ -17,6 +17,7 @@ limitations under the License.
 package snapshot
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -262,7 +263,7 @@ func addParentDirectories(t util.Tar, addedPaths map[string]bool, path string) e
 // filesWithLinks returns the symlink and the target path if its exists.
 func filesWithLinks(path string) ([]string, error) {
 	link, err := util.GetSymLink(path)
-	if err == util.ErrNotSymLink {
+	if errors.Is(err, util.ErrNotSymLink) {
 		return []string{path}, nil
 	} else if err != nil {
 		return nil, err
@@ -272,7 +273,7 @@ func filesWithLinks(path string) ([]string, error) {
 		link = filepath.Join(filepath.Dir(path), link)
 	}
 	if _, err := os.Stat(link); err != nil {
-		return []string{path}, nil
+		return []string{path}, nil //nolint:nilerr
 	}
 	return []string{path, link}, nil
 }