From 76639db1790e8bc37e520261424b779cf9b55f26 Mon Sep 17 00:00:00 2001 From: Sandro Gattuso Date: Tue, 6 Apr 2021 09:12:27 +1000 Subject: [PATCH 1/3] fix: propagation of attributes mode and fail_fast from originalYamlString --- client/pipeline.go | 1 + codefresh/resource_pipeline.go | 41 ++++++++++++++++++----------- codefresh/resource_pipeline_test.go | 2 +- codefresh/resource_step_types.go | 2 +- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/client/pipeline.go b/client/pipeline.go index ed14da0c..4240a68d 100644 --- a/client/pipeline.go +++ b/client/pipeline.go @@ -81,6 +81,7 @@ type Spec struct { Steps *Steps `json:"steps,omitempty"` Stages *Stages `json:"stages,omitempty"` Mode string `json:"mode,omitempty"` + FailFast bool `json:"fail_fast,omitempty"` RuntimeEnvironment RuntimeEnvironment `json:"runtimeEnvironment,omitempty"` TerminationPolicy []map[string]interface{} `json:"terminationPolicy,omitempty"` } diff --git a/codefresh/resource_pipeline.go b/codefresh/resource_pipeline.go index 100b1b6f..bc270445 100644 --- a/codefresh/resource_pipeline.go +++ b/codefresh/resource_pipeline.go @@ -575,13 +575,7 @@ func mapResourceToPipeline(d *schema.ResourceData) *cfClient.Pipeline { Context: d.Get("spec.0.spec_template.0.context").(string), } } else { - stages, steps := extractStagesAndSteps(originalYamlString) - pipeline.Spec.Steps = &cfClient.Steps{ - Steps: steps, - } - pipeline.Spec.Stages = &cfClient.Stages{ - Stages: stages, - } + extractSpecAttributesFromOriginalYamlString(originalYamlString, pipeline) } if _, ok := d.GetOk("spec.0.runtime_environment"); ok { @@ -659,10 +653,10 @@ func mapResourceToPipeline(d *schema.ResourceData) *cfClient.Pipeline { return pipeline } -// extractStagesAndSteps extracts the steps and stages from the original yaml string to enable propagation in the `Spec` attribute of the pipeline +// extractSpecAttributesFromOriginalYamlString extracts the steps and stages from the original yaml string to enable propagation in the `Spec` attribute of the pipeline // We cannot leverage on the standard marshal/unmarshal because the steps attribute needs to maintain the order of elements // while by default the standard function doesn't do it because in JSON maps are unordered -func extractStagesAndSteps(originalYamlString string) (stages, steps string) { +func extractSpecAttributesFromOriginalYamlString(originalYamlString string, pipeline *cfClient.Pipeline) { // Use mapSlice to preserve order of items from the YAML string m := yaml.MapSlice{} err := yaml.Unmarshal([]byte(originalYamlString), &m) @@ -670,13 +664,19 @@ func extractStagesAndSteps(originalYamlString string) (stages, steps string) { log.Fatal("Unable to unmarshall original_yaml_string") } - stages = "[]" + stages := "[]" // Dynamically build JSON object for steps using String builder stepsBuilder := strings.Builder{} stepsBuilder.WriteString("{") + // Dynamically build JSON object for steps using String builder + hooksBuilder := strings.Builder{} + hooksBuilder.WriteString("{") + // Parse elements of the YAML string to extract Steps and Stages if defined for _, item := range m { - if item.Key == "steps" { + key := item.Key.(string) + switch key { + case "steps": switch x := item.Value.(type) { default: log.Fatalf("unsupported value type: %T", item.Value) @@ -694,17 +694,28 @@ func extractStagesAndSteps(originalYamlString string) (stages, steps string) { } } } - } - if item.Key == "stages" { + case "stages": // For Stages we don't have ordering issue because it's a list y, _ := yaml.Marshal(item.Value) j2, _ := ghodss.YAMLToJSON(y) stages = string(j2) + case "mode": + pipeline.Spec.Mode = item.Value.(string) + case "fail_fast": + pipeline.Spec.FailFast = item.Value.(bool) + default: + log.Printf("Unsupported entry %s", key) } } stepsBuilder.WriteString("}") - steps = stepsBuilder.String() - return + steps := stepsBuilder.String() + pipeline.Spec.Steps = &cfClient.Steps{ + Steps: steps, + } + pipeline.Spec.Stages = &cfClient.Stages{ + Stages: stages, + } + } func getSupportedTerminationPolicyAttributes(policy string) map[string]interface{} { diff --git a/codefresh/resource_pipeline_test.go b/codefresh/resource_pipeline_test.go index 0229710b..f1486a38 100644 --- a/codefresh/resource_pipeline_test.go +++ b/codefresh/resource_pipeline_test.go @@ -165,7 +165,7 @@ func TestAccCodefreshPipeline_RuntimeEnvironment(t *testing.T) { func TestAccCodefreshPipeline_OriginalYamlString(t *testing.T) { name := pipelineNamePrefix + acctest.RandString(10) resourceName := "codefresh_pipeline.test" - originalYamlString := "version: \"1.0\"\nsteps:\n test:\n image: alpine:latest\n commands:\n - echo \"ACC tests\"" + originalYamlString := "version: \"1.0\"\nfail_fast: false\nmode: parallel\nsteps:\n test:\n image: alpine:latest\n commands:\n - echo \"ACC tests\"" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, diff --git a/codefresh/resource_step_types.go b/codefresh/resource_step_types.go index 46ff2fc0..85ef5a4e 100644 --- a/codefresh/resource_step_types.go +++ b/codefresh/resource_step_types.go @@ -382,7 +382,7 @@ func mapResourceToStepTypesVersions(d *schema.ResourceData) *cfClient.StepTypesV return &stepTypesVersions } -// extractStagesAndSteps extracts the steps and stages from the original yaml string to enable propagation in the `Spec` attribute of the pipeline +// extractSteps extracts the steps and stages from the original yaml string to enable propagation in the `Spec` attribute of the pipeline // We cannot leverage on the standard marshal/unmarshal because the steps attribute needs to maintain the order of elements // while by default the standard function doesn't do it because in JSON maps are unordered func extractSteps(stepTypesYaml string) (steps *orderedmap.OrderedMap) { From be434d847110cd42cf0eb525110bc6a6a665502a Mon Sep 17 00:00:00 2001 From: Sandro Gattuso Date: Thu, 15 Apr 2021 08:54:47 +1000 Subject: [PATCH 2/3] Add support for Hooks in original Yaml String --- client/context.go | 2 +- client/pipeline.go | 18 +++++- codefresh/resource_pipeline.go | 52 +++++++++++++++++- codefresh/resource_pipeline_test.go | 85 ++++++++++++++++++++++++++++- 4 files changed, 152 insertions(+), 5 deletions(-) diff --git a/client/context.go b/client/context.go index c28c2790..71d51d40 100644 --- a/client/context.go +++ b/client/context.go @@ -67,7 +67,7 @@ func (client *Client) CreateContext(context *Context) (*Context, error) { } resp, err := client.RequestAPI(&opts) - log.Printf("[DEBUG] Called API for context with Body %v", body) + if err != nil { log.Printf("[DEBUG] Call to API for context creation failed with Error = %v for Body %v", err, body) return nil, err diff --git a/client/pipeline.go b/client/pipeline.go index 4240a68d..d6616c1d 100644 --- a/client/pipeline.go +++ b/client/pipeline.go @@ -84,6 +84,7 @@ type Spec struct { FailFast bool `json:"fail_fast,omitempty"` RuntimeEnvironment RuntimeEnvironment `json:"runtimeEnvironment,omitempty"` TerminationPolicy []map[string]interface{} `json:"terminationPolicy,omitempty"` + Hooks *Hooks `json:"hooks,omitempty"` } type Steps struct { @@ -93,6 +94,10 @@ type Stages struct { Stages string } +type Hooks struct { + Hooks string +} + func (d Steps) MarshalJSON() ([]byte, error) { bytes := []byte(d.Steps) return bytes, nil @@ -102,14 +107,23 @@ func (d Stages) MarshalJSON() ([]byte, error) { return bytes, nil } -func (d Steps) UnmarshalJSON(data []byte) error { +func (d Hooks) MarshalJSON() ([]byte, error) { + bytes := []byte(d.Hooks) + return bytes, nil +} + +func (d *Steps) UnmarshalJSON(data []byte) error { d.Steps = string(data) return nil } -func (d Stages) UnmarshalJSON(data []byte) error { +func (d *Stages) UnmarshalJSON(data []byte) error { d.Stages = string(data) return nil } +func (d *Hooks) UnmarshalJSON(data []byte) error { + d.Hooks = string(data) + return nil +} type Pipeline struct { Metadata Metadata `json:"metadata,omitempty"` diff --git a/codefresh/resource_pipeline.go b/codefresh/resource_pipeline.go index bc270445..057260ce 100644 --- a/codefresh/resource_pipeline.go +++ b/codefresh/resource_pipeline.go @@ -661,7 +661,7 @@ func extractSpecAttributesFromOriginalYamlString(originalYamlString string, pipe m := yaml.MapSlice{} err := yaml.Unmarshal([]byte(originalYamlString), &m) if err != nil { - log.Fatal("Unable to unmarshall original_yaml_string") + log.Fatalf("Unable to unmarshall original_yaml_string. Error: %v", err) } stages := "[]" @@ -699,6 +699,51 @@ func extractSpecAttributesFromOriginalYamlString(originalYamlString string, pipe y, _ := yaml.Marshal(item.Value) j2, _ := ghodss.YAMLToJSON(y) stages = string(j2) + case "hooks": + switch hooks := item.Value.(type) { + default: + log.Fatalf("unsupported value type: %T", item.Value) + + case yaml.MapSlice: + numberOfHooks := len(hooks) + for indexHook, hook := range hooks { + // E.g. on_finish + hooksBuilder.WriteString("\"" + hook.Key.(string) + "\" : {") + numberOfAttributes := len(hook.Value.(yaml.MapSlice)) + for indexAttribute, hookAttribute := range hook.Value.(yaml.MapSlice) { + attribute := hookAttribute.Key.(string) + switch attribute { + case "steps": + hooksBuilder.WriteString("\"steps\" : {") + numberOfSteps := len(hookAttribute.Value.(yaml.MapSlice)) + for indexStep, step := range hookAttribute.Value.(yaml.MapSlice) { + // We only need to preserve order at the first level to guarantee order of the steps, hence the child nodes can be marshalled + // with the standard library + y, _ := yaml.Marshal(step.Value) + j2, _ := ghodss.YAMLToJSON(y) + hooksBuilder.WriteString("\"" + step.Key.(string) + "\" : " + string(j2)) + if indexStep < numberOfSteps-1 { + hooksBuilder.WriteString(",") + } + } + hooksBuilder.WriteString("}") + default: + // For Other elements we don't need to preserve order + y, _ := yaml.Marshal(hookAttribute.Value) + j2, _ := ghodss.YAMLToJSON(y) + hooksBuilder.WriteString("\"" + hookAttribute.Key.(string) + "\" : " + string(j2)) + } + + if indexAttribute < numberOfAttributes-1 { + hooksBuilder.WriteString(",") + } + } + hooksBuilder.WriteString("}") + if indexHook < numberOfHooks-1 { + hooksBuilder.WriteString(",") + } + } + } case "mode": pipeline.Spec.Mode = item.Value.(string) case "fail_fast": @@ -708,13 +753,18 @@ func extractSpecAttributesFromOriginalYamlString(originalYamlString string, pipe } } stepsBuilder.WriteString("}") + hooksBuilder.WriteString("}") steps := stepsBuilder.String() + hooks := hooksBuilder.String() pipeline.Spec.Steps = &cfClient.Steps{ Steps: steps, } pipeline.Spec.Stages = &cfClient.Stages{ Stages: stages, } + pipeline.Spec.Hooks = &cfClient.Hooks{ + Hooks: hooks, + } } diff --git a/codefresh/resource_pipeline_test.go b/codefresh/resource_pipeline_test.go index f1486a38..fc2d3f8f 100644 --- a/codefresh/resource_pipeline_test.go +++ b/codefresh/resource_pipeline_test.go @@ -2,6 +2,7 @@ package codefresh import ( "fmt" + "reflect" "regexp" "testing" @@ -165,7 +166,56 @@ func TestAccCodefreshPipeline_RuntimeEnvironment(t *testing.T) { func TestAccCodefreshPipeline_OriginalYamlString(t *testing.T) { name := pipelineNamePrefix + acctest.RandString(10) resourceName := "codefresh_pipeline.test" - originalYamlString := "version: \"1.0\"\nfail_fast: false\nmode: parallel\nsteps:\n test:\n image: alpine:latest\n commands:\n - echo \"ACC tests\"" + originalYamlString := `version: 1.0 +fail_fast: false +stages: + - test +mode: parallel +hooks: + on_finish: + steps: + secondmycleanup: + commands: + - echo echo cleanup step + image: alpine:3.9 + firstmynotification: + commands: + - echo Notify slack + image: cloudposse/slack-notifier + on_elected: + exec: + commands: + - echo 'Creating an adhoc test environment' + image: alpine:3.9 + annotations: + set: + - annotations: + - my_annotation_example1: 10.45 + - my_string_annotation: Hello World + entity_type: build +steps: + zz_firstStep: + stage: test + image: alpine + commands: + - echo Hello World First Step + aa_secondStep: + stage: test + image: alpine + commands: + - echo Hello World Second Step` + + expectedSpecAttributes := &cfClient.Spec{ + Steps: &cfClient.Steps{ + Steps: `{"zz_firstStep":{"commands":["echo Hello World First Step"],"image":"alpine","stage":"test"},"aa_secondStep":{"commands":["echo Hello World Second Step"],"image":"alpine","stage":"test"}}`, + }, + Stages: &cfClient.Stages{ + Stages: `["test"]`, + }, + Hooks: &cfClient.Hooks{ + Hooks: `{"on_finish":{"steps":{"secondmycleanup":{"commands":["echo echo cleanup step"],"image":"alpine:3.9"},"firstmynotification":{"commands":["echo Notify slack"],"image":"cloudposse/slack-notifier"}}},"on_elected":{"exec":{"commands":["echo 'Creating an adhoc test environment'"],"image":"alpine:3.9"},"annotations":{"set":[{"annotations":[{"my_annotation_example1":10.45},{"my_string_annotation":"Hello World"}],"entity_type":"build"}]}}}`, + }, + } resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -178,6 +228,7 @@ func TestAccCodefreshPipeline_OriginalYamlString(t *testing.T) { testAccCheckCodefreshPipelineExists(resourceName), resource.TestCheckResourceAttr(resourceName, "original_yaml_string", originalYamlString), + testAccCheckCodefreshPipelineOriginalYamlStringAttributePropagation(resourceName, expectedSpecAttributes), ), }, { @@ -426,6 +477,38 @@ func testAccCheckCodefreshPipelineDestroy(s *terraform.State) error { return nil } +func testAccCheckCodefreshPipelineOriginalYamlStringAttributePropagation(resource string, spec *cfClient.Spec) resource.TestCheckFunc { + return func(state *terraform.State) error { + + rs, ok := state.RootModule().Resources[resource] + if !ok { + return fmt.Errorf("Not found: %s", resource) + } + if rs.Primary.ID == "" { + return fmt.Errorf("No Record ID is set") + } + + pipelineID := rs.Primary.ID + + apiClient := testAccProvider.Meta().(*cfClient.Client) + pipeline, err := apiClient.GetPipeline(pipelineID) + + if !reflect.DeepEqual(pipeline.Spec.Steps, spec.Steps) { + return fmt.Errorf("Expected Step %v. Got %v", spec.Steps, pipeline.Spec.Steps) + } + if !reflect.DeepEqual(pipeline.Spec.Stages, spec.Stages) { + return fmt.Errorf("Expected Stages %v. Got %v", spec.Stages, pipeline.Spec.Stages) + } + if !reflect.DeepEqual(pipeline.Spec.Hooks, spec.Hooks) { + return fmt.Errorf("Expected Hooks %v. Got %v", spec.Hooks, pipeline.Spec.Hooks) + } + if err != nil { + return fmt.Errorf("error fetching pipeline with resource %s. %s", resource, err) + } + return nil + } +} + // CONFIGS func testAccCodefreshPipelineBasicConfig(rName, repo, path, revision, context string) string { return fmt.Sprintf(` From 03cd76e22427e356ba532d42029eb179036c2b94 Mon Sep 17 00:00:00 2001 From: Sandro Gattuso Date: Fri, 16 Apr 2021 14:52:35 +1000 Subject: [PATCH 3/3] Introduce retry logic for project delete to address eventual consistency --- codefresh/resource_project.go | 17 +++++++++++++++-- go.mod | 2 ++ go.sum | 5 +++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/codefresh/resource_project.go b/codefresh/resource_project.go index 2212094b..2ad4d42b 100644 --- a/codefresh/resource_project.go +++ b/codefresh/resource_project.go @@ -1,6 +1,10 @@ package codefresh import ( + "log" + "time" + + "github.com/cenkalti/backoff" cfClient "github.com/codefresh-io/terraform-provider-codefresh/client" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -90,8 +94,17 @@ func resourceProjectUpdate(d *schema.ResourceData, meta interface{}) error { func resourceProjectDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*cfClient.Client) - - err := client.DeleteProject(d.Id()) + // Adding a Retry backoff to address eventual consistency for the API + expBackoff := backoff.NewExponentialBackOff() + expBackoff.MaxElapsedTime = 2 * time.Second + err := backoff.Retry( + func() error { + err := client.DeleteProject(d.Id()) + if err != nil { + log.Printf("Unable to destroy Project due to error %v", err) + } + return err + }, expBackoff) if err != nil { return err } diff --git a/go.mod b/go.mod index d243ede5..06392ee5 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,8 @@ require ( github.com/aws/aws-sdk-go v1.30.12 // indirect github.com/bflad/tfproviderdocs v0.6.0 github.com/bflad/tfproviderlint v0.14.0 + github.com/cenkalti/backoff v2.2.1+incompatible + github.com/cenkalti/backoff/v4 v4.1.0 github.com/client9/misspell v0.3.4 github.com/dlclark/regexp2 v1.4.0 github.com/ghodss/yaml v1.0.0 diff --git a/go.sum b/go.sum index 1f23cf99..5e18af4d 100644 --- a/go.sum +++ b/go.sum @@ -56,6 +56,11 @@ github.com/bmatcuk/doublestar v1.2.1 h1:eetYiv8DDYOZcBADY+pRvRytf3Dlz1FhnpvL2FsC github.com/bmatcuk/doublestar v1.2.1/go.mod h1:wiQtGV+rzVYxB7WIlirSN++5HPtPlXEo9MEoZQC/PmE= github.com/bombsimon/wsl/v3 v3.0.0 h1:w9f49xQatuaeTJFaNP4SpiWSR5vfT6IstPtM62JjcqA= github.com/bombsimon/wsl/v3 v3.0.0/go.mod h1:st10JtZYLE4D5sC7b8xV4zTKZwAQjCH/Hy2Pm1FNZIc= +github.com/cenkalti/backoff v1.1.0 h1:QnvVp8ikKCDWOsFheytRCoYWYPO/ObCTBGxT19Hc+yE= +github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= +github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= +github.com/cenkalti/backoff/v4 v4.1.0 h1:c8LkOFQTzuO0WBM/ae5HdGQuZPfPxp7lqBRwQRm4fSc= +github.com/cenkalti/backoff/v4 v4.1.0/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cheggaaa/pb v1.0.27/go.mod h1:pQciLPpbU0oxA0h+VJYYLxO+XeDQb5pZijXscXHm81s=