Skip to content

no-jira: cluster-api: use zstd for compression #10450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
patrickdillon wants to merge 1 commit into
base: main
Choose a base branch
from
Open

no-jira: cluster-api: use zstd for compression #10450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ require (
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
github.com/klauspost/compress v1.18.0 // indirect
github.com/klauspost/compress v1.18.0
github.com/kr/fs v0.1.0 // indirect
github.com/kylelemons/godebug v1.1.0 // indirect
github.com/leodido/go-urn v1.4.0 // indirect
Expand Down
11 changes: 8 additions & 3 deletions hack/build-cluster-api.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,19 @@ copy_cluster_api_to_mirror() {
mkdir -p "${CLUSTER_API_MIRROR_DIR}"

# Clean the mirror, but preserve the README file.
rm -rf "${CLUSTER_API_MIRROR_DIR:?}/*.zip"
rm -f "${CLUSTER_API_MIRROR_DIR:?}"/*.zst "${CLUSTER_API_MIRROR_DIR:?}"/dict "${CLUSTER_API_MIRROR_DIR:?}"/*.zip

if test "${SKIP_ENVTEST}" != y; then
sync_envtest
fi

# Zip every binary in the folder into a single zip file.
zip -j1 "${CLUSTER_API_MIRROR_DIR}/cluster-api.zip" "${CLUSTER_API_BIN_DIR}"/*
# Train a zstd dictionary on all binaries for cross-binary deduplication,
# then compress each binary individually with the shared dictionary.
zstd -q --train "${CLUSTER_API_BIN_DIR}"/* -o "${CLUSTER_API_MIRROR_DIR}/dict" --maxdict=262144
for bin in "${CLUSTER_API_BIN_DIR}"/*; do
name=$(basename "$bin")
zstd -9 -D "${CLUSTER_API_MIRROR_DIR}/dict" "$bin" -o "${CLUSTER_API_MIRROR_DIR}/${name}.zst"
done
}

sync_envtest() {
Expand Down
87 changes: 28 additions & 59 deletions pkg/clusterapi/providers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,20 @@
package clusterapi

import (
"archive/zip"
"bytes"
"embed"
"fmt"
"io"
"os"
"path"
"path/filepath"
"strings"

"github.com/pkg/errors"
"github.com/klauspost/compress/zstd"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/util/sets"
)

const (
zipFile = "cluster-api.zip"
dictFile = "dict"
)

var (
Expand Down Expand Up @@ -81,82 +78,54 @@ var Mirror embed.FS

// Extract extracts the provider from the embedded data into the specified directory.
func (p Provider) Extract(dir string) error {
f, err := Mirror.Open(path.Join("mirror", zipFile))
// Load the shared dictionary.
dictData, err := Mirror.ReadFile(path.Join("mirror", dictFile))
if err != nil {
return errors.Wrap(err, "failed to open cluster api zip from mirror")
}
defer f.Close()
stat, err := f.Stat()
if err != nil {
return errors.Wrap(err, "failed to stat cluster api zip")
}
seek, ok := f.(io.ReaderAt)
if !ok {
// If the file does not support ReaderAt interface (<Go1.20)
// we need to read the whole file into memory.
b, err := io.ReadAll(f)
if err != nil {
return errors.Wrap(err, "failed to read cluster api zip")
}
seek = bytes.NewReader(b)
}

// Open a zip archive for reading.
r, err := zip.NewReader(seek, stat.Size())
if err != nil {
return errors.Wrap(err, "failed to open cluster api zip")
return fmt.Errorf("failed to read zstd dictionary from mirror: %w", err)
}

// Ensure the directory exists.
logrus.Debugf("Creating %s directory", dir)
if err := os.MkdirAll(dir, 0o777); err != nil {
return errors.Wrapf(err, "could not make directory for the %s provider", p.Name)
return fmt.Errorf("could not make directory for the %s provider: %w", p.Name, err)
}

// Extract the files.
for _, f := range r.File {
name := f.Name
nameWithoutExt := strings.TrimSuffix(name, ".exe")
if !p.Sources.Has(name) && !p.Sources.Has(nameWithoutExt) {
continue
}

path, err := sanitizeArchivePath(dir, name)
// Extract only the files needed for this provider.
for source := range p.Sources {
zstFile := source + ".zst"
f, err := Mirror.Open(path.Join("mirror", zstFile))
if err != nil {
return errors.Wrapf(err, "failed to sanitize archive file %q", name)
return fmt.Errorf("failed to open %s from mirror: %w", zstFile, err)
}
logrus.Debugf("Extracting %s file", path)
if err := unpackFile(f, path); err != nil {
return errors.Wrapf(err, "failed to extract %q", path)

destPath := filepath.Join(dir, source)
logrus.Debugf("Extracting %s", destPath)
if err := decompressFile(f, destPath, dictData); err != nil {
f.Close()
return fmt.Errorf("failed to extract %s: %w", source, err)
}
f.Close()
}
return nil
}

func unpackFile(f *zip.File, destPath string) error {
src, err := f.Open()
func decompressFile(src io.Reader, destPath string, dictData []byte) error {
decoder, err := zstd.NewReader(src, zstd.WithDecoderDicts(dictData))
if err != nil {
return errors.Wrapf(err, "failed to open file %s", f.Name)
return fmt.Errorf("failed to create zstd decoder: %w", err)
}
defer src.Close()
destFile, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o777)
defer decoder.Close()

dest, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o777)
if err != nil {
return err
}
defer destFile.Close()
if _, err := io.CopyN(destFile, src, f.FileInfo().Size()); err != nil {
return err
}
return nil
}
defer dest.Close()

func sanitizeArchivePath(d, t string) (v string, err error) {
v = filepath.Join(d, t)
if strings.HasPrefix(v, filepath.Clean(d)) {
return v, nil
if _, err := io.Copy(dest, decoder); err != nil {
return fmt.Errorf("failed to decompress: %w", err)
}
Comment on lines +119 to 127
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Restore executable permissions explicitly after decompression.

Raw .zst payloads no longer carry the mode bits the zip path used to preserve. OpenFile(..., 0o777) only affects brand-new files and relies on umask, so reruns can leave a stale non-executable mode in place and first-time extracts can be overly permissive. Since these files are executed later, set the final mode explicitly and surface Close errors.

Possible hardening 
-	dest, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o777)
+	dest, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
 	if err != nil {
 		return err
 	}
-	defer dest.Close()
 
 	if _, err := io.Copy(dest, decoder); err != nil {
-		return fmt.Errorf("failed to decompress: %w", err)
+		dest.Close()
+		return fmt.Errorf("failed to decompress %s: %w", destPath, err)
 	}
+	if err := dest.Close(); err != nil {
+		return fmt.Errorf("failed to close %s: %w", destPath, err)
+	}
+	if err := os.Chmod(destPath, 0o755); err != nil {
+		return fmt.Errorf("failed to chmod %s: %w", destPath, err)
+	}
 	return nil
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dest, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o777)
if err != nil {
return err
}
defer destFile.Close()
if _, err := io.CopyN(destFile, src, f.FileInfo().Size()); err != nil {
return err
}
return nil
}
defer dest.Close()
func sanitizeArchivePath(d, t string) (v string, err error) {
v = filepath.Join(d, t)
if strings.HasPrefix(v, filepath.Clean(d)) {
return v, nil
if _, err := io.Copy(dest, decoder); err != nil {
return fmt.Errorf("failed to decompress: %w", err)
}
dest, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
if err != nil {
return err
}
if _, err := io.Copy(dest, decoder); err != nil {
dest.Close()
return fmt.Errorf("failed to decompress %s: %w", destPath, err)
}
if err := dest.Close(); err != nil {
return fmt.Errorf("failed to close %s: %w", destPath, err)
}
if err := os.Chmod(destPath, 0o755); err != nil {
return fmt.Errorf("failed to chmod %s: %w", destPath, err)
}
return nil
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/clusterapi/providers.go` around lines 119 - 127, The decompressed file at
destPath is left with unpredictable mode bits and Close errors are ignored;
after writing with os.OpenFile (destPath, ...), ensure you explicitly set the
final executable mode (e.g., os.Chmod(destPath, 0o755) or a properly restrictive
mode) once io.Copy to dest completes, and surface any errors from dest.Close()
and os.Chmod by returning/wrapping them; reference destPath, dest (the *os.File
returned by os.OpenFile), decoder and the io.Copy/Close sequence when making the
change.


return "", fmt.Errorf("%s: %s", "content filepath is tainted", t)
return nil
}

// UnpackClusterAPIBinary unpacks the cluster-api binary from the embedded data so that it can be run to create the
Expand Down