diff --git a/.builder-image-version.txt b/.builder-image-version.txt index 73a29c94c..13fec6724 100644 --- a/.builder-image-version.txt +++ b/.builder-image-version.txt @@ -1 +1 @@ -1.1.34 +1.1.35 diff --git a/.github/workflows/pr-lint.yml b/.github/workflows/pr-lint.yml index e62fbc020..ca4f5b37a 100644 --- a/.github/workflows/pr-lint.yml +++ b/.github/workflows/pr-lint.yml @@ -21,7 +21,7 @@ jobs: if: github.event_name != 'pull_request' || !github.event.pull_request.draft runs-on: ubuntu-latest container: - image: ghcr.io/sovereigncloudstack/cso-builder:1.1.34 + image: ghcr.io/sovereigncloudstack/cso-builder:1.1.35 credentials: username: ${{ github.actor }} password: ${{ secrets.github_token }} diff --git a/.github/workflows/schedule-scan-image.yml b/.github/workflows/schedule-scan-image.yml index 386428ac0..b9c7be427 100644 --- a/.github/workflows/schedule-scan-image.yml +++ b/.github/workflows/schedule-scan-image.yml @@ -9,7 +9,7 @@ jobs: name: Trivy runs-on: ubuntu-latest container: - image: ghcr.io/sovereigncloudstack/cso-builder:1.1.34 + image: ghcr.io/sovereigncloudstack/cso-builder:1.1.35 credentials: username: ${{ github.actor }} password: ${{ secrets.github_token }} diff --git a/.golangci.yaml b/.golangci.yaml index 70936465a..4ccfb5a3a 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -225,6 +225,11 @@ linters: - linters: - wrapcheck path: _test\.go + # pkg/version is public API, renaming it would break importers. + - linters: + - revive + path: pkg/version/ + text: avoid package names that conflict with Go standard library package names paths: - vendor$ - third_party$ diff --git a/internal/clusterstackrelease/util.go b/internal/clusterstackrelease/util.go index 99ffd1c19..23f282e19 100644 --- a/internal/clusterstackrelease/util.go +++ b/internal/clusterstackrelease/util.go @@ -55,6 +55,8 @@ func Summary(csr *csov1alpha1.ClusterStackRelease) (csov1alpha1.ClusterStackRele summary.Phase = csov1alpha1.ClusterStackReleasePhaseProviderSpecificWork case conditions.IsFalse(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition): summary.Phase = csov1alpha1.ClusterStackReleasePhaseDownloadingAssets + default: + // None of the conditions is set (yet), the phase stays as it is. } return summary, nil diff --git a/internal/controller/clusteraddon_controller.go b/internal/controller/clusteraddon_controller.go index e31b66c30..2a14c32b8 100644 --- a/internal/controller/clusteraddon_controller.go +++ b/internal/controller/clusteraddon_controller.go @@ -529,7 +529,7 @@ func (r *ClusterAddonReconciler) getNewReleaseObjects(ctx context.Context, in *t } } - helmTemplate, err := helmTemplateClusterAddon(filepath.Join(in.newDestinationClusterAddonChartDir, stage.Name), newBuildTemplate, in.kubernetesVersion) + helmTemplate, err := helmTemplateClusterAddon(ctx, filepath.Join(in.newDestinationClusterAddonChartDir, stage.Name), newBuildTemplate, in.kubernetesVersion) if err != nil { return nil, fmt.Errorf("failed to template new helm chart of the latest cluster stack: %w", err) } @@ -568,7 +568,7 @@ func (r *ClusterAddonReconciler) getOldReleaseObjects(ctx context.Context, in *t return nil, fmt.Errorf("failed to build template from the old cluster stack cluster addon values: %w", err) } - helmTemplate, err := helmTemplateClusterAddon(oldRelease.ClusterAddonChartPath(), buildTemplate, oldRelease.Meta.Versions.Kubernetes) + helmTemplate, err := helmTemplateClusterAddon(ctx, oldRelease.ClusterAddonChartPath(), buildTemplate, oldRelease.Meta.Versions.Kubernetes) if err != nil { return nil, fmt.Errorf("failed to template helm chart: %w", err) } @@ -605,7 +605,7 @@ func (r *ClusterAddonReconciler) getOldReleaseObjects(ctx context.Context, in *t } } - helmTemplate, err := helmTemplateClusterAddon(filepath.Join(in.oldDestinationClusterAddonChartDir, stage.Name), newBuildTemplate, oldRelease.Meta.Versions.Kubernetes) + helmTemplate, err := helmTemplateClusterAddon(ctx, filepath.Join(in.oldDestinationClusterAddonChartDir, stage.Name), newBuildTemplate, oldRelease.Meta.Versions.Kubernetes) if err != nil { return nil, fmt.Errorf("failed to template new helm chart: %w", err) } @@ -746,7 +746,7 @@ func (r *ClusterAddonReconciler) templateAndApplyClusterAddonHelmChart(ctx conte return false, fmt.Errorf("failed to build template from cluster addon values: %w", err) } - helmTemplate, err := helmTemplateClusterAddon(clusterAddonChart, buildTemplate, in.kubernetesVersion) + helmTemplate, err := helmTemplateClusterAddon(ctx, clusterAddonChart, buildTemplate, in.kubernetesVersion) if err != nil { return false, fmt.Errorf("failed to template helm chart: %w", err) } @@ -863,7 +863,7 @@ check: } // Delete part logger.V(1).Info("starting to template helm chart", "clusterStack", in.clusterAddon.Spec.ClusterStack, "name", stage.Name, "hook", in.clusterAddon.Spec.Hook) - helmTemplate, err := helmTemplateNewClusterStack(in, stage.Name) + helmTemplate, err := helmTemplateNewClusterStack(ctx, in, stage.Name) if err != nil { conditions.MarkFalse( in.clusterAddon, @@ -931,6 +931,9 @@ check: } in.clusterAddon.SetStagePhase(stage.Name, stage.Action, csov1alpha1.StagePhaseDone) + + default: + // Other phases (e.g. Done) need no action here. } return false, nil @@ -1038,7 +1041,7 @@ func (r *ClusterAddonReconciler) templateNewClusterStackAddonHelmChart(ctx conte return true, nil, nil, nil } - oldHelmTemplate, err = helmTemplateClusterAddon(oldClusterStackSubDirPath, oldBuildTemplate, in.oldKubernetesVersion) + oldHelmTemplate, err = helmTemplateClusterAddon(ctx, oldClusterStackSubDirPath, oldBuildTemplate, in.oldKubernetesVersion) if err != nil { conditions.MarkFalse( in.clusterAddon, @@ -1082,7 +1085,7 @@ func (r *ClusterAddonReconciler) templateNewClusterStackAddonHelmChart(ctx conte } } - newHelmTemplate, err = helmTemplateClusterAddon(newClusterStackSubDirPath, newBuildTemplate, in.kubernetesVersion) + newHelmTemplate, err = helmTemplateClusterAddon(ctx, newClusterStackSubDirPath, newBuildTemplate, in.kubernetesVersion) if err != nil { conditions.MarkFalse( in.clusterAddon, @@ -1105,11 +1108,11 @@ func (r *ClusterAddonReconciler) templateNewClusterStackAddonHelmChart(ctx conte return false, oldHelmTemplate, newHelmTemplate, nil } -func helmTemplateNewClusterStack(in *templateAndApplyClusterAddonInput, name string) (newTemplate []byte, err error) { +func helmTemplateNewClusterStack(ctx context.Context, in *templateAndApplyClusterAddonInput, name string) (newTemplate []byte, err error) { var buildTemplate []byte newClusterStackSubDirPath := filepath.Join(in.newDestinationClusterAddonChartDir, name) - newHelmTemplate, err := helmTemplateClusterAddon(newClusterStackSubDirPath, buildTemplate, in.kubernetesVersion) + newHelmTemplate, err := helmTemplateClusterAddon(ctx, newClusterStackSubDirPath, buildTemplate, in.kubernetesVersion) if err != nil { return nil, fmt.Errorf("failed to template new helm chart: %w", err) } @@ -1252,16 +1255,18 @@ func buildTemplateFromClusterAddonValues(ctx context.Context, addonValuePath str // Then it returns the path of the generated yaml file. // Example: helm template /tmp/downloads/cluster-stacks/myprovider-myclusterstack-1-26-v2/myprovider-myclusterstack-1-26-v2.tgz // The return yaml file path will be /tmp/downloads/cluster-stacks/myprovider-myclusterstack-1-26-v2/myprovider-myclusterstack-1-26-v2.tgz.yaml. -func helmTemplateClusterAddon(chartPath string, helmTemplate []byte, kubernetesVersion string) ([]byte, error) { +func helmTemplateClusterAddon(ctx context.Context, chartPath string, helmTemplate []byte, kubernetesVersion string) ([]byte, error) { helmCommand := "helm" - helmArgs := []string{"template", "--include-crds", "--dependency-update"} + helmArgs := []string{ + "template", "--include-crds", "--dependency-update", + "--kube-version", kubernetesVersion, "cluster-addon", filepath.Base(chartPath), "--namespace", clusterAddonNamespace, "-f", "-", + } input := bytes.NewBuffer(helmTemplate) var cmdOutput bytes.Buffer - helmArgs = append(helmArgs, "--kube-version", kubernetesVersion, "cluster-addon", filepath.Base(chartPath), "--namespace", clusterAddonNamespace, "-f", "-") - helmTemplateCmd := exec.Command(helmCommand, helmArgs...) + helmTemplateCmd := exec.CommandContext(ctx, helmCommand, helmArgs...) //nolint:gosec // G204: fixed command "helm", the arguments come from the downloaded release helmTemplateCmd.Stderr = os.Stderr helmTemplateCmd.Dir = filepath.Dir(chartPath) helmTemplateCmd.Stdout = &cmdOutput @@ -1366,6 +1371,11 @@ func unTarContent(src, dst string) error { return fmt.Errorf("reading tar archive: %w", err) } + // Do not extract entries that would end up outside of the destination directory. + if !filepath.IsLocal(header.Name) { + return fmt.Errorf("%q: tar entry points outside of the destination directory", header.Name) + } + // Calculate the target file path targetPath := filepath.Join(dst, header.Name) // #nosec @@ -1381,7 +1391,7 @@ func unTarContent(src, dst string) error { return fmt.Errorf("%q: creating directory: %w", filepath.Dir(targetPath), err) } - outputFile, err := os.Create(filepath.Clean(targetPath)) + outputFile, err := os.Create(filepath.Clean(targetPath)) //nolint:gosec // G703: header.Name is checked with filepath.IsLocal above if err != nil { return fmt.Errorf("%q: creating file: %w", targetPath, err) } diff --git a/internal/controller/clusterstack_controller_test.go b/internal/controller/clusterstack_controller_test.go index 69c0c813a..76900af5b 100644 --- a/internal/controller/clusterstack_controller_test.go +++ b/internal/controller/clusterstack_controller_test.go @@ -18,6 +18,7 @@ package controller import ( "reflect" + "slices" "sort" "testing" @@ -330,7 +331,7 @@ func TestGetUsableClusterStackReleaseVersions(t *testing.T) { expectedVersions := []string{"v1", "v3"} // Check if the result matches the expected versions - if !equalStringSlices(usableVersions, expectedVersions) { + if !slices.Equal(usableVersions, expectedVersions) { t.Errorf("Expected versions %v, but got %v", expectedVersions, usableVersions) } }) @@ -348,7 +349,7 @@ func TestGetUsableClusterStackReleaseVersions(t *testing.T) { expectedVersions := []string{} // Check if the result matches the expected versions - if !equalStringSlices(usableVersions, expectedVersions) { + if !slices.Equal(usableVersions, expectedVersions) { t.Errorf("Expected versions %v, but got %v", expectedVersions, usableVersions) } }) @@ -379,18 +380,6 @@ func TestGetClusterStackReleasesInSpec(t *testing.T) { } } -func equalStringSlices(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i := range a { - if a[i] != b[i] { - return false - } - } - return true -} - func toString(arr1 []*csov1alpha1.ClusterStackRelease) []string { arr2 := make([]string, 0, len(arr1)) for _, v := range arr1 { diff --git a/internal/controller/clusterstackrelease_controller.go b/internal/controller/clusterstackrelease_controller.go index 107a6e5f8..cd844656d 100644 --- a/internal/controller/clusterstackrelease_controller.go +++ b/internal/controller/clusterstackrelease_controller.go @@ -224,7 +224,7 @@ func (r *ClusterStackReleaseReconciler) reconcileDelete(ctx context.Context, rel } } - template, err := r.templateClusterClassHelmChart(releaseAssets, clusterStackReleaseCR.Name, clusterStackReleaseCR.Namespace) + template, err := r.templateClusterClassHelmChart(ctx, releaseAssets, clusterStackReleaseCR.Name, clusterStackReleaseCR.Namespace) if err != nil { return reconcile.Result{}, fmt.Errorf("failed to perform helm template: %w", err) } @@ -297,7 +297,7 @@ func (r *ClusterStackReleaseReconciler) updateProviderClusterStackRelease(ctx co func (r *ClusterStackReleaseReconciler) templateAndApply(ctx context.Context, releaseAssets *release.Release, clusterStackRelease *csov1alpha1.ClusterStackRelease, kubeClient kube.Client) (bool, error) { // template helm chart and apply objects - template, err := r.templateClusterClassHelmChart(releaseAssets, clusterStackRelease.Name, clusterStackRelease.Namespace) + template, err := r.templateClusterClassHelmChart(ctx, releaseAssets, clusterStackRelease.Name, clusterStackRelease.Namespace) if err != nil { return false, fmt.Errorf("failed to template clusterClass helm chart: %w", err) } @@ -318,7 +318,7 @@ func (r *ClusterStackReleaseReconciler) templateAndApply(ctx context.Context, re } // templateClusterClassHelmChart templates the clusterClass helm chart. -func (*ClusterStackReleaseReconciler) templateClusterClassHelmChart(releaseAssets *release.Release, name, namespace string) ([]byte, error) { +func (*ClusterStackReleaseReconciler) templateClusterClassHelmChart(ctx context.Context, releaseAssets *release.Release, name, namespace string) ([]byte, error) { clusterClassChart, e := releaseAssets.ClusterClassChartPath() if e != nil { return nil, fmt.Errorf("failed to template clusterClass helm chart: %w", e) @@ -327,7 +327,7 @@ func (*ClusterStackReleaseReconciler) templateClusterClassHelmChart(releaseAsset splittedName := strings.Split(name, clusterstack.Separator) releaseName := strings.Join(splittedName[0:4], clusterstack.Separator) - template, err := helmTemplate(clusterClassChart, releaseName, namespace) + template, err := helmTemplate(ctx, clusterClassChart, releaseName, namespace) if err != nil { return nil, fmt.Errorf("failed to template clusterClass helm chart: %w", err) } @@ -335,14 +335,13 @@ func (*ClusterStackReleaseReconciler) templateClusterClassHelmChart(releaseAsset return template, nil } -func helmTemplate(chartPath, releaseName, namespace string) ([]byte, error) { +func helmTemplate(ctx context.Context, chartPath, releaseName, namespace string) ([]byte, error) { helmCommand := "helm" - helmArgs := []string{"template"} + helmArgs := []string{"template", releaseName, filepath.Base(chartPath), "--namespace", namespace} var cmdOutput bytes.Buffer - helmArgs = append(helmArgs, releaseName, filepath.Base(chartPath), "--namespace", namespace) - helmTemplateCmd := exec.Command(helmCommand, helmArgs...) + helmTemplateCmd := exec.CommandContext(ctx, helmCommand, helmArgs...) //nolint:gosec // G204: fixed command "helm", the arguments come from the downloaded release helmTemplateCmd.Stderr = os.Stderr helmTemplateCmd.Dir = filepath.Dir(chartPath) helmTemplateCmd.Stdout = &cmdOutput diff --git a/internal/controller/untar_test.go b/internal/controller/untar_test.go new file mode 100644 index 000000000..f22846e99 --- /dev/null +++ b/internal/controller/untar_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2023 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. +*/ + +package controller + +import ( + "archive/tar" + "compress/gzip" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func writeTestArchive(t *testing.T, path string, files map[string]string) { + t.Helper() + + file, err := os.Create(filepath.Clean(path)) + if err != nil { + t.Fatalf("creating archive: %s", err) + } + + gzipWriter := gzip.NewWriter(file) + tarWriter := tar.NewWriter(gzipWriter) + + for name, content := range files { + header := &tar.Header{ + Name: name, + Typeflag: tar.TypeReg, + Mode: 0o600, + Size: int64(len(content)), + } + if err := tarWriter.WriteHeader(header); err != nil { + t.Fatalf("writing tar header: %s", err) + } + if _, err := tarWriter.Write([]byte(content)); err != nil { + t.Fatalf("writing tar content: %s", err) + } + } + + if err := tarWriter.Close(); err != nil { + t.Fatalf("closing tar writer: %s", err) + } + if err := gzipWriter.Close(); err != nil { + t.Fatalf("closing gzip writer: %s", err) + } + if err := file.Close(); err != nil { + t.Fatalf("closing archive: %s", err) + } +} + +func TestUnTarContent(t *testing.T) { + t.Run("extracts the files into the destination", func(t *testing.T) { + tmp := t.TempDir() + archive := filepath.Join(tmp, "chart.tgz") + dst := filepath.Join(tmp, "dst") + writeTestArchive(t, archive, map[string]string{"chart/Chart.yaml": "name: chart\n"}) + + assert.NoError(t, unTarContent(archive, dst)) + + content, err := os.ReadFile(filepath.Clean(filepath.Join(dst, "chart", "Chart.yaml"))) + assert.NoError(t, err) + assert.Equal(t, "name: chart\n", string(content)) + }) + + t.Run("rejects entries that would be written outside of the destination", func(t *testing.T) { + tmp := t.TempDir() + archive := filepath.Join(tmp, "evil.tgz") + dst := filepath.Join(tmp, "dst") + writeTestArchive(t, archive, map[string]string{"../escaped.txt": "x"}) + + assert.Error(t, unTarContent(archive, dst)) + assert.NoFileExists(t, filepath.Join(tmp, "escaped.txt")) + }) +} diff --git a/internal/test/helpers/webhook.go b/internal/test/helpers/webhook.go index 35f84b2c9..2947a8f4f 100644 --- a/internal/test/helpers/webhook.go +++ b/internal/test/helpers/webhook.go @@ -17,6 +17,7 @@ limitations under the License. package helpers import ( + "context" "fmt" "net" "os" @@ -111,9 +112,10 @@ func (*TestEnvironment) WaitForWebhooks() { klog.V(2).Infof("Waiting for webhook port %d to be open prior to running tests", port) timeout := 1 * time.Second + dialer := net.Dialer{Timeout: timeout} for { time.Sleep(1 * time.Second) - conn, err := net.DialTimeout("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port)), timeout) + conn, err := dialer.DialContext(context.Background(), "tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port))) if err != nil { klog.V(2).Infof("Webhook port is not ready, will retry in %v: %s", timeout, err) continue diff --git a/pkg/assetsclient/github/client.go b/pkg/assetsclient/github/client.go index 4a7e88c51..f5a835724 100644 --- a/pkg/assetsclient/github/client.go +++ b/pkg/assetsclient/github/client.go @@ -155,7 +155,7 @@ func (c *realGhClient) handleRedirect(ctx context.Context, url string, assetFile return fmt.Errorf("failed to define http get request: %w", err) } - resp, err := c.httpclient.Do(req) + resp, err := c.httpclient.Do(req) //nolint:gosec // G704: url is the download location the GitHub API returned for the release asset if err != nil { return fmt.Errorf("failed to get URL %q: %w", url, err) } diff --git a/pkg/kube/helpers.go b/pkg/kube/helpers.go index c9fa762bb..c813e9dd3 100644 --- a/pkg/kube/helpers.go +++ b/pkg/kube/helpers.go @@ -172,7 +172,11 @@ func setLabel(target *unstructured.Unstructured, key, val string) error { return fmt.Errorf("error setting labels on .spec.selector for apps/extensions group: %w", err) } } + default: + // Other API versions have no selector to adjust. } + default: + // Other kinds have no pod template to label. } case "batch": if gvk.Kind == JobKind { @@ -189,6 +193,8 @@ func setLabel(target *unstructured.Unstructured, key, val string) error { return fmt.Errorf("error setting labels under spec.template for batch group: %w", err) } } + default: + // Other groups have no pod template to label. } return nil } diff --git a/pkg/version/version.go b/pkg/version/version.go index 4e05d1adb..e1079af26 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -201,7 +201,11 @@ func (csv Version) Compare(input Version) (int, error) { return -1, nil case csv.Patch == input.Patch: return 0, nil + default: + // Unreachable: Patch is neither greater, smaller nor equal. } + default: + // Unreachable: Major is neither greater, smaller nor equal. } return 0, nil