From bf30f9939f62696a9ddd26ad85e30b62aecfb227 Mon Sep 17 00:00:00 2001 From: Richard LT Date: Wed, 17 Mar 2021 17:07:44 +0100 Subject: [PATCH] feat(api): export workflow purge info (#5769) --- engine/api/workflow/dao.go | 30 ++++++++++++----- engine/api/workflow/workflow_importer.go | 5 --- engine/api/workflow_export_test.go | 14 ++++---- sdk/exportentities/v1/workflow.go | 2 -- sdk/exportentities/v1/workflow_test.go | 42 ++++++++++------------- sdk/exportentities/v2/workflow.go | 14 +++++--- sdk/exportentities/v2/workflow_test.go | 43 +++++++++++------------- sdk/workflow.go | 3 +- 8 files changed, 75 insertions(+), 78 deletions(-) diff --git a/engine/api/workflow/dao.go b/engine/api/workflow/dao.go index 871e143d58..813c30f61a 100644 --- a/engine/api/workflow/dao.go +++ b/engine/api/workflow/dao.go @@ -27,10 +27,6 @@ import ( "github.com/ovh/cds/sdk/telemetry" ) -const ( - DefaultRetentionRule = "return (git_branch_exist == \"false\" and run_days_before < 2) or run_days_before < 365" -) - type PushSecrets struct { ApplicationsSecrets map[int64][]sdk.Variable EnvironmentdSecrets map[int64][]sdk.Variable @@ -312,8 +308,10 @@ func Insert(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St if w.HistoryLength == 0 { w.HistoryLength = sdk.DefaultHistoryLength } + if w.RetentionPolicy == "" { + w.RetentionPolicy = sdk.DefaultRetentionRule + } w.MaxRuns = maxRuns - w.RetentionPolicy = DefaultRetentionRule w.LastModified = time.Now() if err := db.QueryRow(`INSERT INTO workflow ( @@ -611,10 +609,6 @@ func Update(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St return err } - if wf.RetentionPolicy == "" { - wf.RetentionPolicy = DefaultRetentionRule - } - if err := CheckValidity(ctx, db, wf); err != nil { return err } @@ -633,6 +627,24 @@ func Update(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St return sdk.WrapError(err, "Unable to load existing workflow with proj:%s ID:%d", proj.Key, wf.ID) } + // Set or keep HistoryLength + if wf.HistoryLength == 0 { + if oldWf.HistoryLength == 0 { + wf.HistoryLength = sdk.DefaultHistoryLength + } else { + wf.HistoryLength = oldWf.HistoryLength + } + } + + // Set or keep RetentionPolicy + if wf.RetentionPolicy == "" { + if oldWf.RetentionPolicy == "" { + wf.RetentionPolicy = sdk.DefaultRetentionRule + } else { + wf.RetentionPolicy = oldWf.RetentionPolicy + } + } + // Keep MaxRun wf.MaxRuns = oldWf.MaxRuns diff --git a/engine/api/workflow/workflow_importer.go b/engine/api/workflow/workflow_importer.go index 49f8d6994d..4d0f34cf7e 100644 --- a/engine/api/workflow/workflow_importer.go +++ b/engine/api/workflow/workflow_importer.go @@ -25,11 +25,6 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St w.ProjectKey = proj.Key w.ProjectID = proj.ID - // Default value of history length is 20 - if w.HistoryLength == 0 { - w.HistoryLength = 20 - } - if w.WorkflowData.Node.Context == nil { w.WorkflowData.Node.Context = &sdk.NodeContext{} } diff --git a/engine/api/workflow_export_test.go b/engine/api/workflow_export_test.go index 234f8a44c7..0bc3d4670f 100644 --- a/engine/api/workflow_export_test.go +++ b/engine/api/workflow_export_test.go @@ -145,9 +145,7 @@ workflow: depends_on: - fork pipeline: pip1 -retention_policy: return (git_branch_exist == "false" and run_days_before < 2) or run_days_before < 365 `, rec.Body.String()) - } func Test_getWorkflowExportHandlerWithPermissions(t *testing.T) { @@ -209,10 +207,11 @@ func Test_getWorkflowExportHandlerWithPermissions(t *testing.T) { })) w := sdk.Workflow{ - Name: "test_1", - ProjectID: proj.ID, - ProjectKey: proj.Key, - HistoryLength: 25, + Name: "test_1", + ProjectID: proj.ID, + ProjectKey: proj.Key, + RetentionPolicy: "return true", + HistoryLength: 25, Groups: []sdk.GroupPermission{ { Group: *group2, @@ -279,10 +278,9 @@ workflow: pipeline: pip1 permissions: Test_getWorkflowExportHandlerWithPermissions-Group2: 7 -retention_policy: return (git_branch_exist == "false" and run_days_before < 2) or run_days_before < 365 +retention_policy: return true history_length: 25 `, rec.Body.String()) - } func Test_getWorkflowPullHandler(t *testing.T) { diff --git a/sdk/exportentities/v1/workflow.go b/sdk/exportentities/v1/workflow.go index d530891220..a90f865834 100644 --- a/sdk/exportentities/v1/workflow.go +++ b/sdk/exportentities/v1/workflow.go @@ -205,8 +205,6 @@ func (w Workflow) GetWorkflow() (*sdk.Workflow, error) { } if w.HistoryLength != nil && *w.HistoryLength > 0 { wf.HistoryLength = *w.HistoryLength - } else { - wf.HistoryLength = sdk.DefaultHistoryLength } rand.Seed(time.Now().Unix()) diff --git a/sdk/exportentities/v1/workflow_test.go b/sdk/exportentities/v1/workflow_test.go index 370513be38..329ff5d47e 100644 --- a/sdk/exportentities/v1/workflow_test.go +++ b/sdk/exportentities/v1/workflow_test.go @@ -229,8 +229,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "pipeline", @@ -266,9 +265,8 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, - Description: "this is my description", + Name: "myworkflow", + Description: "this is my description", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "pipeline", @@ -331,8 +329,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -396,8 +393,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -511,8 +507,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -601,8 +596,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "A", @@ -732,8 +726,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "pipeline", @@ -767,8 +760,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myworkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myworkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "A", @@ -838,17 +830,17 @@ func TestWorkflow_GetWorkflow(t *testing.T) { got.Environments = nil got.ProjectIntegrations = nil - expextedValues, _ := dump.ToStringMap(tt.want) + expectedValues, _ := dump.ToStringMap(tt.want) actualValues, _ := dump.ToStringMap(got) - var keysExpextedValues []string - for k := range expextedValues { - keysExpextedValues = append(keysExpextedValues, k) + var keysExpectedValues []string + for k := range expectedValues { + keysExpectedValues = append(keysExpectedValues, k) } - sort.Strings(keysExpextedValues) + sort.Strings(keysExpectedValues) - for _, expectedKey := range keysExpextedValues { - expectedValue := expextedValues[expectedKey] + for _, expectedKey := range keysExpectedValues { + expectedValue := expectedValues[expectedKey] actualValue, ok := actualValues[expectedKey] if strings.Contains(expectedKey, ".Ref") { assert.NotEmpty(t, actualValue, "value %s is empty but shoud not be empty", expectedKey) @@ -859,7 +851,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { } for actualKey := range actualValues { - _, ok := expextedValues[actualKey] + _, ok := expectedValues[actualKey] assert.True(t, ok, "got %s, but not found is expected workflow", actualKey) } }) diff --git a/sdk/exportentities/v2/workflow.go b/sdk/exportentities/v2/workflow.go index 5626641f92..e26873134a 100644 --- a/sdk/exportentities/v2/workflow.go +++ b/sdk/exportentities/v2/workflow.go @@ -26,7 +26,7 @@ type Workflow struct { Permissions map[string]int `json:"permissions,omitempty" yaml:"permissions,omitempty" jsonschema_description:"The permissions for the workflow (ex: myGroup: 7).\nhttps://ovh.github.io/cds/docs/concepts/permissions"` Metadata map[string]string `json:"metadata,omitempty" yaml:"metadata,omitempty"` PurgeTags []string `json:"purge_tags,omitempty" yaml:"purge_tags,omitempty"` - RetentionPolicy string `json:"retention_policy,omitempty" yaml:"retention_policy,omitempty"` + RetentionPolicy *string `json:"retention_policy,omitempty" yaml:"retention_policy,omitempty"` Notifications []NotificationEntry `json:"notifications,omitempty" yaml:"notifications,omitempty"` // This is used when the workflow have only one pipeline HistoryLength *int64 `json:"history_length,omitempty" yaml:"history_length,omitempty"` } @@ -104,7 +104,7 @@ func NewWorkflow(ctx context.Context, w sdk.Workflow, version string, opts ...Ex exportedWorkflow.Version = version exportedWorkflow.Workflow = map[string]NodeEntry{} exportedWorkflow.Hooks = map[string][]HookEntry{} - exportedWorkflow.RetentionPolicy = w.RetentionPolicy + if len(w.Metadata) > 0 { exportedWorkflow.Metadata = make(map[string]string, len(w.Metadata)) for k, v := range w.Metadata { @@ -119,6 +119,10 @@ func NewWorkflow(ctx context.Context, w sdk.Workflow, version string, opts ...Ex exportedWorkflow.HistoryLength = &w.HistoryLength } + if w.RetentionPolicy != "" && w.RetentionPolicy != sdk.DefaultRetentionRule { + exportedWorkflow.RetentionPolicy = &w.RetentionPolicy + } + exportedWorkflow.PurgeTags = w.PurgeTags nodes := w.WorkflowData.Array() @@ -371,7 +375,6 @@ func (w Workflow) GetWorkflow() (*sdk.Workflow, error) { wf.Pipelines = make(map[int64]sdk.Pipeline) wf.Environments = make(map[int64]sdk.Environment) wf.ProjectIntegrations = make(map[int64]sdk.ProjectIntegration) - wf.RetentionPolicy = w.RetentionPolicy if err := w.CheckValidity(); err != nil { return nil, sdk.WrapError(err, "unable to check validity") @@ -388,8 +391,9 @@ func (w Workflow) GetWorkflow() (*sdk.Workflow, error) { } if w.HistoryLength != nil && *w.HistoryLength > 0 { wf.HistoryLength = *w.HistoryLength - } else { - wf.HistoryLength = sdk.DefaultHistoryLength + } + if w.RetentionPolicy != nil && *w.RetentionPolicy != "" { + wf.RetentionPolicy = *w.RetentionPolicy } r := rand.New(rand.NewSource(time.Now().Unix())) diff --git a/sdk/exportentities/v2/workflow_test.go b/sdk/exportentities/v2/workflow_test.go index 9bd02be712..7b7bb293cc 100644 --- a/sdk/exportentities/v2/workflow_test.go +++ b/sdk/exportentities/v2/workflow_test.go @@ -188,8 +188,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myWorkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myWorkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -254,8 +253,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myWorkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myWorkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -317,12 +315,14 @@ func TestWorkflow_GetWorkflow(t *testing.T) { PipelineName: "pipeline-root", }, }, - HistoryLength: 25, + HistoryLength: 25, + RetentionPolicy: "return true", }, wantErr: false, want: sdk.Workflow{ - Name: "myWorkflow", - HistoryLength: 25, + Name: "myWorkflow", + HistoryLength: 25, + RetentionPolicy: "return true", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -369,8 +369,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myWorkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myWorkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "root", @@ -459,8 +458,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myWorkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myWorkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "A", @@ -600,8 +598,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { }, wantErr: false, want: sdk.Workflow{ - Name: "myWorkflow", - HistoryLength: sdk.DefaultHistoryLength, + Name: "myWorkflow", WorkflowData: sdk.WorkflowData{ Node: sdk.Node{ Name: "A", @@ -643,7 +640,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { Hooks: tt.fields.Hooks, Permissions: tt.fields.Permissions, HistoryLength: &tt.fields.HistoryLength, - RetentionPolicy: tt.fields.RetentionPolicy, + RetentionPolicy: &tt.fields.RetentionPolicy, } got, err := exportentities.ParseWorkflow(w) if (err != nil) != tt.wantErr { @@ -663,17 +660,17 @@ func TestWorkflow_GetWorkflow(t *testing.T) { got.Environments = nil got.ProjectIntegrations = nil - expextedValues, _ := dump.ToStringMap(tt.want) + expectedValues, _ := dump.ToStringMap(tt.want) actualValues, _ := dump.ToStringMap(got) - var keysExpextedValues []string - for k := range expextedValues { - keysExpextedValues = append(keysExpextedValues, k) + var keysExpectedValues []string + for k := range expectedValues { + keysExpectedValues = append(keysExpectedValues, k) } - sort.Strings(keysExpextedValues) + sort.Strings(keysExpectedValues) - for _, expectedKey := range keysExpextedValues { - expectedValue := expextedValues[expectedKey] + for _, expectedKey := range keysExpectedValues { + expectedValue := expectedValues[expectedKey] actualValue, ok := actualValues[expectedKey] if strings.Contains(expectedKey, ".Ref") { assert.NotEmpty(t, actualValue, "value %s is empty but shoud not be empty", expectedKey) @@ -684,7 +681,7 @@ func TestWorkflow_GetWorkflow(t *testing.T) { } for actualKey := range actualValues { - _, ok := expextedValues[actualKey] + _, ok := expectedValues[actualKey] assert.True(t, ok, "got %s, but not found is expected workflow", actualKey) } }) @@ -1133,7 +1130,7 @@ workflow: script: return cds_manual == "true" one_at_a_time: true metadata: - default_tags: git.branch,git.author + default_tags: git.branch,git.author notifications: - type: vcs settings: diff --git a/sdk/workflow.go b/sdk/workflow.go index 4dcb530ee5..60de67ad48 100644 --- a/sdk/workflow.go +++ b/sdk/workflow.go @@ -13,7 +13,8 @@ import ( // DefaultHistoryLength is the default history length const ( - DefaultHistoryLength int64 = 20 + DefaultHistoryLength int64 = 20 + DefaultRetentionRule string = "return (git_branch_exist == \"false\" and run_days_before < 2) or run_days_before < 365" ) // ColorRegexp represent the regexp for a format to hexadecimal color