From d5de91168ce9a8cd98c441aa5f6fcabd7be20f5f Mon Sep 17 00:00:00 2001 From: Patrick Hamann Date: Tue, 12 May 2020 14:51:24 +0100 Subject: [PATCH] Verify fastly crate version during compute build. (#67) * Verify fastly crate version is up-to-date during compute build. * Fix typo Co-authored-by: Adam C. Foltzer * Change fastly crate update remediation error message. * Add more logic crate update messaging to cover edge cases. * Add more robust tests for crate verification logic * Apply suggestions from code review Co-authored-by: Peter Bourgon Co-authored-by: Adam C. Foltzer Co-authored-by: Peter Bourgon --- pkg/app/run.go | 2 +- pkg/compute/build.go | 28 ++--- pkg/compute/compute_test.go | 155 +++++++++++++++++++---- pkg/compute/rust.go | 171 +++++++++++++++++++++++++- pkg/compute/testdata/build/Cargo.lock | 5 +- pkg/compute/testdata/build/Cargo.toml | 1 + pkg/testutil/assert.go | 23 ++++ 7 files changed, 341 insertions(+), 44 deletions(-) diff --git a/pkg/app/run.go b/pkg/app/run.go index 954c6fd49..20565924b 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -93,7 +93,7 @@ func Run(args []string, env config.Environment, file config.File, configFilePath computeRoot := compute.NewRootCommand(app, &globals) computeInit := compute.NewInitCommand(computeRoot.CmdClause, &globals) - computeBuild := compute.NewBuildCommand(computeRoot.CmdClause, &globals) + computeBuild := compute.NewBuildCommand(computeRoot.CmdClause, httpClient, &globals) computeDeploy := compute.NewDeployCommand(computeRoot.CmdClause, httpClient, &globals) computeUpdate := compute.NewUpdateCommand(computeRoot.CmdClause, httpClient, &globals) computeValidate := compute.NewValidateCommand(computeRoot.CmdClause, &globals) diff --git a/pkg/compute/build.go b/pkg/compute/build.go index ec5c14994..d6783df55 100644 --- a/pkg/compute/build.go +++ b/pkg/compute/build.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" + "github.com/fastly/cli/pkg/api" "github.com/fastly/cli/pkg/common" "github.com/fastly/cli/pkg/compute/manifest" "github.com/fastly/cli/pkg/config" @@ -20,16 +21,6 @@ type Toolchain interface { Build(out io.Writer, verbose bool) error } -// GetToolchain returns a Toolchain for the provided language. -func GetToolchain(l string) (Toolchain, error) { - switch l { - case "rust": - return &Rust{}, nil - default: - return nil, fmt.Errorf("unsupported language %s", l) - } -} - func createPackageArchive(files []string, destination string) error { tar := archiver.NewTarGz() tar.OverwriteExisting = true // @@ -47,14 +38,16 @@ func createPackageArchive(files []string, destination string) error { // BuildCommand produces a deployable artifact from files on the local disk. type BuildCommand struct { common.Base - name string - lang string + client api.HTTPClient + name string + lang string } // NewBuildCommand returns a usable command registered under the parent. -func NewBuildCommand(parent common.Registerer, globals *config.Data) *BuildCommand { +func NewBuildCommand(parent common.Registerer, client api.HTTPClient, globals *config.Data) *BuildCommand { var c BuildCommand c.Globals = globals + c.client = client c.CmdClause = parent.Command("build", "Build a Compute@Edge package locally") c.CmdClause.Flag("name", "Package name").StringVar(&c.name) c.CmdClause.Flag("language", "Language type").StringVar(&c.lang) @@ -108,9 +101,12 @@ func (c *BuildCommand) Exec(in io.Reader, out io.Writer) (err error) { } name = sanitize.BaseName(name) - toolchain, err := GetToolchain(lang) - if err != nil { - return err + var toolchain Toolchain + switch lang { + case "rust": + toolchain = &Rust{c.client} + default: + return fmt.Errorf("unsupported language %s", lang) } progress.Step(fmt.Sprintf("Verifying local %s toolchain...", lang)) diff --git a/pkg/compute/compute_test.go b/pkg/compute/compute_test.go index 216ca5f3a..62563bd9c 100644 --- a/pkg/compute/compute_test.go +++ b/pkg/compute/compute_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/fastly/cli/pkg/api" @@ -274,39 +275,75 @@ func TestBuild(t *testing.T) { } for _, testcase := range []struct { - name string - args []string - manifest string - wantError string - wantOutputContains string + name string + args []string + fastlyManifest string + cargoManifest string + cargoLock string + client api.HTTPClient + wantError string + wantRemediationError string + wantOutputContains string }{ { name: "no fastly.toml manifest", args: []string{"compute", "build"}, + client: versionClient{[]string{"0.0.0"}}, wantError: "error reading package manifest: open fastly.toml:", // actual message differs on Windows }, { - name: "empty language", - args: []string{"compute", "build"}, - manifest: "name = \"test\"\n", - wantError: "language cannot be empty, please provide a language", + name: "empty language", + args: []string{"compute", "build"}, + fastlyManifest: "name = \"test\"\n", + client: versionClient{[]string{"0.0.0"}}, + wantError: "language cannot be empty, please provide a language", }, { - name: "empty name", - args: []string{"compute", "build"}, - manifest: "language = \"rust\"\n", - wantError: "name cannot be empty, please provide a name", + name: "empty name", + args: []string{"compute", "build"}, + fastlyManifest: "language = \"rust\"\n", + client: versionClient{[]string{"0.0.0"}}, + wantError: "name cannot be empty, please provide a name", }, { - name: "unknown language", - args: []string{"compute", "build"}, - manifest: "name = \"test\"\nlanguage = \"javascript\"\n", - wantError: "unsupported language javascript", + name: "unknown language", + args: []string{"compute", "build"}, + fastlyManifest: "name = \"test\"\nlanguage = \"javascript\"\n", + client: versionClient{[]string{"0.0.0"}}, + wantError: "unsupported language javascript", + }, + { + name: "fastly crate not found", + args: []string{"compute", "build"}, + fastlyManifest: "name = \"test\"\nlanguage = \"rust\"\n", + cargoLock: " ", + client: versionClient{[]string{"0.4.0"}}, + wantError: "fastly crate not found", + wantRemediationError: "fastly = \"^0.4.0\"", + }, + { + name: "fastly crate out-of-date but within minor range", + args: []string{"compute", "build"}, + fastlyManifest: "name = \"test\"\nlanguage = \"rust\"\n", + cargoLock: "[[package]]\nname = \"fastly\"\nversion = \"0.3.2\"", + client: versionClient{[]string{"0.3.3"}}, + wantError: "fastly crate not up-to-date", + wantRemediationError: "cargo update -p fastly", + }, + { + name: "fastly crate out-of-date but lower than minor range", + args: []string{"compute", "build"}, + fastlyManifest: "name = \"test\"\nlanguage = \"rust\"\n", + cargoLock: "[[package]]\nname = \"fastly\"\nversion = \"0.3.2\"", + client: versionClient{[]string{"0.4.0"}}, + wantError: "fastly crate not up-to-date", + wantRemediationError: "fastly = \"^0.4.0\"", }, { name: "success", args: []string{"compute", "build"}, - manifest: "name = \"test\"\nlanguage = \"rust\"\n", + fastlyManifest: "name = \"test\"\nlanguage = \"rust\"\n", + client: versionClient{[]string{"0.0.0"}}, wantOutputContains: "Built rust package test", }, } { @@ -320,7 +357,7 @@ func TestBuild(t *testing.T) { // Create our build environment in a temp dir. // Defer a call to clean it up. - rootdir := makeBuildEnvironment(t, testcase.manifest) + rootdir := makeBuildEnvironment(t, testcase.fastlyManifest, testcase.cargoManifest, testcase.cargoLock) defer os.RemoveAll(rootdir) // Before running the test, chdir into the build environment. @@ -337,7 +374,7 @@ func TestBuild(t *testing.T) { file = config.File{} appConfigFile = "/dev/null" clientFactory = mock.APIClient(mock.API{}) - httpClient = http.DefaultClient + httpClient = testcase.client versioner update.Versioner = nil in io.Reader = nil buf bytes.Buffer @@ -345,6 +382,7 @@ func TestBuild(t *testing.T) { ) err = app.Run(args, env, file, appConfigFile, clientFactory, httpClient, versioner, in, out) testutil.AssertErrorContains(t, err, testcase.wantError) + testutil.AssertRemediationErrorContains(t, err, testcase.wantRemediationError) if testcase.wantOutputContains != "" { testutil.AssertStringContains(t, buf.String(), testcase.wantOutputContains) } @@ -965,6 +1003,44 @@ func TestGetIdealPackage(t *testing.T) { } } +func TestGetLatestCrateVersion(t *testing.T) { + for _, testcase := range []struct { + name string + inputClient api.HTTPClient + wantVersion string + wantError string + }{ + { + name: "http error", + inputClient: &errorClient{errTest}, + wantError: "fixture error", + }, + { + name: "no valid versions", + inputClient: &versionClient{[]string{}}, + wantError: "no valid crate versions found", + }, + { + name: "unsorted", + inputClient: &versionClient{[]string{"0.5.23", "0.1.0", "1.2.3", "0.7.3"}}, + wantVersion: "1.2.3", + }, + { + name: "reverse chronological", + inputClient: &versionClient{[]string{"1.2.3", "0.8.3", "0.3.2"}}, + wantVersion: "1.2.3", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + v, err := compute.GetLatestCrateVersion(testcase.inputClient, "fastly") + testutil.AssertErrorContains(t, err, testcase.wantError) + if v != testcase.wantVersion { + t.Errorf("wanted version %s, got %s", testcase.wantVersion, v) + } + }) + } +} + func makeInitEnvironment(t *testing.T) (rootdir string) { t.Helper() @@ -986,7 +1062,7 @@ func makeInitEnvironment(t *testing.T) (rootdir string) { return rootdir } -func makeBuildEnvironment(t *testing.T, manifestContent string) (rootdir string) { +func makeBuildEnvironment(t *testing.T, fastlyManifestContent, cargoManifestContent, cargoLockContent string) (rootdir string) { t.Helper() p := make([]byte, 8) @@ -1014,9 +1090,23 @@ func makeBuildEnvironment(t *testing.T, manifestContent string) (rootdir string) copyFile(t, fromFilename, toFilename) } - if manifestContent != "" { + if fastlyManifestContent != "" { filename := filepath.Join(rootdir, compute.ManifestFilename) - if err := ioutil.WriteFile(filename, []byte(manifestContent), 0777); err != nil { + if err := ioutil.WriteFile(filename, []byte(fastlyManifestContent), 0777); err != nil { + t.Fatal(err) + } + } + + if cargoManifestContent != "" { + filename := filepath.Join(rootdir, "Cargo.toml") + if err := ioutil.WriteFile(filename, []byte(cargoManifestContent), 0777); err != nil { + t.Fatal(err) + } + } + + if cargoLockContent != "" { + filename := filepath.Join(rootdir, "Cargo.lock") + if err := ioutil.WriteFile(filename, []byte(cargoLockContent), 0777); err != nil { t.Fatal(err) } } @@ -1230,3 +1320,22 @@ func (c codeClient) Do(*http.Request) (*http.Response, error) { rec.WriteHeader(c.code) return rec.Result(), nil } + +type versionClient struct { + versions []string +} + +func (v versionClient) Do(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + + var versions []string + for _, vv := range v.versions { + versions = append(versions, fmt.Sprintf(`{"num":"%s"}`, vv)) + } + + _, err := rec.Write([]byte(fmt.Sprintf(`{"versions":[%s]}`, strings.Join(versions, ",")))) + if err != nil { + return nil, err + } + return rec.Result(), nil +} diff --git a/pkg/compute/rust.go b/pkg/compute/rust.go index 183ada035..3884422cc 100644 --- a/pkg/compute/rust.go +++ b/pkg/compute/rust.go @@ -3,8 +3,11 @@ package compute import ( "bufio" "bytes" + "encoding/json" "fmt" "io" + "io/ioutil" + "net/http" "os" "os/exec" "path/filepath" @@ -12,6 +15,8 @@ import ( "sync" "github.com/BurntSushi/toml" + "github.com/blang/semver" + "github.com/fastly/cli/pkg/api" "github.com/fastly/cli/pkg/common" "github.com/fastly/cli/pkg/errors" "github.com/fastly/cli/pkg/text" @@ -26,9 +31,11 @@ const ( ) // CargoPackage models the package confuiguration properties of a Rust Cargo -// package which we are interested in and is embedded within CargoManifest. +// package which we are interested in and is embedded within CargoManifest and +// CargoLock. type CargoPackage struct { - Name string `toml:"name"` + Name string `toml:"name"` + Version string `toml:"version"` } // CargoManifest models the package confuiguration properties of a Rust Cargo @@ -44,8 +51,23 @@ func (m *CargoManifest) Read(filename string) error { return err } +// CargoLock models the package confuiguration properties of a Rust Cargo +// lock file which we are interested in and are read from the Cargo.lock file +// within the $PWD of the package. +type CargoLock struct { + Package []CargoPackage +} + +// Read the contents of the Cargo.lock file from filename. +func (m *CargoLock) Read(filename string) error { + _, err := toml.DecodeFile(filename, m) + return err +} + // Rust is an implments Toolchain for the Rust lanaguage. -type Rust struct{} +type Rust struct { + client api.HTTPClient +} // Verify implments the Toolchain interface and verifies whether the Rust // language toolchain is correctly configured on the host. @@ -148,6 +170,90 @@ func (r Rust) Verify(out io.Writer) error { fmt.Fprintf(out, "Found Cargo.toml at %s\n", fpath) + // 5) Verify `fastly` crate version + // + // A valid and up-to-date version of the fastly crate is required. + fpath, err = filepath.Abs("Cargo.lock") + if err != nil { + return fmt.Errorf("error getting Cargo.lock path: %w", err) + } + + if !common.FileExists(fpath) { + return fmt.Errorf("%s not found", fpath) + } + + var lock CargoLock + if err := lock.Read("Cargo.lock"); err != nil { + return fmt.Errorf("error reading Cargo.lock file: %w", err) + } + + // Fetch the latest crate version from cargo.io API. + l, err := GetLatestCrateVersion(r.client, "fastly") + if err != nil { + return fmt.Errorf("error fetching latest crate version: %w", err) + } + latest, err := semver.Parse(l) + if err != nil { + return fmt.Errorf("error parsing latest crate SemVer: %w", err) + } + + // Find the crate version declared in Cargo.lock lockfile. + var crate CargoPackage + for _, p := range lock.Package { + if p.Name == "fastly" { + crate = p + break + } + } + + // If crate not found in lockfile, error with dual remediation steps. + if crate.Name == "" { + return errors.RemediationError{ + Inner: fmt.Errorf("fastly crate not found"), + Remediation: fmt.Sprintf( + "To fix this error, edit %s with:\n\n\t %s\n\nAnd then run the following command:\n\n\t$ %s\n", + text.Bold("Cargo.toml"), + text.Bold(fmt.Sprintf(`fastly = "^%s"`, latest)), + text.Bold("cargo update -p fastly"), + ), + } + } + + // Parse lockfile version to semver. + version, err := semver.Parse(crate.Version) + if err != nil { + return fmt.Errorf("error parsing Cargo.lock file: %w", err) + } + + // Parse the lowest minor semver for the latest release. + // I.e. v0.3.2 becomes v0.3.0. + latestMinor, err := semver.Parse(fmt.Sprintf("%d.%d.0", latest.Major, latest.Minor)) + if err != nil { + return fmt.Errorf("error parsing Cargo.lock file: %w", err) + } + + // If the lockfile version is within the minor range but lower than the + // latest, error with remediation to run `cargo update`. + if version.GTE(latestMinor) && version.LT(latest) { + return errors.RemediationError{ + Inner: fmt.Errorf("fastly crate not up-to-date"), + Remediation: fmt.Sprintf("To fix this error, run the following command:\n\n\t$ %s\n", text.Bold("cargo update -p fastly")), + } + } + + // If version is on a lower minor or major, error with a dual remediation. + if version.LT(latest) { + return errors.RemediationError{ + Inner: fmt.Errorf("fastly crate not up-to-date"), + Remediation: fmt.Sprintf( + "To fix this error, edit %s with:\n\n\t %s\n\nAnd then run the following command:\n\n\t$ %s\n", + text.Bold("Cargo.toml"), + text.Bold(fmt.Sprintf(`fastly = "^%s"`, latest)), + text.Bold("cargo update -p fastly"), + ), + } + } + return nil } @@ -264,3 +370,62 @@ func (r Rust) Build(out io.Writer, verbose bool) error { return nil } + +// CargoCrateVersion models a Cargo crate version returned by the crates.io API. +type CargoCrateVersion struct { + Version string `json:"num"` +} + +// CargoCrateVersions models a Cargo crate version returned by the crates.io API. +type CargoCrateVersions struct { + Versions []CargoCrateVersion `json:"versions"` +} + +// GetLatestCrateVersion fetches all versions of a given Rust crate from the +// crates.io HTTP API and returns the latest valid semver version. +func GetLatestCrateVersion(client api.HTTPClient, name string) (string, error) { + url := fmt.Sprintf("https://crates.io/api/v1/crates/%s/versions", name) + + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + resp, err := client.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("error fetching latest crate version: %s", resp.Status) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + crate := CargoCrateVersions{} + err = json.Unmarshal(body, &crate) + if err != nil { + return "", err + } + + var versions []semver.Version + for _, v := range crate.Versions { + if version, err := semver.Parse(v.Version); err == nil { + versions = append(versions, version) + } + } + + if len(versions) < 1 { + return "", fmt.Errorf("no valid crate versions found") + } + + semver.Sort(versions) + + latest := versions[len(versions)-1] + + return latest.String(), nil +} diff --git a/pkg/compute/testdata/build/Cargo.lock b/pkg/compute/testdata/build/Cargo.lock index f472ff32b..6e68d8c84 100644 --- a/pkg/compute/testdata/build/Cargo.lock +++ b/pkg/compute/testdata/build/Cargo.lock @@ -1,6 +1,9 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. [[package]] -name = "edge-compute-default-rust-template" +name = "fastly-template-default-rust" version = "0.1.0" +[[package]] +name = "fastly" +version = "0.3.2" diff --git a/pkg/compute/testdata/build/Cargo.toml b/pkg/compute/testdata/build/Cargo.toml index efb0f1738..c01be340e 100644 --- a/pkg/compute/testdata/build/Cargo.toml +++ b/pkg/compute/testdata/build/Cargo.toml @@ -8,3 +8,4 @@ edition = "2018" debug = true [dependencies] +fastly = "^0.3.2" diff --git a/pkg/testutil/assert.go b/pkg/testutil/assert.go index e22236396..a4cd6e15c 100644 --- a/pkg/testutil/assert.go +++ b/pkg/testutil/assert.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "github.com/fastly/cli/pkg/errors" "github.com/google/go-cmp/cmp" ) @@ -57,3 +58,25 @@ func AssertErrorContains(t *testing.T, err error, target string) { } } } + +// AssertRemediationErrorContains fatals a test if the error's RemediationError +// remediation string doesn't contain target. As a special case, if target is +// the empty string, we assume the error should be nil. +func AssertRemediationErrorContains(t *testing.T, err error, target string) { + t.Helper() + + re, ok := err.(errors.RemediationError) + + switch { + case err == nil && target == "": + return // great + case err == nil && target != "": + t.Fatalf("want %q, have no error", target) + case err != nil && target != "" && !ok: + t.Fatal("have no RemediationError") + case err != nil && target != "": + if want, have := target, re.Remediation; !strings.Contains(have, want) { + t.Fatalf("want %q, have %q", want, have) + } + } +}