Repository: digitalocean/doctl
Pull request #1297 (2022-10-25 15:00:07)

Implement sls undeploy in native golang
Implements `doctl sls undeploy` subtree in goland instead of using the nim plugin. 


Patch:
diff --git a/commands/serverless.go b/commands/serverless.go
index 8c69e2de..866a0b2a 100644
--- a/commands/serverless.go
+++ b/commands/serverless.go
@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	"github.com/digitalocean/doctl"
+	"github.com/digitalocean/doctl/commands/charm/template"
 	"github.com/digitalocean/doctl/do"
 	"github.com/spf13/cobra"
 )
@@ -116,6 +117,8 @@ the entire packages are removed.`, Writer)
 	AddBoolFlag(undeploy, "packages", "p", false, "interpret simple name arguments as packages")
 	AddBoolFlag(undeploy, "triggers", "", false, "interpret all arguments as triggers")
 	AddBoolFlag(undeploy, "all", "", false, "remove all packages and functions")
+	AddBoolFlag(undeploy, doctl.ArgForce, doctl.ArgShortForce, false, "Delete namespace resources without confirmation prompt")
+	undeploy.Flags().MarkHidden(doctl.ArgForce)
 	undeploy.Flags().MarkHidden("triggers") // support is experimental at this point
 
 	cmd.AddCommand(Activations())
@@ -399,7 +402,11 @@ func RunServerlessUndeploy(c *CmdConfig) error {
 	haveArgs := len(c.Args) > 0
 	pkgFlag, _ := c.Doit.GetBool(c.NS, "packages")
 	trigFlag, _ := c.Doit.GetBool(c.NS, "triggers")
+
 	all, _ := c.Doit.GetBool(c.NS, "all")
+
+	sls := c.Serverless()
+
 	if haveArgs && all {
 		return errUndeployAllAndArgs
 	}
@@ -412,74 +419,43 @@ func RunServerlessUndeploy(c *CmdConfig) error {
 	if all && trigFlag {
 		return cleanTriggers(c)
 	}
+
 	if all {
-		return cleanNamespace(c)
+		err := sls.CleanNamespace()
+		if err != nil {
+			return err
+		}
+		template.Print(`{{success checkmark}} All resources in the namespace have been undeployed.{{nl}}`, nil)
+		return nil
 	}
+
 	var lastError error
 	errorCount := 0
 	var ctx context.Context
-	var sls do.ServerlessService
+
 	if trigFlag {
 		ctx = context.TODO()
-		sls = c.Serverless()
 	}
+
 	for _, arg := range c.Args {
 		var err error
 		if trigFlag {
 			err = sls.DeleteTrigger(ctx, arg)
 		} else if strings.Contains(arg, "/") || !pkgFlag {
-			err = deleteFunction(c, arg)
+			err = sls.DeleteFunction(arg, true)
 		} else {
-			err = deletePackage(c, arg)
+			err = sls.DeletePackage(arg, true)
 		}
+
 		if err != nil {
 			lastError = err
 			errorCount++
 		}
 	}
+
 	if errorCount > 0 {
 		return fmt.Errorf("there were %d errors detected, e.g.: %w", errorCount, lastError)
 	}
-	if all {
-		fmt.Fprintln(c.Out, "All resources in the functions namespace have been undeployed")
-	} else {
-		fmt.Fprintln(c.Out, "The requested resources have been undeployed")
-	}
-	return nil
-}
-
-// cleanNamespace is a subroutine of RunServerlessDeploy for clearing the entire namespace
-func cleanNamespace(c *CmdConfig) error {
-	result, err := ServerlessExec(c, "namespace/clean", "--force")
-	if err != nil {
-		return err
-	}
-	if result.Error != "" {
-		return fmt.Errorf(result.Error)
-	}
-	return nil
-}
-
-// deleteFunction is a subroutine of RunServerlessDeploy for deleting one function
-func deleteFunction(c *CmdConfig, fn string) error {
-	result, err := ServerlessExec(c, "action/delete", fn)
-	if err != nil {
-		return err
-	}
-	if result.Error != "" {
-		return fmt.Errorf(result.Error)
-	}
-	return nil
-}
-
-// deletePackage is a subroutine of RunServerlessDeploy for deleting a package
-func deletePackage(c *CmdConfig, pkg string) error {
-	result, err := ServerlessExec(c, "package/delete", pkg, "--recursive")
-	if err != nil {
-		return err
-	}
-	if result.Error != "" {
-		return fmt.Errorf(result.Error)
-	}
+	template.Print(`{{success checkmark}} The requested resources have been undeployed.{{nl}}`, nil)
 	return nil
 }
diff --git a/do/mocks/ServerlessService.go b/do/mocks/ServerlessService.go
index d56985fb..33cec77a 100644
--- a/do/mocks/ServerlessService.go
+++ b/do/mocks/ServerlessService.go
@@ -51,6 +51,20 @@ func (mr *MockServerlessServiceMockRecorder) CheckServerlessStatus() *gomock.Cal
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckServerlessStatus", reflect.TypeOf((*MockServerlessService)(nil).CheckServerlessStatus))
 }
 
+// CleanNamespace mocks base method.
+func (m *MockServerlessService) CleanNamespace() error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "CleanNamespace")
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// CleanNamespace indicates an expected call of CleanNamespace.
+func (mr *MockServerlessServiceMockRecorder) CleanNamespace() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanNamespace", reflect.TypeOf((*MockServerlessService)(nil).CleanNamespace))
+}
+
 // Cmd mocks base method.
 func (m *MockServerlessService) Cmd(arg0 string, arg1 []string) (*exec.Cmd, error) {
 	m.ctrl.T.Helper()
@@ -81,6 +95,20 @@ func (mr *MockServerlessServiceMockRecorder) CreateNamespace(arg0, arg1, arg2 in
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNamespace", reflect.TypeOf((*MockServerlessService)(nil).CreateNamespace), arg0, arg1, arg2)
 }
 
+// DeleteFunction mocks base method.
+func (m *MockServerlessService) DeleteFunction(arg0 string, arg1 bool) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "DeleteFunction", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// DeleteFunction indicates an expected call of DeleteFunction.
+func (mr *MockServerlessServiceMockRecorder) DeleteFunction(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFunction", reflect.TypeOf((*MockServerlessService)(nil).DeleteFunction), arg0, arg1)
+}
+
 // DeleteNamespace mocks base method.
 func (m *MockServerlessService) DeleteNamespace(arg0 context.Context, arg1 string) error {
 	m.ctrl.T.Helper()
@@ -95,6 +123,20 @@ func (mr *MockServerlessServiceMockRecorder) DeleteNamespace(arg0, arg1 interfac
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNamespace", reflect.TypeOf((*MockServerlessService)(nil).DeleteNamespace), arg0, arg1)
 }
 
+// DeletePackage mocks base method.
+func (m *MockServerlessService) DeletePackage(arg0 string, arg1 bool) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "DeletePackage", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// DeletePackage indicates an expected call of DeletePackage.
+func (mr *MockServerlessServiceMockRecorder) DeletePackage(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeletePackage", reflect.TypeOf((*MockServerlessService)(nil).DeletePackage), arg0, arg1)
+}
+
 // DeleteTrigger mocks base method.
 func (m *MockServerlessService) DeleteTrigger(arg0 context.Context, arg1 string) error {
 	m.ctrl.T.Helper()
diff --git a/do/serverless.go b/do/serverless.go
index 04e457ae..076d6c67 100644
--- a/do/serverless.go
+++ b/do/serverless.go
@@ -198,6 +198,7 @@ type ServerlessService interface {
 	GetNamespaceFromCluster(string, string) (string, error)
 	CreateNamespace(context.Context, string, string) (ServerlessCredentials, error)
 	DeleteNamespace(context.Context, string) error
+	CleanNamespace() error
 	ListTriggers(context.Context, string) ([]ServerlessTrigger, error)
 	GetTrigger(context.Context, string) (ServerlessTrigger, error)
 	DeleteTrigger(context.Context, string) error
@@ -206,8 +207,10 @@ type ServerlessService interface {
 	GetHostInfo(string) (ServerlessHostInfo, error)
 	CheckServerlessStatus() error
 	InstallServerless(string, bool) error
+	DeletePackage(string, bool) error
 	GetFunction(string, bool) (whisk.Action, []FunctionParameter, error)
 	ListFunctions(string, int, int) ([]whisk.Action, error)
+	DeleteFunction(string, bool) error
 	InvokeFunction(string, interface{}, bool, bool) (interface{}, error)
 	InvokeFunctionViaWeb(string, interface{}) error
 	ListActivations(whisk.ActivationListOptions) ([]whisk.Activation, error)
@@ -678,6 +681,45 @@ func (s *serverlessService) DeleteNamespace(ctx context.Context, name string) er
 	return err
 }
 
+func (s *serverlessService) CleanNamespace() error {
+	// Deletes all triggers
+	ctx := context.TODO()
+	triggers, err := s.ListTriggers(ctx, "")
+
+	// Intentionally ignore errors when listing triggers, the trigger API is behind a
+	// feature flag and may will return an error for users not enabled.
+	if err == nil {
+		for _, trig := range triggers {
+			err = s.DeleteTrigger(ctx, trig.Name)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	// Deletes all functions
+	fns, err := s.ListFunctions("", 0, 0)
+	if err != nil {
+		return err
+	}
+
+	for _, fn := range fns {
+		name := fn.Name
+		pkg := strings.Split(fn.Namespace, "/")
+		if len(pkg) == 2 {
+			name = pkg[1] + "/" + fn.Name
+		}
+
+		// All triggers for the namespace are deleted above so we don't need to remove the trigger for each individual function.
+		err := s.DeleteFunction(name, false)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // GetHostInfo returns the HostInfo structure of the provided API host
 func (s *serverlessService) GetHostInfo(APIHost string) (ServerlessHostInfo, error) {
 	endpoint := APIHost + "/api/v1"
@@ -735,6 +777,64 @@ func (s *serverlessService) ListFunctions(pkg string, skip int, limit int) ([]wh
 	return list, err
 }
 
+// DeleteFunction removes a function from the namespace
+func (s *serverlessService) DeleteFunction(name string, deleteTriggers bool) error {
+	err := initWhisk(s)
+	if err != nil {
+		return err
+	}
+
+	if deleteTriggers {
+		ctx := context.TODO()
+		triggers, err := s.ListTriggers(ctx, name)
+
+		// Intentionally ignore errors when listing triggers, the trigger API is behind a
+		// feature flag and may will return an error for users not enabled.
+		if err == nil {
+			for _, trig := range triggers {
+				err = s.DeleteTrigger(ctx, trig.Name)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	_, e := s.owClient.Actions.Delete(name)
+
+	return e
+}
+
+// DeletePackage removes a package and all its namespace from the package from the namespace
+// If force is set to true, it will remove all functions in the package.
+func (s *serverlessService) DeletePackage(name string, force bool) error {
+	err := initWhisk(s)
+	if err != nil {
+		return err
+	}
+
+	if force {
+		pkg, _, err := s.owClient.Packages.Get(name)
+		if err != nil {
+			return err
+		}
+
+		for _, fn := range pkg.Actions {
+			funcName := name + "/" + fn.Name
+
+			err = s.DeleteFunction(funcName, true)
+
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	_, err = s.owClient.Packages.Delete(name)
+
+	return err
+}
+
 // InvokeFunction invokes a function via POST with authentication
 func (s *serverlessService) InvokeFunction(name string, params interface{}, blocking bool, result bool) (interface{}, error) {
 	var empty map[string]interface{}


Test patch:
diff --git a/commands/serverless_test.go b/commands/serverless_test.go
index b3ecaab5..6ba0fdd3 100644
--- a/commands/serverless_test.go
+++ b/commands/serverless_test.go
@@ -476,110 +476,65 @@ func TestServerlessDeploy(t *testing.T) {
 }
 
 func TestServerlessUndeploy(t *testing.T) {
-	type testNimCmd struct {
-		cmd  string
-		args []string
-	}
-
 	tests := []struct {
-		name                 string
-		doctlArgs            []string
-		doctlFlags           map[string]string
-		expectedNimCmds      []testNimCmd
-		expectedError        error
-		expectTriggerDeletes []string
-		expectTriggerList    bool
+		name          string
+		doctlArgs     []string
+		doctlFlags    map[string]string
+		expectedError error
 	}{
 		{
-			name:            "no arguments or flags",
-			doctlArgs:       nil,
-			doctlFlags:      nil,
-			expectedNimCmds: nil,
-			expectedError:   errUndeployTooFewArgs,
+			name:          "no arguments or flags",
+			doctlArgs:     nil,
+			doctlFlags:    nil,
+			expectedError: errUndeployTooFewArgs,
 		},
 		{
-			name:       "with --all flag only",
-			doctlArgs:  nil,
-			doctlFlags: map[string]string{"all": ""},
-			expectedNimCmds: []testNimCmd{
-				{
-					cmd:  "namespace/clean",
-					args: []string{"--force"},
-				},
-			},
+			name:          "with --all flag only",
+			doctlArgs:     nil,
+			doctlFlags:    map[string]string{"all": ""},
 			expectedError: nil,
 		},
 		{
-			name:       "mixed args, no flags",
-			doctlArgs:  []string{"foo/bar", "baz"},
-			doctlFlags: nil,
-			expectedNimCmds: []testNimCmd{
-				{
-					cmd:  "action/delete",
-					args: []string{"foo/bar"},
-				},
-				{
-					cmd:  "action/delete",
-					args: []string{"baz"},
-				},
-			},
+			name:          "mixed args, no flags",
+			doctlArgs:     []string{"foo/bar", "baz"},
+			doctlFlags:    nil,
 			expectedError: nil,
 		},
 		{
-			name:       "mixed args, --packages flag",
-			doctlArgs:  []string{"foo/bar", "baz"},
-			doctlFlags: map[string]string{"packages": ""},
-			expectedNimCmds: []testNimCmd{
-				{
-					cmd:  "action/delete",
-					args: []string{"foo/bar"},
-				},
-				{
-					cmd:  "package/delete",
-					args: []string{"baz", "--recursive"},
-				},
-			},
+			name:          "mixed args, --packages flag",
+			doctlArgs:     []string{"foo/bar", "baz"},
+			doctlFlags:    map[string]string{"packages": ""},
 			expectedError: nil,
 		},
 		{
-			name:            "mixed args, --all flag",
-			doctlArgs:       []string{"foo/bar", "baz"},
-			doctlFlags:      map[string]string{"all": ""},
-			expectedNimCmds: nil,
-			expectedError:   errUndeployAllAndArgs,
+			name:          "mixed args, --all flag",
+			doctlArgs:     []string{"foo/bar", "baz"},
+			doctlFlags:    map[string]string{"all": ""},
+			expectedError: errUndeployAllAndArgs,
 		},
 		{
-			name:            "--triggers and --packages",
-			doctlArgs:       []string{"foo/bar", "baz"},
-			doctlFlags:      map[string]string{"triggers": "", "packages": ""},
-			expectedNimCmds: nil,
-			expectedError:   errUndeployTrigPkg,
+			name:          "--triggers and --packages",
+			doctlArgs:     []string{"foo/bar", "baz"},
+			doctlFlags:    map[string]string{"triggers": "", "packages": ""},
+			expectedError: errUndeployTrigPkg,
 		},
 		{
-			name:                 "--triggers and args",
-			doctlArgs:            []string{"fire1", "fire2"},
-			doctlFlags:           map[string]string{"triggers": ""},
-			expectedNimCmds:      nil,
-			expectedError:        nil,
-			expectTriggerDeletes: []string{"fire1", "fire2"},
+			name:          "--triggers and args",
+			doctlArgs:     []string{"fire1", "fire2"},
+			doctlFlags:    map[string]string{"triggers": ""},
+			expectedError: nil,
 		},
 		{
-			name:                 "--triggers and --all",
-			doctlArgs:            nil,
-			doctlFlags:           map[string]string{"triggers": "", "all": ""},
-			expectedNimCmds:      nil,
-			expectedError:        nil,
-			expectTriggerDeletes: []string{"fireA", "fireB"},
-			expectTriggerList:    true,
+			name:          "--triggers and --all",
+			doctlArgs:     nil,
+			doctlFlags:    map[string]string{"triggers": "", "all": ""},
+			expectedError: nil,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
-				fakeCmd := &exec.Cmd{
-					Stdout: config.Out,
-				}
 				cannedTriggerList := []do.ServerlessTrigger{
 					{Name: "fireA"},
 					{Name: "fireB"},
@@ -589,8 +544,24 @@ func TestServerlessUndeploy(t *testing.T) {
 					config.Args = append(config.Args, tt.doctlArgs...)
 				}
 
+				var pkg bool = false
+				var trig bool = false
+				var all bool = false
+
 				if tt.doctlFlags != nil {
 					for k, v := range tt.doctlFlags {
+						if k == "all" {
+							all = true
+						}
+
+						if k == "packages" {
+							pkg = true
+						}
+
+						if k == "triggers" {
+							trig = true
+						}
+
 						if v == "" {
 							config.Doit.Set(config.NS, k, true)
 						} else {
@@ -599,20 +570,29 @@ func TestServerlessUndeploy(t *testing.T) {
 					}
 				}
 
-				if tt.expectedError == nil && len(tt.expectedNimCmds) > 0 {
-					tm.serverless.EXPECT().CheckServerlessStatus().MinTimes(1).Return(nil)
-				}
-				if tt.expectTriggerList {
+				if all && !trig && !pkg && len(config.Args) == 0 {
+					tm.serverless.EXPECT().CleanNamespace().Return(nil)
+				} else if all && trig && len(config.Args) == 0 {
 					tm.serverless.EXPECT().ListTriggers(context.TODO(), "").Return(cannedTriggerList, nil)
-				}
-				for i := range tt.expectedNimCmds {
-					tm.serverless.EXPECT().Cmd(tt.expectedNimCmds[i].cmd, tt.expectedNimCmds[i].args).Return(fakeCmd, nil)
-					tm.serverless.EXPECT().Exec(fakeCmd).Return(do.ServerlessOutput{}, nil)
-				}
-				for _, trig := range tt.expectTriggerDeletes {
-					tm.serverless.EXPECT().DeleteTrigger(context.TODO(), trig)
+					for _, st := range cannedTriggerList {
+						tm.serverless.EXPECT().DeleteTrigger(context.TODO(), st.Name)
+					}
+
+				} else if !all && !trig && len(config.Args) > 0 {
+					for _, arg := range config.Args {
+						if !pkg || strings.Contains(arg, "/") {
+							tm.serverless.EXPECT().DeleteFunction(arg, true).Return(nil)
+						} else {
+							tm.serverless.EXPECT().DeletePackage(arg, true).Return(nil)
+						}
+					}
+				} else if !all && trig && !pkg && len(config.Args) > 0 {
+					for _, t := range config.Args {
+						tm.serverless.EXPECT().DeleteTrigger(context.TODO(), t)
+					}
 				}
 				err := RunServerlessUndeploy(config)
+
 				if tt.expectedError != nil {
 					require.Error(t, err)
 					assert.ErrorIs(t, err, tt.expectedError)