Fix: update envBinding related workflow bug (#2831)

* Fix: fix update envBinding related workflow bug

* Fix: support cloudresource step

* Fix: add unit test

* Fix: add unit test

* Fix: add unit test

* unit test

Co-authored-by: zhuxiaobing <zhuxiaobing@lixiang.com>

pkg/apiserver/rest/usecase/application.go

@@ -126,7 +126,8 @@ func (c *applicationUsecaseImpl) ListApplications(ctx context.Context, listOptio
 	}
 	var list []*apisv1.ApplicationBase
 	for _, entity := range entitys {
-		appBase := c.converAppModelToBase(entity.(*model.Application))
+		appModel := entity.(*model.Application)
+		appBase := c.converAppModelToBase(appModel)
 		if listOptions.Query != "" &&
 			!(strings.Contains(appBase.Alias, listOptions.Query) ||
 				strings.Contains(appBase.Name, listOptions.Query) ||
@@ -134,8 +135,8 @@ func (c *applicationUsecaseImpl) ListApplications(ctx context.Context, listOptio
 			continue
 		}
 		if listOptions.TargetName != "" {
-			targetIsContain, _ := c.envBindingUsecase.CheckAppEnvBindingsContainTarget(ctx, &app, listOptions.TargetName)
-			if targetIsContain {
+			targetIsContain, _ := c.envBindingUsecase.CheckAppEnvBindingsContainTarget(ctx, appModel, listOptions.TargetName)
+			if !targetIsContain {
 				continue
 			}
 		}

pkg/apiserver/rest/usecase/application_test.go

@@ -163,6 +163,14 @@ var _ = Describe("Test application usecase function", func() {
 		Expect(err).Should(BeNil())
 	})
 
+	It("Test ListApplications and filter by targetName function", func() {
+		list, err := appUsecase.ListApplications(context.TODO(), v1.ListApplicatioOptions{
+			Namespace:  "test-app-namespace",
+			TargetName: "dev-target"})
+		Expect(err).Should(BeNil())
+		Expect(cmp.Diff(len(list), 2)).Should(BeEmpty())
+	})
+
 	It("Test DetailApplication function", func() {
 		appModel, err := appUsecase.GetApplication(context.TODO(), "test-app-sadasd")
 		Expect(err).Should(BeNil())

pkg/apiserver/rest/usecase/envbinding.go

@@ -255,8 +255,44 @@ func (e *envBindingUsecaseImpl) createEnvWorkflow(ctx context.Context, app *mode
 }
 
 func (e *envBindingUsecaseImpl) updateEnvWorkflow(ctx context.Context, app *model.Application, env *model.EnvBinding) error {
-	// TODO: update env workflow
 	// The existing step configuration should be maintained and the delivery target steps should be automatically updated.
+	envSteps := e.genEnvWorkflowSteps(ctx, env, app)
+	workflow, err := e.workflowUsecase.GetWorkflow(ctx, app, convertWorkflowName(env.Name))
+	if err != nil {
+		return err
+	}
+
+	var envStepNames = env.TargetNames
+	var workflowStepNames []string
+	for _, step := range workflow.Steps {
+		if isEnvStepType(step.Type) {
+			workflowStepNames = append(workflowStepNames, step.Name)
+		}
+	}
+
+	var filteredSteps []apisv1.WorkflowStep
+	_, readyToDeleteSteps, readyToAddSteps := compareSlices(workflowStepNames, envStepNames)
+
+	for _, step := range workflow.Steps {
+		if isEnvStepType(step.Type) && utils.StringsContain(readyToDeleteSteps, step.Name) {
+			continue
+		}
+		filteredSteps = append(filteredSteps, convertFromWorkflowStepModel(step))
+	}
+
+	for _, step := range envSteps {
+		if isEnvStepType(step.Type) && utils.StringsContain(readyToAddSteps, step.Name) {
+			filteredSteps = append(filteredSteps, step)
+		}
+	}
+
+	_, err = e.workflowUsecase.UpdateWorkflow(ctx, workflow, apisv1.UpdateWorkflowRequest{
+		Steps:       filteredSteps,
+		Description: workflow.Description,
+	})
+	if err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -417,3 +453,32 @@ func (e *envBindingUsecaseImpl) genEnvWorkflowSteps(ctx context.Context, env *mo
 func convertWorkflowName(envName string) string {
 	return fmt.Sprintf("workflow-%s", envName)
 }
+
+func compareSlices(a []string, b []string) ([]string, []string, []string) {
+	m := make(map[string]uint8)
+	for _, k := range a {
+		m[k] |= 1 << 0
+	}
+	for _, k := range b {
+		m[k] |= 1 << 1
+	}
+
+	var inAAndB, inAButNotB, inBButNotA []string
+	for k, v := range m {
+		a := v&(1<<0) != 0
+		b := v&(1<<1) != 0
+		switch {
+		case a && b:
+			inAAndB = append(inAAndB, k)
+		case a && !b:
+			inAButNotB = append(inAButNotB, k)
+		case !a && b:
+			inBButNotA = append(inBButNotA, k)
+		}
+	}
+	return inAAndB, inAButNotB, inBButNotA
+}
+
+func isEnvStepType(stepType string) bool {
+	return stepType == Deploy2Env || stepType == DeployCloudResource
+}

pkg/apiserver/rest/usecase/envbinding_test.go

@@ -23,6 +23,8 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
+	"github.com/oam-dev/kubevela/pkg/apiserver/rest/utils"
+
 	"github.com/oam-dev/kubevela/pkg/apiserver/model"
 	apisv1 "github.com/oam-dev/kubevela/pkg/apiserver/rest/apis/v1"
 )
@@ -31,6 +33,7 @@ var _ = Describe("Test envBindingUsecase functions", func() {
 	var (
 		envBindingUsecase *envBindingUsecaseImpl
 		workflowUsecase   *workflowUsecaseImpl
+		definitionUsecase DefinitionUsecase
 		envBindingDemo1   apisv1.EnvBinding
 		envBindingDemo2   apisv1.EnvBinding
 		testApp           *model.Application
@@ -41,7 +44,8 @@ var _ = Describe("Test envBindingUsecase functions", func() {
 			Namespace: "default",
 		}
 		workflowUsecase = &workflowUsecaseImpl{ds: ds, kubeClient: k8sClient}
-		envBindingUsecase = &envBindingUsecaseImpl{ds: ds, workflowUsecase: workflowUsecase, kubeClient: k8sClient}
+		definitionUsecase = &definitionUsecaseImpl{kubeClient: k8sClient, caches: make(map[string]*utils.MemoryCache)}
+		envBindingUsecase = &envBindingUsecaseImpl{ds: ds, workflowUsecase: workflowUsecase, definitionUsecase: definitionUsecase, kubeClient: k8sClient}
 		envBindingDemo1 = apisv1.EnvBinding{
 			Name:        "dev",
 			Alias:       "dev alias",
@@ -99,14 +103,24 @@ var _ = Describe("Test envBindingUsecase functions", func() {
 
 	It("Test Application UpdateEnv function", func() {
 		envBinding, err := envBindingUsecase.UpdateEnvBinding(context.TODO(), testApp, "prod", apisv1.PutApplicationEnvRequest{
-			TargetNames: []string{"prod-target-new1"},
+			TargetNames: []string{"prod-target-new1", "prod-target-new2"},
 		})
 		Expect(err).Should(BeNil())
 		Expect(envBinding).ShouldNot(BeNil())
 		Expect(cmp.Diff(envBinding.TargetNames[0], "prod-target-new1")).Should(BeEmpty())
-		// workflow, err := workflowUsecase.GetWorkflow(context.TODO(), testApp, "prod")
-		// Expect(err).Should(BeNil())
-		// Expect(cmp.Diff(workflow.Steps[0].Name, "prod-target-new1")).Should(BeEmpty())
+		workflow, err := workflowUsecase.GetWorkflow(context.TODO(), testApp, "workflow-prod")
+		Expect(err).Should(BeNil())
+		Expect(cmp.Diff(workflow.Steps[0].Name, "prod-target-new1")).Should(BeEmpty())
+
+		envBinding, err = envBindingUsecase.UpdateEnvBinding(context.TODO(), testApp, "prod", apisv1.PutApplicationEnvRequest{
+			TargetNames: []string{"prod-target-new3", "prod-target-new2"},
+		})
+		Expect(err).Should(BeNil())
+		Expect(envBinding).ShouldNot(BeNil())
+		Expect(cmp.Diff(envBinding.TargetNames[0], "prod-target-new3")).Should(BeEmpty())
+		workflow, err = workflowUsecase.GetWorkflow(context.TODO(), testApp, "workflow-prod")
+		Expect(err).Should(BeNil())
+		Expect(cmp.Diff(workflow.Steps[1].Name, "prod-target-new3")).Should(BeEmpty())
 	})
 
 	It("Test Application DeleteEnv function", func() {

pkg/apiserver/rest/usecase/workflow.go

@@ -229,20 +229,7 @@ func (w *workflowUsecaseImpl) UpdateWorkflow(ctx context.Context, workflow *mode
 func converWorkflowBase(workflow *model.Workflow) apisv1.WorkflowBase {
 	var steps []apisv1.WorkflowStep
 	for _, step := range workflow.Steps {
-		apiStep := apisv1.WorkflowStep{
-			Name:        step.Name,
-			Type:        step.Type,
-			Alias:       step.Alias,
-			Description: step.Description,
-			Inputs:      step.Inputs,
-			Outputs:     step.Outputs,
-			Properties:  step.Properties.JSON(),
-			DependsOn:   step.DependsOn,
-		}
-		if step.Properties != nil {
-			apiStep.Properties = step.Properties.JSON()
-		}
-		steps = append(steps, apiStep)
+		steps = append(steps, convertFromWorkflowStepModel(step))
 	}
 	return apisv1.WorkflowBase{
 		Name:        workflow.Name,
@@ -674,6 +661,23 @@ func convertFromRecordModel(record *model.WorkflowRecord) *apisv1.WorkflowRecord
 	}
 }
 
+func convertFromWorkflowStepModel(step model.WorkflowStep) apisv1.WorkflowStep {
+	apiStep := apisv1.WorkflowStep{
+		Name:        step.Name,
+		Type:        step.Type,
+		Alias:       step.Alias,
+		Description: step.Description,
+		Inputs:      step.Inputs,
+		Outputs:     step.Outputs,
+		Properties:  step.Properties.JSON(),
+		DependsOn:   step.DependsOn,
+	}
+	if step.Properties != nil {
+		apiStep.Properties = step.Properties.JSON()
+	}
+	return apiStep
+}
+
 func convertBool(b *bool) bool {
 	if b == nil {
 		return false

pkg/apiserver/rest/webservice/application.go

@@ -63,7 +63,7 @@ func (c *applicationWebService) GetWebService() *restful.WebService {
 		Metadata(restfulspec.KeyOpenAPITags, tags).
 		Param(ws.QueryParameter("query", "Fuzzy search based on name or description").DataType("string")).
 		Param(ws.QueryParameter("namespace", "The namespace of the managed cluster").DataType("string")).
-		Param(ws.QueryParameter("target", "Name of the application delivery target").DataType("string")).
+		Param(ws.QueryParameter("targetName", "Name of the application delivery target").DataType("string")).
 		Returns(200, "", apis.ListApplicationResponse{}).
 		Returns(400, "", bcode.Bcode{}).
 		Writes(apis.ListApplicationResponse{}))
@@ -506,7 +506,7 @@ func (c *applicationWebService) createApplication(req *restful.Request, res *res
 func (c *applicationWebService) listApplications(req *restful.Request, res *restful.Response) {
 	apps, err := c.applicationUsecase.ListApplications(req.Request.Context(), apis.ListApplicatioOptions{
 		Namespace:  req.QueryParameter("namespace"),
-		TargetName: req.QueryParameter("target"),
+		TargetName: req.QueryParameter("targetName"),
 		Query:      req.QueryParameter("query"),
 	})
 	if err != nil {