Go code review (#8)

* init commit

* add comments

* add getter interface, serverless struct, serverless constructor, and run method, additional comments/questions

* add new package for refactoring to easily preserve old code

* start working on rehash

* add rehash function and apply to CustomizeDiff callback

* add create

* add read

* add update

* add delete

* add provider

* fix lint error

* replace old code

* rename resource back to deployment

* remove temp comments and fix typo

* refactor how serverles commands are run.  Export a public API

* remove uneeded iotas

* add type assertion error handling for Servless struct

Author: dmmulroy

serverless/resource_deployment.go

@@ -1,17 +1,9 @@
 package serverless
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
-	"encoding/json"
-	"fmt"
-	"os/exec"
-	"path/filepath"
-	"runtime"
+	"errors"
 	"strings"
 
-	"golang.org/x/mod/sumdb/dirhash"
-
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/aws/session"
@@ -20,104 +12,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 )
 
-type serverlessConfig struct {
-	Service string
-}
-
-func getServerlessConfig(configDir string, serverlessBinDir string) ([]byte, error) {
-	cmd := exec.Command(getServerlessBin(configDir, serverlessBinDir), "print", "--format", "json")
-	cmd.Dir = configDir
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		return []byte{}, fmt.Errorf("%v\n%w", string(output), err)
-	}
-
-	return output, nil
-}
-
-func getServiceName(configJson []byte) (string, error) {
-	config := serverlessConfig{}
-	err := json.Unmarshal([]byte(configJson), &config)
-
-	return config.Service, err
-}
-
-// Create a hash of the Serverless config and the Serverless zip archive.
-// Note that dirhash.HashZip ignores all zip metadata and correctly hashes
-// contents of the archive.
-func hashServerlessDir(
-	configDir string,
-	packagePath string,
-	serviceName string,
-	configJson []byte,
-) (string, error) {
-	absolutePackagePath := filepath.Join(configDir, packagePath)
-	zipPath := filepath.Join(absolutePackagePath, fmt.Sprintf("%s.zip", serviceName))
-
-	zipHash, err := dirhash.HashZip(zipPath, dirhash.Hash1)
-	if err != nil {
-		return "", err
-	}
-
-	configHashBytes := sha256.Sum256(configJson)
-	configHash := hex.EncodeToString(configHashBytes[:])
-
-	return strings.Join([]string{zipHash, configHash}, "-"), nil
-}
-
-func getServerlessBin(configDir string, binPath string) string {
-	if binPath == "" {
-		binPath = filepath.Join(configDir, "node_modules", ".bin")
-	}
-	suffix := ""
-	if runtime.GOOS == "windows" {
-		suffix = ".cmd"
-	}
-	return filepath.Join(binPath, fmt.Sprintf("serverless%s", suffix))
-}
-
-type serverlessParams struct {
-	command          string
-	serverlessBinDir string
-	configDir        string
-	packageDir       string
-	stage            string
-	args             []interface{}
-}
-
-func runServerless(params *serverlessParams) error {
-	stringArgs := make([]string, len(params.args))
-	for i, v := range stringArgs {
-		stringArgs[i] = fmt.Sprint(v)
-	}
-
-	requiredArgs := []string{
-		params.command,
-		"-s",
-		params.stage,
-	}
-
-	if params.command == "deploy" || params.command == "package" {
-		requiredArgs = append(requiredArgs, "-p", params.packageDir)
-	}
-
-	stringArgs = append(
-		requiredArgs,
-		stringArgs...,
-	)
-
-	cmd := exec.Command(getServerlessBin(params.configDir, params.serverlessBinDir), stringArgs...)
-	cmd.Dir = params.configDir
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("%v\n%w", string(output), err)
-	}
-
-	return nil
-}
-
 func resourceDeployment() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceDeploymentCreate,
@@ -167,66 +61,46 @@ func resourceDeployment() *schema.Resource {
 		},
 
 		// Only trigger a deploy if either the Serverless config or Serverless zip archive has changed.
-		// `sls package` isn't deterministic according to experiments, so in practive this means that
+		// `sls package` isn't deterministic according to experiments, so in practice this means that
 		// we only deploy after the user has run `sls package` again.
 		CustomizeDiff: customdiff.ComputedIf("package_hash", func(d *schema.ResourceDiff, meta interface{}) bool {
-			configDir := d.Get("config_dir").(string)
-			packageDir := d.Get("package_dir").(string)
-			serverlessBinDir := d.Get("serverless_bin_dir").(string)
-			currentHash := d.Get("package_hash").(string)
+			serverless, err := NewServerless(d)
 
-			configJson, err := getServerlessConfig(configDir, serverlessBinDir)
 			if err != nil {
 				return false
 			}
 
-			hash, err := hashServerlessDir(configDir, packageDir, d.Id(), configJson)
+			changed, err := serverless.Hash()
+
 			if err != nil {
 				return false
 			}
 
-			return hash != currentHash
+			return changed
 		}),
 	}
 }
 
 func resourceDeploymentCreate(d *schema.ResourceData, m interface{}) error {
-	configDir := d.Get("config_dir").(string)
-	serverlessBinDir := d.Get("serverless_bin_dir").(string)
-	packageDir := d.Get("package_dir").(string)
-	stage := d.Get("stage").(string)
-	args := d.Get("args").([]interface{})
+	serverless, err := NewServerless(d)
 
-	configJson, err := getServerlessConfig(configDir, serverlessBinDir)
 	if err != nil {
 		return err
 	}
 
-	id, err := getServiceName(configJson)
-	if err != nil {
-		return err
-	}
-	d.SetId(id)
+	serviceName, ok := serverless.config["service"].(string)
 
-	hash, err := hashServerlessDir(configDir, packageDir, id, configJson)
-	if err != nil {
-		return err
+	if !ok {
+		return errors.New("service name was not found in serverless config")
 	}
-	err = d.Set("package_hash", hash)
-	if err != nil {
+
+	d.SetId(serviceName)
+
+	if err := d.Set("package_hash", serverless.hash); err != nil {
 		return err
 	}
 
-	err = runServerless(&serverlessParams{
-		command:          "deploy",
-		serverlessBinDir: serverlessBinDir,
-		configDir:        configDir,
-		packageDir:       packageDir,
-		stage:            stage,
-		args:             args,
-	})
-
-	if err != nil {
+	if err := serverless.Deploy(); err != nil {
 		return err
 	}
 
@@ -273,24 +147,16 @@ func resourceDeploymentUpdate(d *schema.ResourceData, m interface{}) error {
 }
 
 func resourceDeploymentDelete(d *schema.ResourceData, m interface{}) error {
-	configDir := d.Get("config_dir").(string)
-	serverlessBinDir := d.Get("serverless_bin_dir").(string)
-	packageDir := d.Get("package_dir").(string)
-	stage := d.Get("stage").(string)
-	args := d.Get("args").([]interface{})
+	serverless, err := NewServerless(d)
 
-	err := runServerless(&serverlessParams{
-		command:          "remove",
-		serverlessBinDir: serverlessBinDir,
-		configDir:        configDir,
-		packageDir:       packageDir,
-		stage:            stage,
-		args:             args,
-	})
 	if err != nil {
 		return err
 	}
 
+	if err := serverless.Remove(); err != nil {
+		return err
+	}
+
 	d.SetId("")
 
 	return nil

serverless/resource_deployment_test.go

@@ -73,13 +73,16 @@ func testAccPreCheck(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = runServerless(&serverlessParams{
-		command:          "package",
-		configDir:        configDir,
-		serverlessBinDir: serverlessBinDir,
-		packageDir:       ".terraform-serverless",
-		stage:            "sandbox",
-	})
+	s := &Serverless{
+		configDir:  configDir,
+		binDir:     serverlessBinDir,
+		binPath:    buildBinPath(configDir, serverlessBinDir),
+		packageDir: ".terraform-serverless",
+		stage:      "sandbox",
+	}
+
+	err = s.Package()
+
 	if err != nil {
 		t.Fatal(err)
 	}