diff --git a/pkg/apiserver/domain/service/application.go b/pkg/apiserver/domain/service/application.go index b0fbd49c4..4c2149916 100644 --- a/pkg/apiserver/domain/service/application.go +++ b/pkg/apiserver/domain/service/application.go @@ -19,6 +19,7 @@ package service import ( "bytes" "context" + "encoding/json" "errors" "fmt" "math/rand" @@ -1168,16 +1169,16 @@ func (c *applicationServiceImpl) DeleteComponent(ctx context.Context, app *model return nil } -func (c *applicationServiceImpl) CreatePolicy(ctx context.Context, app *model.Application, createpolicy apisv1.CreatePolicyRequest) (*apisv1.PolicyBase, error) { +func (c *applicationServiceImpl) CreatePolicy(ctx context.Context, app *model.Application, createPolicy apisv1.CreatePolicyRequest) (*apisv1.PolicyBase, error) { userName, _ := ctx.Value(&apisv1.CtxKeyUser).(string) policyModel := model.ApplicationPolicy{ AppPrimaryKey: app.PrimaryKey(), - Description: createpolicy.Description, + Description: createPolicy.Description, Creator: userName, - Name: createpolicy.Name, - Type: createpolicy.Type, + Name: createPolicy.Name, + Type: createPolicy.Type, } - properties, err := model.NewJSONStructByString(createpolicy.Properties) + properties, err := model.NewJSONStructByString(createPolicy.Properties) if err != nil { return nil, bcode.ErrInvalidProperties } @@ -1189,6 +1190,9 @@ func (c *applicationServiceImpl) CreatePolicy(ctx context.Context, app *model.Ap log.Logger.Warnf("add policy for app %s failure %s", app.PrimaryKey(), err.Error()) return nil, err } + if err = c.handlePolicyBindingWorkflowStep(ctx, app, createPolicy.Name, createPolicy.WorkflowPolicyBindings); err != nil { + return nil, err + } return assembler.ConvertPolicyModelToBase(&policyModel), nil } @@ -1197,6 +1201,13 @@ func (c *applicationServiceImpl) DeletePolicy(ctx context.Context, app *model.Ap AppPrimaryKey: app.PrimaryKey(), Name: policyName, } + used, err := c.checkPolicyUsedByWorkflow(ctx, app, policyName) + if err != nil { + return err + } + if used { + return bcode.ErrApplicationPolicyIsBeingUsed + } if err := c.Store.Delete(ctx, &policy); err != nil { if errors.Is(err, datastore.ErrRecordNotExist) { return bcode.ErrApplicationPolicyNotExist @@ -1231,6 +1242,9 @@ func (c *applicationServiceImpl) UpdatePolicy(ctx context.Context, app *model.Ap if err := c.Store.Put(ctx, &policy); err != nil { return nil, err } + if err = c.handlePolicyBindingWorkflowStep(ctx, app, policyName, policyUpdate.WorkflowPolicyBindings); err != nil { + return nil, err + } return &apisv1.DetailPolicyResponse{ PolicyBase: *assembler.ConvertPolicyModelToBase(&policy), }, nil @@ -1753,3 +1767,87 @@ func NewTestApplicationService(ds datastore.DataStore, c client.Client, cfg *res UserService: userService, } } + +// handlePolicyBindingWorkflowStep handle every operation(create/update) against policy, if this policy is bind to a workflow step, will recorde in workflow step. +func (c *applicationServiceImpl) handlePolicyBindingWorkflowStep(ctx context.Context, app *model.Application, policyName string, wfBindings []apisv1.WorkflowPolicyBinding) error { + const pattern string = "%s-%s" + bindings := map[string]bool{} + for _, binding := range wfBindings { + for _, s := range binding.Steps { + bindings[fmt.Sprintf(pattern, binding.Name, s)] = true + } + } + + workflows, err := c.WorkflowService.ListApplicationWorkflow(ctx, app) + if err != nil { + return err + } + needUpdate := false + for _, w := range workflows { + for i, step := range w.Steps { + p := step.Properties + policies, properties, err := extractPolicyListAndProperty(p) + if err != nil { + return err + } + if policies == nil || properties == nil { + policies = []string{} + properties = map[string]interface{}{} + } + var added, deleted bool + if ok := bindings[fmt.Sprintf(pattern, w.Name, step.Name)]; ok { + policies, added = guaranteePolicyExist(policies, policyName) + } else { + policies, deleted = guaranteePolicyNotExist(policies, policyName) + } + if added || deleted { + properties["policies"] = policies + pStr, err := json.Marshal(properties) + if err != nil { + return err + } + w.Steps[i].Properties = string(pStr) + needUpdate = true + } + } + if needUpdate { + _, err := c.WorkflowService.UpdateWorkflow(ctx, + &model.Workflow{ + BaseModel: model.BaseModel{CreateTime: w.CreateTime, UpdateTime: time.Now()}, + Description: w.Description, + Name: w.Name, + Alias: w.Alias, + Default: &w.Default, + AppPrimaryKey: app.Name, + EnvName: w.EnvName, + }, apisv1.UpdateWorkflowRequest{Steps: w.Steps, Description: w.Description}) + if err != nil { + return err + } + } + } + return nil +} + +// checkPolicyUsedByWorkflow check a policy whether used by any workflow step +func (c *applicationServiceImpl) checkPolicyUsedByWorkflow(ctx context.Context, app *model.Application, policyName string) (bool, error) { + workflows, err := c.WorkflowService.ListApplicationWorkflow(ctx, app) + if err != nil { + return false, err + } + for _, w := range workflows { + for _, step := range w.Steps { + p := step.Properties + policies, _, err := extractPolicyListAndProperty(p) + if err != nil { + return false, err + } + for _, policy := range policies { + if policy == policyName { + return true, nil + } + } + } + } + return false, nil +} diff --git a/pkg/apiserver/domain/service/application_test.go b/pkg/apiserver/domain/service/application_test.go index 8f125d831..911ce19f1 100644 --- a/pkg/apiserver/domain/service/application_test.go +++ b/pkg/apiserver/domain/service/application_test.go @@ -18,6 +18,7 @@ package service import ( "context" + "encoding/json" "fmt" "os" "strings" @@ -793,6 +794,195 @@ var _ = Describe("Test application component service function", func() { }) }) +var _ = Describe("Test apiserver policy rest api", func() { + var ( + appService *applicationServiceImpl + projectService *projectServiceImpl + envService *envServiceImpl + testApp string + testProject string + ctx context.Context + ) + + BeforeEach(func() { + ctx = context.Background() + ds, err := NewDatastore(datastore.Config{Type: "kubeapi", Database: "app-test-kubevela"}) + Expect(ds).ToNot(BeNil()) + Expect(err).Should(BeNil()) + rbacService := &rbacServiceImpl{Store: ds} + projectService = &projectServiceImpl{Store: ds, K8sClient: k8sClient, RbacService: rbacService} + envService = &envServiceImpl{Store: ds, KubeClient: k8sClient, ProjectService: projectService} + workflowService := &workflowServiceImpl{Store: ds, EnvService: envService} + envBindingService := &envBindingServiceImpl{Store: ds, EnvService: envService, WorkflowService: workflowService, KubeClient: k8sClient} + + appService = &applicationServiceImpl{ + Store: ds, + Apply: apply.NewAPIApplicator(k8sClient), + KubeClient: k8sClient, + ProjectService: projectService, + WorkflowService: workflowService, + EnvBindingService: envBindingService, + EnvService: envService, + } + testApp = "app-policy-workflow-binding" + testProject = "project-policy-workflow-binding" + }) + + It("Test add policy", func() { + _, err := projectService.CreateProject(context.TODO(), v1.CreateProjectRequest{Name: testProject}) + Expect(err).Should(BeNil()) + _, err = appService.CreateApplication(context.TODO(), v1.CreateApplicationRequest{Name: testApp, Project: testProject}) + Expect(err).Should(BeNil()) + appModel, err := appService.GetApplication(context.TODO(), testApp) + Expect(err).Should(BeNil()) + + workflow := v1.CreateWorkflowRequest{ + Name: "default", + EnvName: "default", + Steps: []v1.WorkflowStep{ + { + Name: "default", + Type: "deploy", + Properties: `{"policies":["local"]}`, + }, + { + Name: "suspend", + Type: "suspend", + Properties: `{"duration": "10m"}`, + }, + { + Name: "second", + Type: "deploy", + Properties: `{"policies":["cluster1"]}`, + }, + }, + } + _, err = appService.WorkflowService.CreateOrUpdateWorkflow(ctx, appModel, workflow) + Expect(err).Should(BeNil()) + + workflow2 := v1.CreateWorkflowRequest{ + Name: "second", + EnvName: "default", + Steps: []v1.WorkflowStep{ + { + Name: "second", + Type: "deploy", + Properties: `{"policies":["cluster3"]}`, + }, + }, + } + _, err = appService.WorkflowService.CreateOrUpdateWorkflow(ctx, appModel, workflow2) + Expect(err).Should(BeNil()) + + policyReq := v1.CreatePolicyRequest{ + Name: "override1", + Type: "override", + Properties: `{"components": [{"image": "busybox","cmd":["sleep", "1000"],"lives": "3","enemies": "alien"}]}`, + WorkflowPolicyBindings: []v1.WorkflowPolicyBinding{ + { + Name: "default", + Steps: []string{"default"}, + }, + }, + } + _, err = appService.CreatePolicy(ctx, appModel, policyReq) + Expect(err).Should(BeNil()) + + checkWorkflow, err := appService.WorkflowService.GetWorkflow(ctx, appModel, "default") + Expect(err).Should(BeNil()) + checkRes, err := json.Marshal(checkWorkflow.Steps[0].Properties) + Expect(err).Should(BeNil()) + Expect(string(checkRes)).Should(BeEquivalentTo(`{"policies":["local","override1"]}`)) + + // guarantee the suspend workflow step shouldn't be changed + suspendStep := checkWorkflow.Steps[1] + Expect(suspendStep.Name).Should(BeEquivalentTo("suspend")) + Expect(suspendStep.Type).Should(BeEquivalentTo("suspend")) + suspendPropertyStr, err := json.Marshal(suspendStep.Properties) + Expect(err).Should(BeNil()) + Expect(string(suspendPropertyStr)).Should(BeEquivalentTo(`{"duration":"10m"}`)) + }) + + It("Update policy to more workflow Step", func() { + appModel, err := appService.GetApplication(context.TODO(), testApp) + Expect(err).Should(BeNil()) + policyName := "override1" + policyRes, err := appService.DetailPolicy(ctx, appModel, policyName) + Expect(err).Should(BeNil()) + propertyStr, err := json.Marshal(policyRes.Properties) + Expect(err).Should(BeNil()) + updatePolicyReq := v1.UpdatePolicyRequest{ + Description: policyRes.Description, + Type: policyRes.Type, + Properties: string(propertyStr), + WorkflowPolicyBindings: []v1.WorkflowPolicyBinding{ + { + Name: "second", + Steps: []string{"second"}, + }, + }, + } + _, err = appService.UpdatePolicy(ctx, appModel, policyName, updatePolicyReq) + Expect(err).Should(BeNil()) + + checkWorkflow, err := appService.WorkflowService.GetWorkflow(ctx, appModel, "default") + Expect(err).Should(BeNil()) + checkRes, err := json.Marshal(checkWorkflow.Steps[0].Properties) + Expect(err).Should(BeNil()) + Expect(string(checkRes)).Should(BeEquivalentTo(`{"policies":["local"]}`)) + + checkWorkflow, err = appService.WorkflowService.GetWorkflow(ctx, appModel, "second") + Expect(err).Should(BeNil()) + checkRes, err = json.Marshal(checkWorkflow.Steps[0].Properties) + Expect(err).Should(BeNil()) + Expect(string(checkRes)).Should(BeEquivalentTo(`{"policies":["cluster3","override1"]}`)) + }) + + It("Exsit binding will block policy delete operation", func() { + appModel, err := appService.GetApplication(context.TODO(), testApp) + Expect(err).Should(BeNil()) + policyName := "override1" + _, err = appService.DetailPolicy(ctx, appModel, policyName) + Expect(err).Should(BeNil()) + err = appService.DeletePolicy(ctx, appModel, policyName) + Expect(err).ShouldNot(BeNil()) + }) + + It("Update workflow delete using step will unblock delete", func() { + appModel, err := appService.GetApplication(context.TODO(), testApp) + Expect(err).Should(BeNil()) + policyName := "override1" + policyRes, err := appService.DetailPolicy(ctx, appModel, policyName) + Expect(err).Should(BeNil()) + propertyStr, err := json.Marshal(policyRes.Properties) + Expect(err).Should(BeNil()) + updatePolicyReq := v1.UpdatePolicyRequest{ + Description: policyRes.Description, + Type: policyRes.Type, + Properties: string(propertyStr), + WorkflowPolicyBindings: nil, + } + _, err = appService.UpdatePolicy(ctx, appModel, policyName, updatePolicyReq) + Expect(err).Should(BeNil()) + + checkWorkflow, err := appService.WorkflowService.GetWorkflow(ctx, appModel, "default") + Expect(err).Should(BeNil()) + checkRes, err := json.Marshal(checkWorkflow.Steps[0].Properties) + Expect(err).Should(BeNil()) + Expect(string(checkRes)).Should(BeEquivalentTo(`{"policies":["local"]}`)) + + checkWorkflow, err = appService.WorkflowService.GetWorkflow(ctx, appModel, "second") + Expect(err).Should(BeNil()) + checkRes, err = json.Marshal(checkWorkflow.Steps[0].Properties) + Expect(err).Should(BeNil()) + Expect(string(checkRes)).Should(BeEquivalentTo(`{"policies":["cluster3"]}`)) + + // try delete again + err = appService.DeletePolicy(ctx, appModel, policyName) + Expect(err).Should(BeNil()) + }) +}) + func createTestSuspendApp(ctx context.Context, appName, envName, revisionVersion, wfName, recordName string, kubeClient client.Client) (*v1beta1.Application, error) { testapp := &v1beta1.Application{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/apiserver/domain/service/util.go b/pkg/apiserver/domain/service/util.go new file mode 100644 index 000000000..2292de749 --- /dev/null +++ b/pkg/apiserver/domain/service/util.go @@ -0,0 +1,76 @@ +/* +Copyright 2022 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "fmt" + + "encoding/json" +) + +// guaranteePolicyExist check the slice whether contain the target policy, if not put it in. +// and tell invoker whether should update the policy +func guaranteePolicyExist(c []string, policy string) ([]string, bool) { + for _, p := range c { + if policy == p { + return c, false + } + } + return append(c, policy), true +} + +// guaranteePolicyNotExist check the slice whether caontain the target policy, if yes delete +// and tell invoker whether should update the policy +func guaranteePolicyNotExist(c []string, policy string) ([]string, bool) { + res := make([]string, len(c)) + i := 0 + for _, p := range c { + if p != policy { + res[i] = p + i++ + } + } + // if len(c) != i, that's mean target policy exist in the list, this function has delete it from returned result, + // and outer caller should update with it. + return res[:i], len(c) != i +} + +// extractPolicyListAndProperty can extract policy from string-format properties, and return +// map-format properties in order to further update operation. +func extractPolicyListAndProperty(property string) ([]string, map[string]interface{}, error) { + content := map[string]interface{}{} + err := json.Unmarshal([]byte(property), &content) + if err != nil { + return nil, nil, err + } + policies := content["policies"] + if policies == nil { + return nil, content, nil + } + list, ok := policies.([]interface{}) + if !ok { + return nil, nil, fmt.Errorf("the policies incorrrect") + } + if len(list) == 0 { + return nil, content, nil + } + res := []string{} + for _, i := range list { + res = append(res, i.(string)) + } + return res, content, nil +} diff --git a/pkg/apiserver/domain/service/util_test.go b/pkg/apiserver/domain/service/util_test.go new file mode 100644 index 000000000..5b922fc01 --- /dev/null +++ b/pkg/apiserver/domain/service/util_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2022 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "testing" + + "gotest.tools/assert" +) + +func TestGuaranteePolicyNotExist(t *testing.T) { + testCases := []struct { + name string + input struct { + list []string + p string + } + res struct { + res []string + needUpdate bool + } + }{ + { + name: "containOne", + input: struct { + list []string + p string + }{list: []string{"policy1", "policy2"}, p: "policy2"}, + res: struct { + res []string + needUpdate bool + }{ + res: []string{"policy1"}, + needUpdate: true, + }, + }, + { + name: "containMulti", + input: struct { + list []string + p string + }{list: []string{"policy1", "policy2", "policy3", "policy2"}, p: "policy2"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy1", "policy3"}, needUpdate: true}, + }, + { + name: "not-contain", + input: struct { + list []string + p string + }{list: []string{"policy1", "policy3"}, p: "policy2"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy1", "policy3"}, needUpdate: false}, + }, + { + name: "first-element", + input: struct { + list []string + p string + }{list: []string{"policy1", "policy3"}, p: "policy1"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy3"}, needUpdate: true}, + }, + { + name: "only-one", + input: struct { + list []string + p string + }{list: []string{"policy1"}, p: "policy1"}, + res: struct { + res []string + needUpdate bool + }{res: []string{}, needUpdate: true}, + }, + } + for _, s := range testCases { + t.Run(s.name, func(t *testing.T) { + res, needUpdate := guaranteePolicyNotExist(s.input.list, s.input.p) + assert.DeepEqual(t, res, s.res.res) + assert.Equal(t, needUpdate, s.res.needUpdate) + }) + } +} + +func TestGuaranteePolicyExist(t *testing.T) { + testCases := []struct { + name string + input struct { + list []string + policy string + } + res struct { + res []string + needUpdate bool + } + }{ + { + name: "not-contain", + input: struct { + list []string + policy string + }{list: []string{"policy1", "policy2"}, policy: "policy3"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy1", "policy2", "policy3"}, needUpdate: true}, + }, + { + name: "contain-already", + input: struct { + list []string + policy string + }{list: []string{"policy1", "policy2"}, policy: "policy2"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy1", "policy2"}, needUpdate: false}, + }, + { + name: "empty", + input: struct { + list []string + policy string + }{list: []string{}, policy: "policy2"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy2"}, needUpdate: true}, + }, + { + name: "nil slice", + input: struct { + list []string + policy string + }{list: nil, policy: "policy2"}, + res: struct { + res []string + needUpdate bool + }{res: []string{"policy2"}, needUpdate: true}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + res, needUpdate := guaranteePolicyExist(testCase.input.list, testCase.input.policy) + assert.DeepEqual(t, res, testCase.res.res) + assert.Equal(t, needUpdate, testCase.res.needUpdate) + }) + } +} + +func TestExtractPolicyListAndProperty(t *testing.T) { + testCases := []struct { + input string + res struct { + policies []string + properties map[string]interface{} + noError bool + } + }{ + { + input: `{"policies":["policy1","policy2"], "components": ["comp1"]}`, + res: struct { + policies []string + properties map[string]interface{} + noError bool + }{policies: []string{"policy1", "policy2"}, properties: map[string]interface{}{ + "policies": []interface{}{"policy1", "policy2"}, + "components": []interface{}{"comp1"}, + }, noError: true}, + }, + { + input: `{"policies":["policy1"], "components": ["comp1"]}`, + res: struct { + policies []string + properties map[string]interface{} + noError bool + }{policies: []string{"policy1"}, properties: map[string]interface{}{ + "policies": []interface{}{"policy1"}, + "components": []interface{}{"comp1"}, + }, noError: true}, + }, + { + input: `{"policies":["policy1", "components": ["comp1"]}`, + res: struct { + policies []string + properties map[string]interface{} + noError bool + }{noError: false}, + }, + } + for _, testCase := range testCases { + policy, properties, err := extractPolicyListAndProperty(testCase.input) + if testCase.res.noError { + assert.NilError(t, err) + } else { + assert.Equal(t, err != nil, true) + } + assert.DeepEqual(t, policy, testCase.res.policies) + assert.DeepEqual(t, properties, testCase.res.properties) + } +} diff --git a/pkg/apiserver/interfaces/api/dto/v1/types.go b/pkg/apiserver/interfaces/api/dto/v1/types.go index e5784c33f..ba7864a1b 100644 --- a/pkg/apiserver/interfaces/api/dto/v1/types.go +++ b/pkg/apiserver/interfaces/api/dto/v1/types.go @@ -883,6 +883,15 @@ type CreatePolicyRequest struct { // Properties json data Properties string `json:"properties"` + + // Bind this policy to workflow + WorkflowPolicyBindings []WorkflowPolicyBinding `json:"workflowPolicyBind"` +} + +// WorkflowPolicyBinding define the relation binding relationShip between policy and workflowStep +type WorkflowPolicyBinding struct { + Name string `json:"name"` + Steps []string `json:"steps"` } // UpdatePolicyRequest update policy @@ -891,6 +900,9 @@ type UpdatePolicyRequest struct { Type string `json:"type" validate:"checkname"` // Properties json data Properties string `json:"properties"` + + // Bind this policy to workflow + WorkflowPolicyBindings []WorkflowPolicyBinding `json:"workflowPolicyBind"` } // PolicyBase application policy base info diff --git a/pkg/apiserver/utils/bcode/001_application.go b/pkg/apiserver/utils/bcode/001_application.go index 8b1962c4b..78c720b14 100644 --- a/pkg/apiserver/utils/bcode/001_application.go +++ b/pkg/apiserver/utils/bcode/001_application.go @@ -93,3 +93,6 @@ var ErrApplicationTriggerNotExist = NewBcode(404, 10024, "application trigger is // ErrApplicationComponentNotAllowDelete means the component is main in one application, and it must be deleted before delete app. var ErrApplicationComponentNotAllowDelete = NewBcode(400, 10025, "main component in application can not be deleted") + +// ErrApplicationPolicyIsBeingUsed means this policy is been used, cannot deleted. +var ErrApplicationPolicyIsBeingUsed = NewBcode(400, 10026, "the policy is being used by workflow, cannot be deleted")