Skip to content

Commit

Permalink
fix(compute/deploy): check package before service clone (#1026)
Browse files Browse the repository at this point in the history 
* doc(compute/hashsum): add FIXME as a reminder for next major release

* fix(compute/deploy): check package before service clone

* test: add mock package API call

* refactor(compute/deploy): address feedback
  • Loading branch information
@Integralist
Integralist authored Oct 6, 2023
1 parent 05f3c00 commit 349975b
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 70 deletions.
119 changes: 49 additions & 70 deletions pkg/commands/compute/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) {
return err
}

newService, serviceID, serviceVersion, cont, err := serviceManagement(serviceID, source, c, in, out, fnActivateTrial, spinner)
newService, serviceID, serviceVersion, cont, err := serviceManagement(pkgPath, serviceID, source, c, in, out, fnActivateTrial, spinner)
if err != nil {
return err
}
Expand Down Expand Up @@ -146,15 +146,15 @@ func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) {
return err
}

cont, err = processPackage(
c, pkgPath, serviceID, serviceVersion.Number, spinner, out,
)
err = pkgUpload(spinner, c.Globals.APIClient, serviceID, serviceVersion.Number, pkgPath)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Package path": pkgPath,
"Service ID": serviceID,
"Service Version": serviceVersion,
})
return err
}
if !cont {
return nil
}

if err = processService(c, serviceID, serviceVersion.Number, spinner); err != nil {
return err
Expand Down Expand Up @@ -443,7 +443,7 @@ func preconfigureActivateTrial(endpoint, token string, httpClient api.HTTPClient
}

func serviceManagement(
serviceID string,
pkgPath, serviceID string,
source manifest.Source,
c *DeployCommand,
in io.Reader,
Expand Down Expand Up @@ -480,7 +480,36 @@ func serviceManagement(
}
}

serviceVersion, err = manageExistingServiceFlow(serviceID, c.ServiceVersion, c.Globals.APIClient, c.Globals.Verbose(), out, c.Globals.ErrLog)
serviceVersion, err = c.ServiceVersion.Parse(serviceID, c.Globals.APIClient)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Service ID": serviceID,
})
return false, "", serviceVersion, false, err
}

filesHash, err := getFilesHash(pkgPath)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Package path": pkgPath,
})
return false, "", nil, false, err
}

cont, err = pkgCompare(c.Globals.APIClient, serviceID, serviceVersion.Number, filesHash, out)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Package path": pkgPath,
"Service ID": serviceID,
"Service Version": serviceVersion,
})
return false, "", nil, false, err
}
if !cont {
return false, "", nil, false, err
}

serviceVersion, err = manageExistingServiceFlow(serviceID, serviceVersion, c.Globals.APIClient, c.Globals.Verbose(), out, c.Globals.ErrLog)
if err != nil {
return false, "", nil, false, err
}
Expand Down Expand Up @@ -712,34 +741,26 @@ func updateManifestServiceID(m *manifest.File, manifestFilename string, serviceI
// manageExistingServiceFlow clones service version if required.
func manageExistingServiceFlow(
serviceID string,
serviceVersionFlag cmd.OptionalServiceVersion,
serviceVersion *fastly.Version,
apiClient api.Interface,
verbose bool,
out io.Writer,
errLog fsterr.LogInterface,
) (serviceVersion *fastly.Version, err error) {
serviceVersion, err = serviceVersionFlag.Parse(serviceID, apiClient)
if err != nil {
errLog.AddWithContext(err, map[string]any{
"Service ID": serviceID,
})
return serviceVersion, err
}

) (*fastly.Version, error) {
// Validate that we're dealing with a Compute@Edge 'wasm' service and not a
// VCL service, for which we cannot upload a wasm package format to.
serviceDetails, err := apiClient.GetServiceDetails(&fastly.GetServiceInput{ID: serviceID})
if err != nil {
errLog.AddWithContext(err, map[string]any{
"Service ID": serviceID,
"Service Version": serviceVersion,
"Service Version": serviceVersion.Number,
})
return serviceVersion, err
}
if serviceDetails.Type != "wasm" {
errLog.AddWithContext(fmt.Errorf("error: invalid service type: '%s'", serviceDetails.Type), map[string]any{
"Service ID": serviceID,
"Service Version": serviceVersion,
"Service Version": serviceVersion.Number,
"Service Type": serviceDetails.Type,
})
return serviceVersion, fsterr.RemediationError{
Expand Down Expand Up @@ -793,19 +814,18 @@ func checkServiceID(serviceID string, client api.Interface) error {

// pkgCompare compares the local package files hash against the existing service
// package version and exits early with message if identical.
func pkgCompare(client api.Interface, serviceID string, version int, filesHash string, out io.Writer) (bool, error) {
func pkgCompare(client api.Interface, serviceID string, version int, filesHash string, out io.Writer) (cont bool, err error) {
p, err := client.GetPackage(&fastly.GetPackageInput{
ServiceID: serviceID,
ServiceVersion: version,
})

if err == nil {
if filesHash == p.Metadata.FilesHash {
text.Info(out, "Skipping package deployment, local and service version are identical. (service %v, version %v) ", serviceID, version)
return false, nil
}
if err != nil {
return false, err
}
if filesHash == p.Metadata.FilesHash {
text.Info(out, "Skipping package deployment, local and service version are identical. (service %v, version %v) ", serviceID, version)
return false, nil
}

return true, nil
}

Expand Down Expand Up @@ -1104,47 +1124,6 @@ func processSetupCreation(
return nil
}

func processPackage(
c *DeployCommand,
pkgPath, serviceID string,
serviceVersion int,
spinner text.Spinner,
out io.Writer,
) (cont bool, err error) {
filesHash, err := getFilesHash(pkgPath)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Package path": pkgPath,
})
return false, err
}

cont, err = pkgCompare(c.Globals.APIClient, serviceID, serviceVersion, filesHash, out)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Package path": pkgPath,
"Service ID": serviceID,
"Service Version": serviceVersion,
})
return false, err
}
if !cont {
return false, nil
}

err = pkgUpload(spinner, c.Globals.APIClient, serviceID, serviceVersion, pkgPath)
if err != nil {
c.Globals.ErrLog.AddWithContext(err, map[string]any{
"Package path": pkgPath,
"Service ID": serviceID,
"Service Version": serviceVersion,
})
return false, err
}

return true, nil
}

func processService(c *DeployCommand, serviceID string, serviceVersion int, spinner text.Spinner) error {
if c.Comment.WasSet {
_, err := c.Globals.APIClient.UpdateVersion(&fastly.UpdateVersionInput{
Expand Down
2 changes: 2 additions & 0 deletions pkg/commands/compute/deploy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ func TestDeploy(t *testing.T) {
CloneVersionFn: testutil.CloneVersionError,
GetServiceDetailsFn: getServiceDetailsWasm,
ListVersionsFn: testutil.ListVersions,
GetPackageFn: getPackageOk,
},
wantError: fmt.Sprintf("error cloning service version: %s", testutil.Err.Error()),
},
Expand All @@ -206,6 +207,7 @@ func TestDeploy(t *testing.T) {
GetServiceFn: getServiceOK,
ListDomainsFn: listDomainsError,
ListVersionsFn: testutil.ListVersions,
GetPackageFn: getPackageOk,
},
wantError: fmt.Sprintf("error fetching service domains: %s", testutil.Err.Error()),
},
Expand Down
1 change: 1 addition & 0 deletions pkg/commands/compute/hashsum.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func NewHashsumCommand(parent cmd.Registerer, g *global.Data, build *BuildComman
// Exec implements the command interface.
func (c *HashsumCommand) Exec(in io.Reader, out io.Writer) (err error) {
if !c.Globals.Flags.Quiet {
// FIXME: Remove `hashsum` subcommand before v11.0.0 is released.
text.Warning(out, "This command is deprecated. Use `fastly compute hash-files` instead.")
if c.Globals.Verbose() || c.SkipBuild {
text.Break(out)
Expand Down

0 comments on commit 349975b

Please sign in to comment.