From 7c90b25131d1572c4e92b05bf922b0686513e009 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 4 Aug 2023 08:19:32 +0200 Subject: [PATCH 1/5] Always copy PHP and legacy CLI files if they have changed Saves static hashes for each file and checks their size / hash on each run --- Makefile | 7 +- go.mod | 5 +- go.sum | 9 ++- internal/file/file.go | 101 ++++++++++++++++++++++++++++ internal/file/file_test.go | 78 +++++++++++++++++++++ internal/legacy/legacy.go | 61 +++-------------- internal/legacy/php_darwin_amd64.go | 3 + internal/legacy/php_darwin_arm64.go | 3 + internal/legacy/php_linux_amd64.go | 3 + internal/legacy/php_linux_arm.go | 3 + internal/legacy/php_linux_arm64.go | 3 + internal/legacy/php_unix.go | 8 +-- internal/legacy/php_windows.go | 15 ++++- 13 files changed, 234 insertions(+), 65 deletions(-) create mode 100644 internal/file/file.go create mode 100644 internal/file/file_test.go diff --git a/Makefile b/Makefile index 629b1e6..302764c 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,12 @@ internal/legacy/archives/cacert.pem: mkdir -p internal/legacy/archives wget https://curl.se/ca/cacert.pem -O internal/legacy/archives/cacert.pem -php: $(PHP_BINARY_PATH) +.PHONY: archive-hashes +archive-hashes: + cd internal/legacy/archives + for f in $(ls -1 | grep -v sha256); do shasum -a 256 "$f" | awk '{print $1}' > "$f".sha256; done + +php: $(PHP_BINARY_PATH) archive-hashes single: internal/legacy/archives/platform.phar php command -v goreleaser >/dev/null || go install github.com/goreleaser/goreleaser@latest diff --git a/go.mod b/go.mod index e78cf8f..9709a07 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,10 @@ require ( github.com/gofrs/flock v0.8.1 github.com/mattn/go-isatty v0.0.16 github.com/platformsh/platformify v0.1.2 + github.com/spf13/afero v1.9.5 github.com/spf13/cobra v1.6.1 github.com/spf13/viper v1.15.0 + github.com/stretchr/testify v1.8.1 github.com/wk8/go-ordered-map/v2 v2.1.7 golang.org/x/exp v0.0.0-20230321023759-10a507213a29 ) @@ -20,6 +22,7 @@ require ( github.com/Masterminds/sprig/v3 v3.2.3 // indirect github.com/bahlo/generic-list-go v0.2.0 // indirect github.com/buger/jsonparser v1.1.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/google/uuid v1.1.2 // indirect github.com/hashicorp/hcl v1.0.0 // indirect @@ -35,8 +38,8 @@ require ( github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/pelletier/go-toml/v2 v2.0.6 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/shopspring/decimal v1.2.0 // indirect - github.com/spf13/afero v1.9.3 // indirect github.com/spf13/cast v1.5.0 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/go.sum b/go.sum index 1e374b6..181dc22 100644 --- a/go.sum +++ b/go.sum @@ -206,8 +206,8 @@ github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBO github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/shopspring/decimal v1.2.0 h1:abSATXmQEYyShuxI4/vyW3tV1MrKAJzCZ/0zLUXYbsQ= github.com/shopspring/decimal v1.2.0/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= -github.com/spf13/afero v1.9.3 h1:41FoI0fD7OR7mGcKE/aOiLkGreyf8ifIOQmJANWogMk= -github.com/spf13/afero v1.9.3/go.mod h1:iUV7ddyEEZPO5gA3zD4fJt6iStLlL+Lg4m2cihcDf8Y= +github.com/spf13/afero v1.9.5 h1:stMpOSZFs//0Lv29HduCmli3GUfpFoF3Y1Q/aXj/wVM= +github.com/spf13/afero v1.9.5/go.mod h1:UBogFpq8E9Hx+xc5CNTTEpTnuHVmXDwZcZcE1eb/UhQ= github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w= github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU= @@ -260,7 +260,7 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.3.0 h1:a06MkbcxBrEFc0w0QIZWXrH/9cCX6KJyWbBOIwAn+7A= golang.org/x/crypto v0.3.0/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -330,6 +330,7 @@ golang.org/x/net v0.0.0-20201031054903-ff519b6c9102/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -386,6 +387,7 @@ golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210104204734-6f8348627aad/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -409,6 +411,7 @@ golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.5.0 h1:OLmvp0KP+FVG99Ct/qFiL/Fhk4zp4QQnZ7b2U+5piUM= diff --git a/internal/file/file.go b/internal/file/file.go new file mode 100644 index 0000000..cc2e29a --- /dev/null +++ b/internal/file/file.go @@ -0,0 +1,101 @@ +package file + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + + "github.com/spf13/afero" +) + +const hashExt = ".sha256" + +// Copy copies a file from the given bytes to destination. +func Copy(destination string, fin []byte) error { + if _, err := os.Stat(destination); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("could not stat file: %w", err) + } + + fout, err := os.Create(destination) + if err != nil { + return fmt.Errorf("could not create file: %w", err) + } + defer fout.Close() + + r := bytes.NewReader(fin) + + if _, err := io.Copy(fout, r); err != nil { + return fmt.Errorf("could copy file: %w", err) + } + + return nil +} + +// CheckSize checks if a file exists and has an exact size. +func CheckSize(filename string, size int) (bool, error) { + stat, err := os.Stat(filename) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, fmt.Errorf("could not stat file: %w", err) + } + return stat.Size() == int64(size), nil +} + +var testableFS = afero.NewOsFs() + +// CheckHash checks if a file has the given SHA256 hash. +// It supports reading the file's current hash from a static file saved next to it with the hashExt extension. +func CheckHash(filename string, hash string) (bool, error) { + if fh, err := afero.ReadFile(testableFS, filename+hashExt); err == nil { + return string(fh) == hash, nil + } + fh, err := sha256Sum(testableFS, filename) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return true, err + } + return fh == hash, nil +} + +func SaveHash(filename string, hash string) error { + return Copy(filename+hashExt, []byte(hash)) +} + +// sha256Sum calculates the SHA256 hash of a file. +func sha256Sum(fs afero.Fs, filename string) (string, error) { + f, err := fs.Open(filename) + if err != nil { + return "", err + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return "", err + } + return hex.EncodeToString(h.Sum(nil)), nil +} + +// CopyIfChanged copies source data to a destination filename if it has changed. +func CopyIfChanged(destFilename string, source []byte, sourceHash string) error { + sizeOK, err := CheckSize(destFilename, len(source)) + if err != nil { + return err + } + if sizeOK { + hashOK, err := CheckHash(destFilename, sourceHash) + if hashOK || err != nil { + return err + } + } + if err := Copy(destFilename, source); err != nil { + return err + } + return SaveHash(destFilename, sourceHash) +} diff --git a/internal/file/file_test.go b/internal/file/file_test.go new file mode 100644 index 0000000..d873597 --- /dev/null +++ b/internal/file/file_test.go @@ -0,0 +1,78 @@ +package file + +import ( + "os" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckHash(t *testing.T) { + // Temporarily swap to a memory filesystem. + testableFS = afero.NewMemMapFs() + defer func() { + testableFS = afero.NewOsFs() + }() + fs := testableFS + + mockContent := "hello world\n" + mockContentHash := "a948904f2f0f479b8f8197694b30184b0d2ed1c1cd2a1ec0fb85d299a192a447" + diffContent := "hello world?\n" + diffContentHash := "d441ffff4b6663c3f150bda9c519a58c0685e34cf13d26e881d7e004f704eeba" + + cases := []struct { + name string + content string + writeHash string + + checkHash string + shouldFail bool + }{ + { + name: "static_hash_good", + writeHash: mockContentHash, + checkHash: mockContentHash, + }, + { + name: "static_hash_bad", + writeHash: diffContentHash, + checkHash: mockContentHash, + shouldFail: true, + }, + { + name: "dynamic_hash_good", + content: diffContent, + checkHash: diffContentHash, + }, + { + name: "dynamic_hash_bad", + content: mockContent, + checkHash: diffContentHash, + shouldFail: true, + }, + } + + filename := "test" + for _, c := range cases { + require.NoError(t, removeIfExists(fs, filename)) + require.NoError(t, removeIfExists(fs, filename+hashExt)) + t.Run(c.name, func(t *testing.T) { + require.NoError(t, afero.WriteFile(fs, filename, []byte(c.content), 0644)) + if c.writeHash != "" { + require.NoError(t, afero.WriteFile(fs, filename+hashExt, []byte(c.writeHash), 0644)) + } + hashOK, err := CheckHash(filename, c.checkHash) + assert.NoError(t, err) + assert.Equal(t, !c.shouldFail, hashOK) + }) + } +} + +func removeIfExists(fs afero.Fs, filename string) error { + if err := fs.Remove(filename); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} diff --git a/internal/legacy/legacy.go b/internal/legacy/legacy.go index 39b56f4..abf96f9 100644 --- a/internal/legacy/legacy.go +++ b/internal/legacy/legacy.go @@ -1,7 +1,6 @@ package legacy import ( - "bytes" "context" _ "embed" "fmt" @@ -12,11 +11,16 @@ import ( "path" "github.com/gofrs/flock" + + "github.com/platformsh/cli/internal/file" ) //go:embed archives/platform.phar var pshCLI []byte +//go:embed archives/platform.phar.sha256 +var pshCLIHash string + var ( PSHVersion = "0.0.0" PHPVersion = "0.0.0" @@ -27,27 +31,6 @@ const prefix = "psh-go" var phpPath = fmt.Sprintf("php-%s", PHPVersion) var pshPath = fmt.Sprintf("psh-%s", PSHVersion) -// copyFile from the given bytes to destination -func copyFile(destination string, fin []byte) error { - if _, err := os.Stat(destination); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("could not stat file: %w", err) - } - - fout, err := os.Create(destination) - if err != nil { - return fmt.Errorf("could not create file: %w", err) - } - defer fout.Close() - - r := bytes.NewReader(fin) - - if _, err := io.Copy(fout, r); err != nil { - return fmt.Errorf("could copy file: %w", err) - } - - return nil -} - // CLIWrapper wraps the legacy CLI type CLIWrapper struct { Stdout io.Writer @@ -78,28 +61,11 @@ func (c *CLIWrapper) Init() error { //nolint:errcheck defer fileLock.Unlock() - if _, err := os.Stat(c.PSHPath()); os.IsNotExist(err) { - if c.CustomPshCliPath != "" { - return fmt.Errorf("given PSH phar path does not exist: %w", err) - } - - c.debugLog("PSH .phar file does not exist, copying: %s", c.PSHPath()) - if err := c.copyPSH(); err != nil { - return fmt.Errorf("could not copy files: %w", err) - } + if err := c.copyPSH(); err != nil { + return fmt.Errorf("could not copy files: %w", err) } - if _, err := os.Stat(c.PHPPath()); os.IsNotExist(err) { - c.debugLog("PHP binary does not exist, copying: %s", c.PHPPath()) - if err := c.copyPHP(); err != nil { - return fmt.Errorf("could not copy files: %w", err) - } - if err := os.Chmod(c.PHPPath(), 0o700); err != nil { - return fmt.Errorf("could not make PHP executable: %w", err) - } - } - - return nil + return c.copyPHP() } // Exec a legacy CLI command with the given arguments @@ -156,16 +122,7 @@ func (c *CLIWrapper) PSHPath() string { // copyPSH to destination, if it does not exist func (c *CLIWrapper) copyPSH() error { - // Do not copy the file, if a custom path was given - if c.CustomPshCliPath != "" { - return nil - } - - if err := copyFile(c.PSHPath(), pshCLI); err != nil { - return fmt.Errorf("could not copy legacy Platform.sh CLI: %w", err) - } - - return nil + return file.CopyIfChanged(c.PSHPath(), pshCLI, pshCLIHash) } // debugLog logs a debugging message, if debug is enabled diff --git a/internal/legacy/php_darwin_amd64.go b/internal/legacy/php_darwin_amd64.go index a13ac10..ba8d870 100644 --- a/internal/legacy/php_darwin_amd64.go +++ b/internal/legacy/php_darwin_amd64.go @@ -6,3 +6,6 @@ import ( //go:embed archives/php_darwin_amd64 var phpCLI []byte + +//go:embed archives/php_darwin_amd64.sha256 +var phpCLIHash string diff --git a/internal/legacy/php_darwin_arm64.go b/internal/legacy/php_darwin_arm64.go index 3f3e40f..5d3f107 100644 --- a/internal/legacy/php_darwin_arm64.go +++ b/internal/legacy/php_darwin_arm64.go @@ -6,3 +6,6 @@ import ( //go:embed archives/php_darwin_arm64 var phpCLI []byte + +//go:embed archives/php_darwin_arm64.sha256 +var phpCLIHash string diff --git a/internal/legacy/php_linux_amd64.go b/internal/legacy/php_linux_amd64.go index 1334de5..2a988ae 100644 --- a/internal/legacy/php_linux_amd64.go +++ b/internal/legacy/php_linux_amd64.go @@ -6,3 +6,6 @@ import ( //go:embed archives/php_linux_amd64 var phpCLI []byte + +//go:embed archives/php_linux_amd64.sha256 +var phpCLIHash string diff --git a/internal/legacy/php_linux_arm.go b/internal/legacy/php_linux_arm.go index 95190df..f38968d 100644 --- a/internal/legacy/php_linux_arm.go +++ b/internal/legacy/php_linux_arm.go @@ -6,3 +6,6 @@ import ( //go:embed archives/php_linux_arm var phpCLI []byte + +//go:embed archives/php_linux_arm.sha256 +var phpCLIHash string diff --git a/internal/legacy/php_linux_arm64.go b/internal/legacy/php_linux_arm64.go index 7cef171..0b666d4 100644 --- a/internal/legacy/php_linux_arm64.go +++ b/internal/legacy/php_linux_arm64.go @@ -6,3 +6,6 @@ import ( //go:embed archives/php_linux_arm64 var phpCLI []byte + +//go:embed archives/php_linux_arm64.sha256 +var phpCLIHash string diff --git a/internal/legacy/php_unix.go b/internal/legacy/php_unix.go index c85a6a1..8690884 100644 --- a/internal/legacy/php_unix.go +++ b/internal/legacy/php_unix.go @@ -4,17 +4,13 @@ package legacy import ( - "fmt" + "github.com/platformsh/cli/internal/file" "path" ) // copyPHP to destination, if it does not exist func (c *CLIWrapper) copyPHP() error { - if err := copyFile(c.PHPPath(), phpCLI); err != nil { - return fmt.Errorf("could not copy PHP CLI: %w", err) - } - - return nil + return file.CopyIfChanged(c.PHPPath(), phpCLI, phpCLIHash) } // PHPPath returns the path that the PHP CLI will reside diff --git a/internal/legacy/php_windows.go b/internal/legacy/php_windows.go index 352ccb7..c9d29d9 100644 --- a/internal/legacy/php_windows.go +++ b/internal/legacy/php_windows.go @@ -11,11 +11,16 @@ import ( "path/filepath" "strings" "text/template" + + "github.com/platformsh/cli/internal/file" ) //go:embed archives/php_windows.zip var phpCLI []byte +//go:embed archives/php_windows.zip.sha256 +var phpCLIHash string + //go:embed archives/windows_php.ini.tpl var phpIniTemplate string @@ -25,6 +30,9 @@ var caCert []byte // copyPHP to destination, if it does not exist func (c *CLIWrapper) copyPHP() error { dest := path.Join(c.cacheDir(), "php") + if hashOK, err := file.CheckHash(path.Join(dest, "hash"), phpCLIHash); hashOK || err != nil { + return err + } br := bytes.NewReader(phpCLI) r, err := zip.NewReader(br, int64(len(phpCLI))) if err != nil { @@ -67,9 +75,12 @@ func (c *CLIWrapper) copyPHP() error { } defer w.Close() template.Must(template.New("php.ini").Parse(phpIniTemplate)).Execute(w, map[string]string{"PSHDir": c.cacheDir()}) - copyFile(path.Join(c.cacheDir(), "php", "extras", "cacert.pem"), caCert) - return nil + if err := file.Copy(path.Join(c.cacheDir(), "php", "extras", "cacert.pem"), caCert); err != nil { + return err + } + + return file.SaveHash(path.Join(dest, "hash"), phpCLIHash) } // PHPPath returns the path that the PHP CLI will reside From f19ce9d9038a2811c0d6dd3236fac88648bf874b Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 4 Aug 2023 09:22:52 +0200 Subject: [PATCH 2/5] Pacify linter --- .github/workflows/ci.yml | 6 ++++++ internal/file/file.go | 4 ++-- internal/file/file_test.go | 4 ++-- internal/legacy/php_unix.go | 3 ++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bfae027..c449a30 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,12 @@ jobs: touch internal/legacy/archives/php_linux_arm64 touch internal/legacy/archives/php_darwin_amd64 touch internal/legacy/archives/php_darwin_arm64 + touch internal/legacy/archives/platform.phar.sha256 + touch internal/legacy/archives/php_windows_amd64.sha256 + touch internal/legacy/archives/php_linux_amd64.sha256 + touch internal/legacy/archives/php_linux_arm64.sha256 + touch internal/legacy/archives/php_darwin_amd64.sha256 + touch internal/legacy/archives/php_darwin_arm64.sha256 - name: Run linter uses: golangci/golangci-lint-action@v3 diff --git a/internal/file/file.go b/internal/file/file.go index cc2e29a..18a14ba 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -50,7 +50,7 @@ var testableFS = afero.NewOsFs() // CheckHash checks if a file has the given SHA256 hash. // It supports reading the file's current hash from a static file saved next to it with the hashExt extension. -func CheckHash(filename string, hash string) (bool, error) { +func CheckHash(filename, hash string) (bool, error) { if fh, err := afero.ReadFile(testableFS, filename+hashExt); err == nil { return string(fh) == hash, nil } @@ -64,7 +64,7 @@ func CheckHash(filename string, hash string) (bool, error) { return fh == hash, nil } -func SaveHash(filename string, hash string) error { +func SaveHash(filename, hash string) error { return Copy(filename+hashExt, []byte(hash)) } diff --git a/internal/file/file_test.go b/internal/file/file_test.go index d873597..b10c7a5 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -59,9 +59,9 @@ func TestCheckHash(t *testing.T) { require.NoError(t, removeIfExists(fs, filename)) require.NoError(t, removeIfExists(fs, filename+hashExt)) t.Run(c.name, func(t *testing.T) { - require.NoError(t, afero.WriteFile(fs, filename, []byte(c.content), 0644)) + require.NoError(t, afero.WriteFile(fs, filename, []byte(c.content), 0o644)) if c.writeHash != "" { - require.NoError(t, afero.WriteFile(fs, filename+hashExt, []byte(c.writeHash), 0644)) + require.NoError(t, afero.WriteFile(fs, filename+hashExt, []byte(c.writeHash), 0o644)) } hashOK, err := CheckHash(filename, c.checkHash) assert.NoError(t, err) diff --git a/internal/legacy/php_unix.go b/internal/legacy/php_unix.go index 8690884..c25e976 100644 --- a/internal/legacy/php_unix.go +++ b/internal/legacy/php_unix.go @@ -4,8 +4,9 @@ package legacy import ( - "github.com/platformsh/cli/internal/file" "path" + + "github.com/platformsh/cli/internal/file" ) // copyPHP to destination, if it does not exist From f0a1218f72759e2a52c462ccf12f91d084949a85 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 23 Aug 2023 14:58:23 -0400 Subject: [PATCH 3/5] file: remove afero, add memoryfs --- go.mod | 3 ++- go.sum | 2 ++ internal/file/file.go | 11 +++++------ internal/file/file_test.go | 23 +++++++---------------- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index 9709a07..c154ca5 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,9 @@ go 1.20 require ( github.com/fatih/color v1.13.0 github.com/gofrs/flock v0.8.1 + github.com/liamg/memoryfs v1.6.0 github.com/mattn/go-isatty v0.0.16 github.com/platformsh/platformify v0.1.2 - github.com/spf13/afero v1.9.5 github.com/spf13/cobra v1.6.1 github.com/spf13/viper v1.15.0 github.com/stretchr/testify v1.8.1 @@ -40,6 +40,7 @@ require ( github.com/pelletier/go-toml/v2 v2.0.6 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/shopspring/decimal v1.2.0 // indirect + github.com/spf13/afero v1.9.5 // indirect github.com/spf13/cast v1.5.0 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/go.sum b/go.sum index 181dc22..1424593 100644 --- a/go.sum +++ b/go.sum @@ -170,6 +170,8 @@ github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/liamg/memoryfs v1.6.0 h1:jAFec2HI1PgMTem5gR7UT8zi9u4BfG5jorCRlLH06W8= +github.com/liamg/memoryfs v1.6.0/go.mod h1:z7mfqXFQS8eSeBBsFjYLlxYRMRyiPktytvYCYTb3BSk= github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0VQdvPDY= github.com/magiconair/properties v1.8.7/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= diff --git a/internal/file/file.go b/internal/file/file.go index 18a14ba..345d02d 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -6,9 +6,8 @@ import ( "encoding/hex" "fmt" "io" + "io/fs" "os" - - "github.com/spf13/afero" ) const hashExt = ".sha256" @@ -46,12 +45,12 @@ func CheckSize(filename string, size int) (bool, error) { return stat.Size() == int64(size), nil } -var testableFS = afero.NewOsFs() +var testableFS = os.DirFS("/") // CheckHash checks if a file has the given SHA256 hash. // It supports reading the file's current hash from a static file saved next to it with the hashExt extension. func CheckHash(filename, hash string) (bool, error) { - if fh, err := afero.ReadFile(testableFS, filename+hashExt); err == nil { + if fh, err := fs.ReadFile(testableFS, filename+hashExt); err == nil { return string(fh) == hash, nil } fh, err := sha256Sum(testableFS, filename) @@ -69,8 +68,8 @@ func SaveHash(filename, hash string) error { } // sha256Sum calculates the SHA256 hash of a file. -func sha256Sum(fs afero.Fs, filename string) (string, error) { - f, err := fs.Open(filename) +func sha256Sum(filesystem fs.FS, filename string) (string, error) { + f, err := filesystem.Open(filename) if err != nil { return "", err } diff --git a/internal/file/file_test.go b/internal/file/file_test.go index b10c7a5..3727ba7 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -4,18 +4,18 @@ import ( "os" "testing" - "github.com/spf13/afero" + "github.com/liamg/memoryfs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestCheckHash(t *testing.T) { // Temporarily swap to a memory filesystem. - testableFS = afero.NewMemMapFs() + testableFS = memoryfs.New() defer func() { - testableFS = afero.NewOsFs() + testableFS = os.DirFS("/") }() - fs := testableFS + filesystem := testableFS.(*memoryfs.FS) //nolint:errcheck mockContent := "hello world\n" mockContentHash := "a948904f2f0f479b8f8197694b30184b0d2ed1c1cd2a1ec0fb85d299a192a447" @@ -54,14 +54,12 @@ func TestCheckHash(t *testing.T) { }, } - filename := "test" for _, c := range cases { - require.NoError(t, removeIfExists(fs, filename)) - require.NoError(t, removeIfExists(fs, filename+hashExt)) t.Run(c.name, func(t *testing.T) { - require.NoError(t, afero.WriteFile(fs, filename, []byte(c.content), 0o644)) + filename := c.name + require.NoError(t, filesystem.WriteFile(filename, []byte(c.content), 0o644)) if c.writeHash != "" { - require.NoError(t, afero.WriteFile(fs, filename+hashExt, []byte(c.writeHash), 0o644)) + require.NoError(t, filesystem.WriteFile(filename+hashExt, []byte(c.writeHash), 0o644)) } hashOK, err := CheckHash(filename, c.checkHash) assert.NoError(t, err) @@ -69,10 +67,3 @@ func TestCheckHash(t *testing.T) { }) } } - -func removeIfExists(fs afero.Fs, filename string) error { - if err := fs.Remove(filename); err != nil && !os.IsNotExist(err) { - return err - } - return nil -} From 3a43a7ca00fa5ed31095add984c1b4ca517b2765 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 23 Aug 2023 15:16:33 -0400 Subject: [PATCH 4/5] Remove unnecessary file.Copy function --- internal/file/file.go | 73 +++++++++++----------------------- internal/file/file_test.go | 2 +- internal/legacy/php_windows.go | 2 +- 3 files changed, 26 insertions(+), 51 deletions(-) diff --git a/internal/file/file.go b/internal/file/file.go index 345d02d..63c6579 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -1,56 +1,39 @@ package file import ( - "bytes" "crypto/sha256" "encoding/hex" - "fmt" "io" "io/fs" "os" ) -const hashExt = ".sha256" +const HashExt = ".sha256" -// Copy copies a file from the given bytes to destination. -func Copy(destination string, fin []byte) error { - if _, err := os.Stat(destination); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("could not stat file: %w", err) - } - - fout, err := os.Create(destination) - if err != nil { - return fmt.Errorf("could not create file: %w", err) - } - defer fout.Close() - - r := bytes.NewReader(fin) +var testableFS = os.DirFS("/") - if _, err := io.Copy(fout, r); err != nil { - return fmt.Errorf("could copy file: %w", err) +// CopyIfChanged copies source data to a destination filename if it has changed. +func CopyIfChanged(destFilename string, source []byte, sourceHash string) error { + sizeOK, err := checkSize(destFilename, len(source)) + if err != nil && !os.IsNotExist(err) { + return err } - - return nil -} - -// CheckSize checks if a file exists and has an exact size. -func CheckSize(filename string, size int) (bool, error) { - stat, err := os.Stat(filename) - if err != nil { - if os.IsNotExist(err) { - return false, nil + if sizeOK { + hashOK, err := CheckHash(destFilename, sourceHash) + if hashOK || err != nil { + return err } - return false, fmt.Errorf("could not stat file: %w", err) } - return stat.Size() == int64(size), nil + if err := os.WriteFile(destFilename, source, 0o644); err != nil { + return err + } + return SaveHash(destFilename, sourceHash) } -var testableFS = os.DirFS("/") - // CheckHash checks if a file has the given SHA256 hash. -// It supports reading the file's current hash from a static file saved next to it with the hashExt extension. +// It supports reading the file's current hash from a static file saved next to it with the HashExt extension. func CheckHash(filename, hash string) (bool, error) { - if fh, err := fs.ReadFile(testableFS, filename+hashExt); err == nil { + if fh, err := fs.ReadFile(testableFS, filename+HashExt); err == nil { return string(fh) == hash, nil } fh, err := sha256Sum(testableFS, filename) @@ -63,8 +46,9 @@ func CheckHash(filename, hash string) (bool, error) { return fh == hash, nil } +// SaveHash saves a hash alongside a file, with the same filename plus the HashExt extension. func SaveHash(filename, hash string) error { - return Copy(filename+hashExt, []byte(hash)) + return os.WriteFile(filename+HashExt, []byte(hash), 0o644) } // sha256Sum calculates the SHA256 hash of a file. @@ -81,20 +65,11 @@ func sha256Sum(filesystem fs.FS, filename string) (string, error) { return hex.EncodeToString(h.Sum(nil)), nil } -// CopyIfChanged copies source data to a destination filename if it has changed. -func CopyIfChanged(destFilename string, source []byte, sourceHash string) error { - sizeOK, err := CheckSize(destFilename, len(source)) +// checkSize checks if a file exists and has an exact size. +func checkSize(filename string, size int) (bool, error) { + stat, err := os.Stat(filename) if err != nil { - return err + return false, err } - if sizeOK { - hashOK, err := CheckHash(destFilename, sourceHash) - if hashOK || err != nil { - return err - } - } - if err := Copy(destFilename, source); err != nil { - return err - } - return SaveHash(destFilename, sourceHash) + return stat.Size() == int64(size), nil } diff --git a/internal/file/file_test.go b/internal/file/file_test.go index 3727ba7..d5e7b57 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -59,7 +59,7 @@ func TestCheckHash(t *testing.T) { filename := c.name require.NoError(t, filesystem.WriteFile(filename, []byte(c.content), 0o644)) if c.writeHash != "" { - require.NoError(t, filesystem.WriteFile(filename+hashExt, []byte(c.writeHash), 0o644)) + require.NoError(t, filesystem.WriteFile(filename+HashExt, []byte(c.writeHash), 0o644)) } hashOK, err := CheckHash(filename, c.checkHash) assert.NoError(t, err) diff --git a/internal/legacy/php_windows.go b/internal/legacy/php_windows.go index c9d29d9..8b43721 100644 --- a/internal/legacy/php_windows.go +++ b/internal/legacy/php_windows.go @@ -76,7 +76,7 @@ func (c *CLIWrapper) copyPHP() error { defer w.Close() template.Must(template.New("php.ini").Parse(phpIniTemplate)).Execute(w, map[string]string{"PSHDir": c.cacheDir()}) - if err := file.Copy(path.Join(c.cacheDir(), "php", "extras", "cacert.pem"), caCert); err != nil { + if err := os.WriteFile(path.Join(c.cacheDir(), "php", "extras", "cacert.pem"), caCert, 0o644); err != nil { return err } From eede8efcc632689faa46f601a668264d30f58772 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 23 Aug 2023 15:20:18 -0400 Subject: [PATCH 5/5] Comment about static hash --- internal/file/file.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/file/file.go b/internal/file/file.go index 63c6579..66ac60d 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -13,6 +13,8 @@ const HashExt = ".sha256" var testableFS = os.DirFS("/") // CopyIfChanged copies source data to a destination filename if it has changed. +// It is considered changed if its length or hash are different. +// The hash may be a static hash saved alongside (with HashExt) or computed dynamically. func CopyIfChanged(destFilename string, source []byte, sourceHash string) error { sizeOK, err := checkSize(destFilename, len(source)) if err != nil && !os.IsNotExist(err) {