Fix: the target conflict when syncing the application (#4312)

Signed-off-by: barnettZQG <barnett.zqg@gmail.com>

pkg/apiserver/domain/service/cloudshell.go

@@ -211,6 +211,7 @@ func (c *cloudShellServiceImpl) prepareKubeConfig(ctx context.Context) error {
 			readOnly = checkReadOnly(p.Name, permissions)
 		}
 		if readOnly {
+			// TODO:(@barnettZQG) there needs a method to revoke the privileges
 			if err := c.managePrivilegesForUser(ctx, p.Name, true, userName, false); err != nil {
 				log.Logger.Errorf("failed to privileges the user %s", err.Error())
 			}
@@ -342,7 +343,6 @@ func checkReadOnly(projectName string, permissions []*model.Permission) bool {
 
 // managePrivilegesForUser grant or revoke privileges for a user
 func (c *cloudShellServiceImpl) managePrivilegesForUser(ctx context.Context, projectName string, readOnly bool, userName string, revoke bool) error {
-
 	targets, err := c.TargetService.ListTargets(ctx, 0, 0, projectName)
 	if err != nil {
 		log.Logger.Infof("failed to list the targets by the project name %s :%s", projectName, err.Error())

pkg/apiserver/domain/service/env.go

@@ -273,9 +273,11 @@ func (p *envServiceImpl) CreateEnv(ctx context.Context, req apisv1.CreateEnvRequ
 		Targets:     req.Targets,
 	}
 
-	pass, err := p.checkEnvTarget(ctx, req.Project, req.Name, req.Targets)
-	if err != nil || !pass {
-		return nil, bcode.ErrEnvTargetConflict
+	if !req.AllowTargetConflict {
+		pass, err := p.checkEnvTarget(ctx, req.Project, req.Name, req.Targets)
+		if err != nil || !pass {
+			return nil, bcode.ErrEnvTargetConflict
+		}
 	}
 
 	targets, err := repository.ListTarget(ctx, p.Store, "", nil)
@@ -312,7 +314,7 @@ func (p *envServiceImpl) checkEnvTarget(ctx context.Context, project string, env
 	if len(targets) == 0 {
 		return true, nil
 	}
-	entitys, err := p.Store.List(ctx, &model.Env{Project: project}, &datastore.ListOptions{})
+	entities, err := p.Store.List(ctx, &model.Env{Project: project}, &datastore.ListOptions{})
 	if err != nil {
 		return false, err
 	}
@@ -320,7 +322,7 @@ func (p *envServiceImpl) checkEnvTarget(ctx context.Context, project string, env
 	for _, new := range targets {
 		newMap[new] = true
 	}
-	for _, entity := range entitys {
+	for _, entity := range entities {
 		env := entity.(*model.Env)
 		for _, existTarget := range env.Targets {
 			if ok := newMap[existTarget]; ok && env.Name != envName {

pkg/apiserver/event/sync/convert.go

@@ -62,40 +62,51 @@ func (c *CR2UX) ConvertApp2DatastoreApp(ctx context.Context, targetApp *v1beta1.
 		AppMeta: appMeta,
 	}
 
-	// 1. convert the target
-
-	existTarget := &model.Target{Project: project}
+	// 2. convert the target
+	existTarget := &model.Target{}
 	existTargets, err := c.ds.List(ctx, existTarget, nil)
 	if err != nil {
 		return nil, fmt.Errorf("fail to list the targets, %w", err)
 	}
-	var envTargetNames []string
+	var envTargetNames map[string]string
 	dsApp.Targets, envTargetNames = convert.FromCRTargets(ctx, c.cli, targetApp, existTargets, project)
 
-	// 2. convert the environment
-	existEnv := &model.Env{Namespace: targetApp.Namespace, Project: project}
+	// 3. convert the environment
+	existEnv := &model.Env{Namespace: targetApp.Namespace}
 	existEnvs, err := c.ds.List(ctx, existEnv, nil)
 	if err != nil {
 		return nil, fmt.Errorf("fail to list the env, %w", err)
 	}
 	if len(existEnvs) > 0 {
 		env := existEnvs[0].(*model.Env)
-		for _, name := range envTargetNames {
-			if !utils.StringsContain(env.Targets, name) {
+		dsApp.AppMeta.Project = env.Project
+		for name, project := range envTargetNames {
+			if !utils.StringsContain(env.Targets, name) && project == env.Project {
 				env.Targets = append(env.Targets, name)
 			}
 		}
 		dsApp.Env = env
 	}
 	if dsApp.Env == nil {
+		var newProject string
+		var targetNames []string
+		for name, project := range envTargetNames {
+			if newProject == "" {
+				newProject = project
+			}
+			if newProject == project {
+				targetNames = append(targetNames, name)
+			}
+		}
 		dsApp.Env = &model.Env{
 			Name:        model.AutoGenEnvNamePrefix + targetApp.Namespace,
 			Namespace:   targetApp.Namespace,
 			Description: model.AutoGenDesc,
-			Project:     project,
+			Project:     newProject,
 			Alias:       model.AutoGenEnvNamePrefix + targetApp.Namespace,
-			Targets:     envTargetNames,
+			Targets:     targetNames,
 		}
+		dsApp.AppMeta.Project = newProject
 	}
 	dsApp.Eb = &model.EnvBinding{
 		AppPrimaryKey: appMeta.PrimaryKey(),
@@ -103,7 +114,11 @@ func (c *CR2UX) ConvertApp2DatastoreApp(ctx context.Context, targetApp *v1beta1.
 		AppDeployName: appMeta.GetAppNameForSynced(),
 	}
 
-	// 3. convert component and trait
+	for i := range dsApp.Targets {
+		dsApp.Targets[i].Project = dsApp.AppMeta.Project
+	}
+
+	// 4. convert component and trait
 	for _, cmp := range targetApp.Spec.Components {
 		compModel, err := convert.FromCRComponent(appMeta.PrimaryKey(), cmp)
 		if err != nil {
@@ -112,7 +127,7 @@ func (c *CR2UX) ConvertApp2DatastoreApp(ctx context.Context, targetApp *v1beta1.
 		dsApp.Comps = append(dsApp.Comps, &compModel)
 	}
 
-	// 4. convert workflow
+	// 5. convert workflow
 	wf, steps, err := convert.FromCRWorkflow(ctx, cli, appMeta.PrimaryKey(), targetApp)
 	if err != nil {
 		return nil, err
@@ -120,7 +135,7 @@ func (c *CR2UX) ConvertApp2DatastoreApp(ctx context.Context, targetApp *v1beta1.
 	wf.EnvName = dsApp.Env.Name
 	dsApp.Workflow = &wf
 
-	// 5. convert policy, some policies are references in workflow step, we need to sync all the outside policy to make that work
+	// 6. convert policy, some policies are references in workflow step, we need to sync all the outside policy to make that work
 	var innerPlc = make(map[string]struct{})
 	for _, plc := range targetApp.Spec.Policies {
 		innerPlc[plc.Name] = struct{}{}

pkg/apiserver/event/sync/convert/convert.go

@@ -129,14 +129,14 @@ func FromCRWorkflow(ctx context.Context, cli client.Client, appPrimaryKey string
 }
 
 // FromCRTargets converts deployed Cluster/Namespace from Application CR Status into velaux data store
-func FromCRTargets(ctx context.Context, cli client.Client, targetApp *v1beta1.Application, existTargets []datastore.Entity, project string) ([]*model.Target, []string) {
+func FromCRTargets(ctx context.Context, cli client.Client, targetApp *v1beta1.Application, existTargets []datastore.Entity, project string) ([]*model.Target, map[string]string) {
 	existTarget := make(map[string]*model.Target)
 	for i := range existTargets {
 		t := existTargets[i].(*model.Target)
 		existTarget[fmt.Sprintf("%s-%s", t.Cluster.ClusterName, t.Cluster.Namespace)] = t
 	}
 	var targets []*model.Target
-	var targetNames []string
+	var targetNames = map[string]string{}
 	nc := make(map[string]struct{})
 	// read the target from the topology policies
 	placements, err := policy.GetPlacementsFromTopologyPolicies(ctx, cli, targetApp.Namespace, targetApp.Spec.Policies, true)
@@ -160,9 +160,9 @@ func FromCRTargets(ctx context.Context, cli client.Client, targetApp *v1beta1.Ap
 		}
 		nc[name] = struct{}{}
 		if target, ok := existTarget[fmt.Sprintf("%s-%s", placement.Cluster, placement.Namespace)]; ok {
-			targetNames = append(targetNames, target.Name)
+			targetNames[target.Name] = target.Project
 		} else {
-			targetNames = append(targetNames, name)
+			targetNames[name] = project
 			targets = append(targets, &model.Target{
 				Name:    name,
 				Project: project,

pkg/apiserver/event/sync/cr2ux_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/oam-dev/kubevela/pkg/apiserver/domain/model"
 	"github.com/oam-dev/kubevela/pkg/apiserver/domain/service"
 	"github.com/oam-dev/kubevela/pkg/apiserver/infrastructure/datastore"
+	v1 "github.com/oam-dev/kubevela/pkg/apiserver/interfaces/api/dto/v1"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
 	common2 "github.com/oam-dev/kubevela/pkg/utils/common"
 )
@@ -150,6 +151,68 @@ var _ = Describe("Test CR convert to ux", func() {
 		Expect(len(appwf2.Steps)).Should(BeEquivalentTo(0))
 	})
 
+	It("Test exist env", func() {
+		dbNamespace := "update-app-db-ns1-test"
+		ds, err := NewDatastore(datastore.Config{Type: "kubeapi", Database: dbNamespace})
+		Expect(err).Should(BeNil())
+
+		cr2ux := newCR2UX(ds)
+
+		projectName := "project-e"
+
+		_, err = cr2ux.projectService.CreateProject(context.TODO(), v1.CreateProjectRequest{
+			Name:  projectName,
+			Owner: "admin",
+		})
+		Expect(err).Should(BeNil())
+
+		_, err = cr2ux.targetService.CreateTarget(context.TODO(), v1.CreateTargetRequest{
+			Name:    "target-test",
+			Project: projectName,
+			Cluster: &v1.ClusterTarget{
+				ClusterName: "local",
+				Namespace:   "target-test",
+			},
+		})
+		Expect(err).Should(BeNil())
+		_, err = cr2ux.envService.CreateEnv(context.TODO(), v1.CreateEnvRequest{
+			Name:      "env-test",
+			Project:   projectName,
+			Namespace: "env-test",
+			Targets:   []string{"target-test"},
+		})
+		Expect(err).Should(BeNil())
+
+		app4 := &v1beta1.Application{}
+		Expect(common2.ReadYamlToObject("testdata/test-app4.yaml", app4)).Should(BeNil())
+		app4.Namespace = "target-test"
+		Expect(cr2ux.AddOrUpdate(context.Background(), app4)).Should(BeNil())
+
+		count, err := cr2ux.envService.ListEnvCount(context.TODO(), v1.ListEnvOptions{Project: projectName})
+		Expect(err).Should(BeNil())
+		Expect(count).Should(Equal(int64(2)))
+
+		resultApp := &model.Application{Name: app4.Name, Project: "project-e"}
+		err = ds.Get(context.TODO(), resultApp)
+		Expect(err).Should(BeNil())
+		Expect(resultApp.Labels[model.LabelSyncNamespace]).Should(Equal("target-test"))
+
+		app5 := &v1beta1.Application{}
+		Expect(common2.ReadYamlToObject("testdata/test-app4.yaml", app5)).Should(BeNil())
+		app5.Namespace = "env-test"
+		Expect(cr2ux.AddOrUpdate(context.Background(), app5)).Should(BeNil())
+
+		resultApp = &model.Application{Name: formatAppComposedName(app5.Name, app5.Namespace), Project: "project-e"}
+		err = ds.Get(context.TODO(), resultApp)
+		Expect(err).Should(BeNil())
+		Expect(resultApp.Labels[model.LabelSyncNamespace]).Should(Equal("env-test"))
+
+		// should not create new env
+		count, err = cr2ux.envService.ListEnvCount(context.TODO(), v1.ListEnvOptions{Project: projectName})
+		Expect(err).Should(BeNil())
+		Expect(count).Should(Equal(int64(2)))
+	})
+
 })
 
 func newCR2UX(ds datastore.DataStore) *CR2UX {

pkg/apiserver/event/sync/store.go

@@ -82,12 +82,13 @@ func StoreEnv(ctx context.Context, app *model.DataStoreApp, ds datastore.DataSto
 		return err
 	}
 	_, err = envService.CreateEnv(ctx, v1.CreateEnvRequest{
-		Name:        app.Env.Name,
-		Alias:       app.Env.Alias,
-		Description: app.Env.Description,
-		Project:     app.Env.Project,
-		Namespace:   app.Env.Namespace,
-		Targets:     app.Env.Targets,
+		Name:                app.Env.Name,
+		Alias:               app.Env.Alias,
+		Description:         app.Env.Description,
+		Project:             app.Env.Project,
+		Namespace:           app.Env.Namespace,
+		Targets:             app.Env.Targets,
+		AllowTargetConflict: true,
 	})
 	if err != nil && !errors.Is(err, bcode.ErrEnvAlreadyExists) {
 		return err

pkg/apiserver/event/sync/testdata/test-app4.yaml

@@ -0,0 +1,16 @@
+apiVersion: core.oam.dev/v1beta1
+kind: Application
+metadata:
+  name: example-exist-env
+spec:
+  components:
+    - name: example-exist-env
+      type: webservice
+      properties:
+        image: wordpress
+      traits:
+        - type: gateway
+          properties:
+            domain: testsvc.example.com
+            http:
+              "/": 8000

pkg/apiserver/interfaces/api/dto/v1/types.go

@@ -794,6 +794,9 @@ type CreateEnvRequest struct {
 	// Targets defines the name of delivery target that belongs to this env
 	// In one project, a delivery target can only belong to one env.
 	Targets []string `json:"targets,omitempty"  optional:"true"`
+
+	// AllowTargetConflict means allow binding the targets that belong to other envs
+	AllowTargetConflict bool `json:"allowTargetConflict,omitempty"  optional:"true"`
 }
 
 // UpdateEnvRequest defines the data of Env for update