From 971205a560c6b54a03ca27184b1e3da944c95757 Mon Sep 17 00:00:00 2001 From: Bong Nguyen Date: Fri, 1 Apr 2022 21:12:41 +1100 Subject: [PATCH 1/2] feat: improve depends_on in workspace steps --- .golangci.yml | 13 +++ Makefile | 2 +- git.go | 4 +- go.mod | 1 + go.sum | 3 +- main.go | 4 +- pipeline.go | 30 ++++++- pipeline_test.go | 165 +++++++++++++++++++++++++++++++++++++ tests/dynamic_pipeline.yml | 4 + tests/post-command.bats | 6 ++ 10 files changed, 225 insertions(+), 7 deletions(-) create mode 100644 .golangci.yml create mode 100644 pipeline_test.go diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..e56df43 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,13 @@ +linters: + disable: + - gocritic + - gosec + + enable: + - govet + - errcheck + - megacheck + - misspell + - goconst + - revive + - goimports diff --git a/Makefile b/Makefile index 12230de..6124804 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ quality: go fmt go mod tidy ifneq (${DOCKER},) - docker run -v ${PWD}:/src -w /src -it golangci/golangci-lint golangci-lint run --true gocritic --true gosec --true golint --true stylecheck --exclude-use-default=false + docker run -v ${PWD}:/src -w /src -it golangci/golangci-lint golangci-lint run endif .PHONY: test diff --git a/git.go b/git.go index ac14f65..4c371af 100644 --- a/git.go +++ b/git.go @@ -23,7 +23,7 @@ func dedupe(list []string) []string { for _, item := range list { _, ok := set[item] - if ok == false { + if !ok { set[item] = true unique = append(unique, item) } @@ -64,7 +64,7 @@ func determineGitArgs(branch string, defaultBranch string) []string { } func index(slice []string, item string) int { - for i, _ := range slice { + for i := range slice { if slice[i] == item { return i } diff --git a/go.mod b/go.mod index 1c176ab..a1c5d07 100644 --- a/go.mod +++ b/go.mod @@ -6,5 +6,6 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/sirupsen/logrus v1.5.0 github.com/stretchr/testify v1.4.0 + golang.org/x/sys v0.0.0-20220330033206-e17cdc41300f // indirect gopkg.in/yaml.v2 v2.2.2 ) diff --git a/go.sum b/go.sum index 9907eb6..9cd784c 100644 --- a/go.sum +++ b/go.sum @@ -13,8 +13,9 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -golang.org/x/sys v0.0.0-20190422165155-953cdadca894 h1:Cz4ceDQGXuKRnVBDTS23GTn/pU5OE2C0WrNTOYK1Uuc= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20220330033206-e17cdc41300f h1:rlezHXNlxYWvBCzNses9Dlc7nGFaNMJeqLolcmQSSZY= +golang.org/x/sys v0.0.0-20220330033206-e17cdc41300f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= diff --git a/main.go b/main.go index a63c090..bbece5d 100644 --- a/main.go +++ b/main.go @@ -54,7 +54,7 @@ func projectsFromBuildProjects(buildProjects string, projects []Project) []Proje for _, projectName := range projectNames { for _, configProject := range projects { if projectName == configProject.Label { - affectedProjects = append(affectedProjects, configProject) + affectedProjects = append(affectedProjects, configProject) } } } @@ -71,7 +71,7 @@ func main() { log.SetLevel(ll) config := NewConfig(os.Getenv(pluginPrefix + "DYNAMIC_PIPELINE")) - buildProjects := os.Getenv(pluginPrefix+"BUILD_PROJECTS") + buildProjects := os.Getenv(pluginPrefix + "BUILD_PROJECTS") var affectedProjects []Project if len(buildProjects) > 0 { diff --git a/pipeline.go b/pipeline.go index 58ff27c..5dbe768 100644 --- a/pipeline.go +++ b/pipeline.go @@ -121,6 +121,7 @@ func generatePipeline(steps []interface{}, pipelineEnv map[string]string, projec projectSteps := generateProjectSteps(steps, step, projects) generatedSteps = append(generatedSteps, projectSteps...) } else { + normaliseWorkspaceStep(stepMap, steps, projects) generatedSteps = append(generatedSteps, step) } } @@ -130,6 +131,33 @@ func generatePipeline(steps []interface{}, pipelineEnv map[string]string, projec } } +func normaliseWorkspaceStep(stepMap map[interface{}]interface{}, steps []interface{}, projects []Project) { + if val, ok := stepMap["depends_on"]; ok { + // depends_on can be an array or a string + inputDependencyList, ok := val.([]interface{}) + if !ok { + inputDependencyList = []interface{}{val} + } + + generatedDependencyList := []interface{}{} + for _, dependency := range inputDependencyList { + depStr := dependency.(string) + + if step := findStepByKey(steps, depStr); step != nil { + if isProjectScopeStep(step) { + for _, project := range projects { + generatedDependencyList = append(generatedDependencyList, fmt.Sprintf("%s:%s", depStr, project.Label)) + } + } else { + generatedDependencyList = append(generatedDependencyList, depStr) + } + } + } + + stepMap["depends_on"] = generatedDependencyList + } +} + func uploadPipeline(pipeline Pipeline) { tmpFile, err := ioutil.TempFile(os.TempDir(), "buildpipe-") if err != nil { @@ -137,7 +165,7 @@ func uploadPipeline(pipeline Pipeline) { } defer os.Remove(tmpFile.Name()) - data, err := yaml.Marshal(&pipeline) + data, _ := yaml.Marshal(&pipeline) fmt.Printf("Pipeline:\n%s", string(data)) diff --git a/pipeline_test.go b/pipeline_test.go new file mode 100644 index 0000000..f9fc4b7 --- /dev/null +++ b/pipeline_test.go @@ -0,0 +1,165 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v2" +) + +func TestNormaliseWorkspaceStep(t *testing.T) { + testCases := map[string]struct { + step string + steps string + projects []Project + expectedStep string + }{ + "should do nothing when there is no depends_on": { + step: ` +label: tag +branches: "master" +command: + - make tag-release`, + steps: ` +- label: build + branches: "master" + env: + BUILDPIPE_SCOPE: project + TEST_ENV_STEP: test-step + command: + - cd $$BUILDPIPE_PROJECT_PATH + - make build + - make publish-image + agents: + - queue=build + depends_on: + - bootstrap # the rendered template should not include the project name for a non-project step + - test # the rendered template should include the project name for a project-scoped step +- wait +- label: tag + branches: "master" + command: + - make tag-release`, + projects: []Project{ + { + Label: "projectA", + }, + { + Label: "projectB", + }, + }, + expectedStep: ` +label: tag +branches: "master" +command: + - make tag-release`, + }, + "should update depends_on when the dependant is a project step": { + step: ` +label: tag +branches: "master" +depends_on: + - build +command: + - make tag-release`, + steps: ` +- label: build + key: build + branches: "master" + env: + BUILDPIPE_SCOPE: project + TEST_ENV_STEP: test-step + command: + - cd $$BUILDPIPE_PROJECT_PATH + - make build + - make publish-image + agents: + - queue=build + depends_on: + - bootstrap # the rendered template should not include the project name for a non-project step + - test # the rendered template should include the project name for a project-scoped step +- wait +- label: tag + branches: "master" + command: + - make tag-release`, + projects: []Project{ + { + Label: "projectA", + }, + { + Label: "projectB", + }, + }, + expectedStep: ` +label: tag +branches: "master" +depends_on: + - build:projectA + - build:projectB +command: + - make tag-release`, + }, + "should not update depends_on when the dependant is a workspace step": { + step: ` +label: tag +branches: "master" +depends_on: + - build +command: + - make tag-release`, + steps: ` +- label: build + key: build + branches: "master" + env: + TEST_ENV_STEP: test-step + command: + - cd $$BUILDPIPE_PROJECT_PATH + - make build + - make publish-image + agents: + - queue=build + depends_on: + - bootstrap # the rendered template should not include the project name for a non-project step + - test # the rendered template should include the project name for a project-scoped step +- wait +- label: tag + branches: "master" + command: + - make tag-release`, + projects: []Project{ + { + Label: "projectA", + }, + { + Label: "projectB", + }, + }, + expectedStep: ` +label: tag +branches: "master" +depends_on: + - build +command: + - make tag-release`, + }, + } + + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + step := map[interface{}]interface{}{} + assert.NoError(t, yaml.Unmarshal([]byte(tc.step), step)) + + steps := []interface{}{} + assert.NoError(t, yaml.Unmarshal([]byte(tc.steps), &steps)) + + expectedStep := map[interface{}]interface{}{} + assert.NoError(t, yaml.Unmarshal([]byte(tc.expectedStep), expectedStep)) + + normaliseWorkspaceStep(step, steps, tc.projects) + assert.Equal(t, expectedStep, step) + }) + } +} diff --git a/tests/dynamic_pipeline.yml b/tests/dynamic_pipeline.yml index d079fc5..91ab2e8 100644 --- a/tests/dynamic_pipeline.yml +++ b/tests/dynamic_pipeline.yml @@ -30,6 +30,7 @@ steps: # the same schema as regular buildkite pipeline steps - make test - wait - label: build + key: build branches: "master" env: BUILDPIPE_SCOPE: project @@ -46,6 +47,9 @@ steps: # the same schema as regular buildkite pipeline steps - wait - label: tag branches: "master" + depends_on: + - bootstrap + - build command: - make tag-release - wait diff --git a/tests/post-command.bats b/tests/post-command.bats index 543c754..7443319 100755 --- a/tests/post-command.bats +++ b/tests/post-command.bats @@ -69,6 +69,7 @@ steps: TEST_ENV_PIPELINE: test-pipeline TEST_ENV_PROJECT: test-project TEST_ENV_STEP: test-step + key: build:project1 label: build project1 - agents: - queue=build @@ -86,11 +87,16 @@ steps: BUILDPIPE_SCOPE: project TEST_ENV_PIPELINE: test-pipeline TEST_ENV_STEP: test-step + key: build:project2 label: build project2 - wait - branches: master command: - make tag-release + depends_on: + - bootstrap + - build:project1 + - build:project2 env: TEST_ENV_PIPELINE: test-pipeline label: tag From 5907a5cbc1c99386c88b40a984837085c7fb9d31 Mon Sep 17 00:00:00 2001 From: Bong Nguyen Date: Sat, 2 Apr 2022 08:00:51 +1100 Subject: [PATCH 2/2] do not remove non-exist key --- pipeline.go | 12 ++++---- pipeline_test.go | 74 +++++++++++++++++++++++++++++++----------------- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/pipeline.go b/pipeline.go index 5dbe768..540b9e7 100644 --- a/pipeline.go +++ b/pipeline.go @@ -143,14 +143,12 @@ func normaliseWorkspaceStep(stepMap map[interface{}]interface{}, steps []interfa for _, dependency := range inputDependencyList { depStr := dependency.(string) - if step := findStepByKey(steps, depStr); step != nil { - if isProjectScopeStep(step) { - for _, project := range projects { - generatedDependencyList = append(generatedDependencyList, fmt.Sprintf("%s:%s", depStr, project.Label)) - } - } else { - generatedDependencyList = append(generatedDependencyList, depStr) + if step := findStepByKey(steps, depStr); step != nil && isProjectScopeStep(step) { + for _, project := range projects { + generatedDependencyList = append(generatedDependencyList, fmt.Sprintf("%s:%s", depStr, project.Label)) } + } else { + generatedDependencyList = append(generatedDependencyList, depStr) } } diff --git a/pipeline_test.go b/pipeline_test.go index f9fc4b7..48c0636 100644 --- a/pipeline_test.go +++ b/pipeline_test.go @@ -8,10 +8,18 @@ import ( ) func TestNormaliseWorkspaceStep(t *testing.T) { + projects := []Project{ + { + Label: "projectA", + }, + { + Label: "projectB", + }, + } + testCases := map[string]struct { step string steps string - projects []Project expectedStep string }{ "should do nothing when there is no depends_on": { @@ -40,14 +48,6 @@ command: branches: "master" command: - make tag-release`, - projects: []Project{ - { - Label: "projectA", - }, - { - Label: "projectB", - }, - }, expectedStep: ` label: tag branches: "master" @@ -83,14 +83,6 @@ command: branches: "master" command: - make tag-release`, - projects: []Project{ - { - Label: "projectA", - }, - { - Label: "projectB", - }, - }, expectedStep: ` label: tag branches: "master" @@ -128,19 +120,49 @@ command: branches: "master" command: - make tag-release`, - projects: []Project{ - { - Label: "projectA", - }, - { - Label: "projectB", - }, - }, + expectedStep: ` label: tag branches: "master" depends_on: - build +command: + - make tag-release`, + }, + "should not update depends_on when couldn't find a step for the dependent key": { + step: ` +label: tag +branches: "master" +depends_on: + - non_exist_key +command: + - make tag-release`, + steps: ` +- label: build + key: build + branches: "master" + env: + TEST_ENV_STEP: test-step + command: + - cd $$BUILDPIPE_PROJECT_PATH + - make build + - make publish-image + agents: + - queue=build + depends_on: + - bootstrap # the rendered template should not include the project name for a non-project step + - test # the rendered template should include the project name for a project-scoped step +- wait +- label: tag + branches: "master" + command: + - make tag-release`, + + expectedStep: ` +label: tag +branches: "master" +depends_on: + - non_exist_key command: - make tag-release`, }, @@ -158,7 +180,7 @@ command: expectedStep := map[interface{}]interface{}{} assert.NoError(t, yaml.Unmarshal([]byte(tc.expectedStep), expectedStep)) - normaliseWorkspaceStep(step, steps, tc.projects) + normaliseWorkspaceStep(step, steps, projects) assert.Equal(t, expectedStep, step) }) }