From 9cc7b197b826ad0bdea51dd89420897b234331bc Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Sun, 19 Dec 2021 18:53:21 +0200 Subject: [PATCH 1/9] Fix: Add glob_pattern validation when cd/PrPlan enabled --- env0/resource_environment.go | 26 +++++++++---- env0/resource_environment_test.go | 61 ++++++++++++++++++++++++++++--- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 10afcd64..8b332afa 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -75,14 +75,14 @@ func resourceEnvironment() *schema.Resource { }, "auto_deploy_on_path_changes_only": { Type: schema.TypeBool, - Description: "redeploy only on path changes only", + Description: "redeploy only on path changes only, if it is set to false, and no glob, a deployment will be triggered by any change", + Default: true, Optional: true, }, "auto_deploy_by_custom_glob": { - Type: schema.TypeString, - Description: "redeploy on file filter pattern", - RequiredWith: []string{"auto_deploy_on_path_changes_only"}, - Optional: true, + Type: schema.TypeString, + Description: "redeploy on file filter pattern", + Optional: true, }, "deployment_id": { Type: schema.TypeString, @@ -217,7 +217,10 @@ func setEnvironmentConfigurationSchema(d *schema.ResourceData, configurationVari func resourceEnvironmentCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { apiClient := meta.(client.ApiClientInterface) - payload := getCreatePayload(d, apiClient) + payload, createPayloadErr := getCreatePayload(d, apiClient) + if createPayloadErr != nil { + return diag.Errorf("%v", createPayloadErr) + } environment, err := apiClient.EnvironmentCreate(payload) if err != nil { @@ -334,7 +337,7 @@ func resourceEnvironmentDelete(ctx context.Context, d *schema.ResourceData, meta return nil } -func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterface) client.EnvironmentCreate { +func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterface) (client.EnvironmentCreate, diag.Diagnostics) { payload := client.EnvironmentCreate{} if name, ok := d.GetOk("name"); ok { @@ -366,6 +369,13 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac if d.HasChange("auto_deploy_by_custom_glob") { payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) + if (payload.ContinuousDeployment == nil || *payload.ContinuousDeployment == false) && + (payload.PullRequestPlanDeployments == nil || *payload.PullRequestPlanDeployments == false) { + return client.EnvironmentCreate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") + } + if *payload.AutoDeployOnPathChangesOnly == true && payload.AutoDeployByCustomGlob != "" { + return client.EnvironmentCreate{}, diag.Errorf("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only") + } } if configuration, ok := d.GetOk("configuration"); ok { @@ -381,7 +391,7 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.DeployRequest = &deployPayload - return payload + return payload, nil } func getUpdatePayload(d *schema.ResourceData) client.EnvironmentUpdate { diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 6078d8ad..09f8ddb6 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -17,6 +17,7 @@ func TestUnitEnvironmentResource(t *testing.T) { accessor := resourceAccessor(resourceType, resourceName) templateId := "template-id" + truthy := true environment := client.Environment{ Id: "id0", Name: "my-environment", @@ -24,6 +25,7 @@ func TestUnitEnvironmentResource(t *testing.T) { LatestDeploymentLog: client.DeploymentLog{ BlueprintId: templateId, }, + AutoDeployOnPathChangesOnly: &truthy, } updatedEnvironment := client.Environment{ @@ -75,6 +77,7 @@ func TestUnitEnvironmentResource(t *testing.T) { DeployRequest: &client.DeployRequest{ BlueprintId: templateId, }, + AutoDeployOnPathChangesOnly: environment.AutoDeployOnPathChangesOnly, }).Times(1).Return(environment, nil) mock.EXPECT().EnvironmentUpdate(updatedEnvironment.Id, client.EnvironmentUpdate{ Name: updatedEnvironment.Name, @@ -217,6 +220,7 @@ func TestUnitEnvironmentResource(t *testing.T) { BlueprintRevision: environment.LatestDeploymentLog.BlueprintRevision, ConfigurationChanges: &client.ConfigurationChanges{configurationVariables}, }, ConfigurationChanges: &client.ConfigurationChanges{configurationVariables}, + AutoDeployOnPathChangesOnly: &truthy, }).Times(1).Return(environment, nil) configurationVariables.Id = "generated-id-from-server" @@ -342,6 +346,7 @@ func TestUnitEnvironmentResource(t *testing.T) { BlueprintId: environment.LatestDeploymentLog.BlueprintId, BlueprintRevision: environment.LatestDeploymentLog.BlueprintRevision, }, + AutoDeployOnPathChangesOnly: &truthy, }).Times(1).Return(environment, nil) mock.EXPECT().EnvironmentDeploy(environment.Id, gomock.Any()).Times(1).Return(client.EnvironmentDeployResponse{ @@ -366,6 +371,7 @@ func TestUnitEnvironmentResource(t *testing.T) { LatestDeploymentLog: client.DeploymentLog{ BlueprintId: "template-id", }, + AutoDeployOnPathChangesOnly: &truthy, } updatedEnvironment := client.Environment{ Id: updatedEnvironment.Id, @@ -375,6 +381,7 @@ func TestUnitEnvironmentResource(t *testing.T) { LatestDeploymentLog: client.DeploymentLog{ BlueprintId: environment.LatestDeploymentLog.BlueprintId, }, + AutoDeployOnPathChangesOnly: &truthy, } testCase := resource.TestCase{ @@ -439,6 +446,7 @@ func TestUnitEnvironmentResource(t *testing.T) { LatestDeploymentLog: client.DeploymentLog{ BlueprintId: "template-id", }, + AutoDeployOnPathChangesOnly: &truthy, } updatedEnvironment := client.Environment{ Id: updatedEnvironment.Id, @@ -447,6 +455,7 @@ func TestUnitEnvironmentResource(t *testing.T) { LatestDeploymentLog: client.DeploymentLog{ BlueprintId: environment.LatestDeploymentLog.BlueprintId, }, + AutoDeployOnPathChangesOnly: &truthy, } testCase := resource.TestCase{ @@ -512,7 +521,7 @@ func TestUnitEnvironmentResource(t *testing.T) { }, ContinuousDeployment: &truthyFruity, - AutoDeployOnPathChangesOnly: &truthyFruity, + AutoDeployOnPathChangesOnly: &falsey, RequiresApproval: &falsey, PullRequestPlanDeployments: &truthyFruity, AutoDeployByCustomGlob: ".*", @@ -526,7 +535,7 @@ func TestUnitEnvironmentResource(t *testing.T) { }, ContinuousDeployment: &falsey, - AutoDeployOnPathChangesOnly: &falsey, + AutoDeployOnPathChangesOnly: &truthy, RequiresApproval: &truthyFruity, PullRequestPlanDeployments: &falsey, AutoDeployByCustomGlob: "", @@ -552,7 +561,7 @@ func TestUnitEnvironmentResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "template_id", environment.LatestDeploymentLog.BlueprintId), resource.TestCheckResourceAttr(accessor, "approve_plan_automatically", "true"), resource.TestCheckResourceAttr(accessor, "run_plan_on_pull_requests", "true"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "true"), + resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "false"), resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", ".*"), ), }, @@ -570,7 +579,7 @@ func TestUnitEnvironmentResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "template_id", environment.LatestDeploymentLog.BlueprintId), resource.TestCheckResourceAttr(accessor, "approve_plan_automatically", "false"), resource.TestCheckResourceAttr(accessor, "run_plan_on_pull_requests", "false"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "false"), + resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "true"), resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", ""), ), }, @@ -583,7 +592,7 @@ func TestUnitEnvironmentResource(t *testing.T) { Name: environment.Name, ContinuousDeployment: &falsey, - AutoDeployOnPathChangesOnly: &falsey, + AutoDeployOnPathChangesOnly: &truthy, RequiresApproval: &truthyFruity, PullRequestPlanDeployments: &falsey, AutoDeployByCustomGlob: "", @@ -653,7 +662,44 @@ func TestUnitEnvironmentResource(t *testing.T) { }) }) - t.Run("Failure in create", func(t *testing.T) { + t.Run("Failure in validation while both glob and pathChanges are enabled (pathChanges is enabled by default)", func(t *testing.T) { + autoDeployWithCustomGlobEnabled := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "force_destroy": true, + "run_plan_on_pull_requests": true, + "auto_deploy_by_custom_glob": "/**", + }), + ExpectError: regexp.MustCompile("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only"), + }, + }, + } + runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) {}) + }) + + t.Run("Failure in validation while prPlan and CD are disabled", func(t *testing.T) { + autoDeployWithCustomGlobEnabled := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "force_destroy": true, + "auto_deploy_by_custom_glob": "/**", + }), + ExpectError: regexp.MustCompile("run_plan_on_pull_requests or deploy_on_push must be enabled.*"), + }, + }, + } + runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) {}) + }) + + t.Run("Failure in create API", func(t *testing.T) { testCase := resource.TestCase{ Steps: []resource.TestStep{ { @@ -695,6 +741,7 @@ func TestUnitEnvironmentResource(t *testing.T) { DeployRequest: &client.DeployRequest{ BlueprintId: templateId, }, + AutoDeployOnPathChangesOnly: &truthy, }).Times(1).Return(environment, nil) mock.EXPECT().ConfigurationVariables(client.ScopeEnvironment, environment.Id).Times(2).Return(client.ConfigurationChanges{}, nil) mock.EXPECT().EnvironmentUpdate(updatedEnvironment.Id, client.EnvironmentUpdate{ @@ -715,6 +762,7 @@ func TestUnitEnvironmentResource(t *testing.T) { BlueprintId: environment.LatestDeploymentLog.BlueprintId, BlueprintRevision: "updated template id", }, + AutoDeployOnPathChangesOnly: &truthy, } testCase := resource.TestCase{ @@ -765,6 +813,7 @@ func TestUnitEnvironmentResource(t *testing.T) { DeployRequest: &client.DeployRequest{ BlueprintId: templateId, }, + AutoDeployOnPathChangesOnly: &truthy, }).Times(1).Return(environment, nil) mock.EXPECT().Environment(gomock.Any()).Return(client.Environment{}, errors.New("error")) mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1) From 02cf5a336d33eed07ea424a97cd23437a01e0e39 Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Mon, 20 Dec 2021 16:29:22 +0200 Subject: [PATCH 2/9] fixed tests --- env0/resource_environment.go | 26 +++++++++----- env0/resource_environment_test.go | 56 +++++++++++++++++++++++++------ 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index e813f852..3e415332 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -81,11 +81,10 @@ func resourceEnvironment() *schema.Resource { Default: true, }, "auto_deploy_by_custom_glob": { - Type: schema.TypeString, - Description: "redeploy on file filter pattern", - RequiredWith: []string{"auto_deploy_on_path_changes_only"}, - Optional: true, - Default: "", + Type: schema.TypeString, + Description: "redeploy on file filter pattern", + Optional: true, + Default: "", }, "deployment_id": { Type: schema.TypeString, @@ -220,7 +219,11 @@ func setEnvironmentConfigurationSchema(d *schema.ResourceData, configurationVari func resourceEnvironmentCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { apiClient := meta.(client.ApiClientInterface) - payload := getCreatePayload(d, apiClient) + payload, createEnvPayloadErr := getCreatePayload(d, apiClient) + + if createEnvPayloadErr != nil { + return diag.Errorf("%v", createEnvPayloadErr) + } environment, err := apiClient.EnvironmentCreate(payload) if err != nil { @@ -337,7 +340,7 @@ func resourceEnvironmentDelete(ctx context.Context, d *schema.ResourceData, meta return nil } -func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterface) client.EnvironmentCreate { +func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterface) (client.EnvironmentCreate, diag.Diagnostics) { payload := client.EnvironmentCreate{} if name, ok := d.GetOk("name"); ok { @@ -370,6 +373,13 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.AutoDeployOnPathChangesOnly = &autoDeployOnPathChangesOnly payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) + if payload.AutoDeployByCustomGlob != "" && (payload.ContinuousDeployment == nil || *payload.ContinuousDeployment == false) && + (payload.PullRequestPlanDeployments == nil || *payload.PullRequestPlanDeployments == false) { + return client.EnvironmentCreate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") + } + if *payload.AutoDeployOnPathChangesOnly == true && payload.AutoDeployByCustomGlob != "" { + return client.EnvironmentCreate{}, diag.Errorf("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only") + } if configuration, ok := d.GetOk("configuration"); ok { configurationChanges := getConfigurationVariables(configuration.([]interface{})) @@ -384,7 +394,7 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.DeployRequest = &deployPayload - return payload + return payload, nil } func getUpdatePayload(d *schema.ResourceData) client.EnvironmentUpdate { diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 68d9bf44..5dde4175 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -532,8 +532,8 @@ func TestUnitEnvironmentResource(t *testing.T) { BlueprintId: "template-id", }, + AutoDeployOnPathChangesOnly: &falsey, ContinuousDeployment: &truthyFruity, - AutoDeployOnPathChangesOnly: &truthyFruity, RequiresApproval: &falsey, PullRequestPlanDeployments: &truthyFruity, AutoDeployByCustomGlob: ".*", @@ -546,11 +546,10 @@ func TestUnitEnvironmentResource(t *testing.T) { BlueprintId: environment.LatestDeploymentLog.BlueprintId, }, - ContinuousDeployment: &falsey, - AutoDeployOnPathChangesOnly: &autoDeployOnPathChangesOnlyDefault, - RequiresApproval: &truthyFruity, - PullRequestPlanDeployments: &falsey, - AutoDeployByCustomGlob: autoDeployByCustomGlobDefault, + ContinuousDeployment: &falsey, + RequiresApproval: &truthyFruity, + PullRequestPlanDeployments: &falsey, + //AutoDeployByCustomGlob: autoDeployByCustomGlobDefault, } testCase := resource.TestCase{ @@ -573,8 +572,8 @@ func TestUnitEnvironmentResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "template_id", environment.LatestDeploymentLog.BlueprintId), resource.TestCheckResourceAttr(accessor, "approve_plan_automatically", "true"), resource.TestCheckResourceAttr(accessor, "run_plan_on_pull_requests", "true"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "true"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", ".*"), + resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "false"), + resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", environment.AutoDeployByCustomGlob), ), }, { @@ -592,7 +591,7 @@ func TestUnitEnvironmentResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "approve_plan_automatically", "false"), resource.TestCheckResourceAttr(accessor, "run_plan_on_pull_requests", "false"), resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "true"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", ""), + resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", autoDeployByCustomGlobDefault), ), }, }, @@ -604,7 +603,7 @@ func TestUnitEnvironmentResource(t *testing.T) { Name: environment.Name, ContinuousDeployment: &falsey, - AutoDeployOnPathChangesOnly: &autoDeployOnPathChangesOnlyDefault, + AutoDeployOnPathChangesOnly: &truthyFruity, RequiresApproval: &truthyFruity, PullRequestPlanDeployments: &falsey, AutoDeployByCustomGlob: autoDeployByCustomGlobDefault, @@ -674,6 +673,43 @@ func TestUnitEnvironmentResource(t *testing.T) { }) }) + t.Run("Failure in validation while both glob and pathChanges are enabled (pathChanges is enabled by default)", func(t *testing.T) { + autoDeployWithCustomGlobEnabled := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "force_destroy": true, + "run_plan_on_pull_requests": true, + "auto_deploy_by_custom_glob": "/**", + }), + ExpectError: regexp.MustCompile("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only"), + }, + }, + } + runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) {}) + }) + + t.Run("Failure in validation while prPlan and CD are disabled", func(t *testing.T) { + autoDeployWithCustomGlobEnabled := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "force_destroy": true, + "auto_deploy_by_custom_glob": "/**", + }), + ExpectError: regexp.MustCompile("run_plan_on_pull_requests or deploy_on_push must be enabled.*"), + }, + }, + } + runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) {}) + }) + t.Run("Failure in create", func(t *testing.T) { testCase := resource.TestCase{ Steps: []resource.TestStep{ From 754c9b4265b20d3f6ca102ff881b0a8490a598c7 Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Mon, 20 Dec 2021 17:45:42 +0200 Subject: [PATCH 3/9] updateResource validation + IT fix --- env0/resource_environment.go | 26 ++++++++++++++++++----- tests/integration/012_environment/main.tf | 1 + 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 3e415332..4ee112c6 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -306,7 +306,12 @@ func deploy(d *schema.ResourceData, apiClient client.ApiClientInterface) diag.Di } func update(d *schema.ResourceData, apiClient client.ApiClientInterface) diag.Diagnostics { - payload := getUpdatePayload(d) + payload, updateEnvPayloadErr := getUpdatePayload(d) + + if updateEnvPayloadErr != nil { + return diag.Errorf("%v", updateEnvPayloadErr) + } + _, err := apiClient.EnvironmentUpdate(d.Id(), payload) if err != nil { return diag.Errorf("could not update environment: %v", err) @@ -397,7 +402,7 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac return payload, nil } -func getUpdatePayload(d *schema.ResourceData) client.EnvironmentUpdate { +func getUpdatePayload(d *schema.ResourceData) (client.EnvironmentUpdate, diag.Diagnostics) { payload := client.EnvironmentUpdate{} if name, ok := d.GetOk("name"); ok { @@ -407,19 +412,30 @@ func getUpdatePayload(d *schema.ResourceData) client.EnvironmentUpdate { requiresApproval := !d.Get("approve_plan_automatically").(bool) payload.RequiresApproval = &requiresApproval } + + continuousDeployment := d.Get("deploy_on_push").(bool) if d.HasChange("deploy_on_push") { - continuousDeployment := d.Get("deploy_on_push").(bool) payload.ContinuousDeployment = &continuousDeployment } + pullRequestPlanDeployments := d.Get("run_plan_on_pull_requests").(bool) if d.HasChange("run_plan_on_pull_requests") { - pullRequestPlanDeployments := d.Get("run_plan_on_pull_requests").(bool) payload.PullRequestPlanDeployments = &pullRequestPlanDeployments } autoDeployOnPathChangesOnly := d.Get("auto_deploy_on_path_changes_only").(bool) payload.AutoDeployOnPathChangesOnly = &autoDeployOnPathChangesOnly payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) - return payload + if payload.AutoDeployByCustomGlob != "" { + if continuousDeployment == false && + pullRequestPlanDeployments == false { + return client.EnvironmentUpdate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") + } + if *payload.AutoDeployOnPathChangesOnly == true { + return client.EnvironmentUpdate{}, diag.Errorf("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only") + } + } + + return payload, nil } func getDeployPayload(d *schema.ResourceData, apiClient client.ApiClientInterface, isRedeploy bool) client.DeployRequest { diff --git a/tests/integration/012_environment/main.tf b/tests/integration/012_environment/main.tf index aa187143..c0061841 100644 --- a/tests/integration/012_environment/main.tf +++ b/tests/integration/012_environment/main.tf @@ -19,6 +19,7 @@ resource "env0_environment" "example" { name = "environment configuration variable" value = "value" } + approve_plan_automatically = true } data "env0_environment" "test" { From 6d598541144705ace04bf2526fa0ef7f33ccac60 Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Mon, 20 Dec 2021 20:07:43 +0200 Subject: [PATCH 4/9] changed auto_deploy_path_changes valiation to check 'false' case because 'true' is needed for the API to enable 'custom glob' --- env0/resource_environment.go | 20 +++++----- env0/resource_environment_test.go | 62 ++++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 4ee112c6..68f338f6 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -288,7 +288,7 @@ func shouldDeploy(d *schema.ResourceData) bool { } func shouldUpdate(d *schema.ResourceData) bool { - return d.HasChanges("name", "approve_plan_automatically", "deploy_on_push", "run_plan_on_pull_requests", "auto_deploy_by_custom_glob") + return d.HasChanges("name", "approve_plan_automatically", "deploy_on_push", "run_plan_on_pull_requests", "auto_deploy_by_custom_glob", "auto_deploy_on_path_changes_only") } func shouldUpdateTTL(d *schema.ResourceData) bool { @@ -378,12 +378,14 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.AutoDeployOnPathChangesOnly = &autoDeployOnPathChangesOnly payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) - if payload.AutoDeployByCustomGlob != "" && (payload.ContinuousDeployment == nil || *payload.ContinuousDeployment == false) && - (payload.PullRequestPlanDeployments == nil || *payload.PullRequestPlanDeployments == false) { - return client.EnvironmentCreate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") - } - if *payload.AutoDeployOnPathChangesOnly == true && payload.AutoDeployByCustomGlob != "" { - return client.EnvironmentCreate{}, diag.Errorf("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only") + if payload.AutoDeployByCustomGlob != "" { + if (payload.ContinuousDeployment == nil || *payload.ContinuousDeployment == false) && + (payload.PullRequestPlanDeployments == nil || *payload.PullRequestPlanDeployments == false) { + return client.EnvironmentCreate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") + } + if *payload.AutoDeployOnPathChangesOnly == false { + return client.EnvironmentCreate{}, diag.Errorf("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled") + } } if configuration, ok := d.GetOk("configuration"); ok { @@ -430,8 +432,8 @@ func getUpdatePayload(d *schema.ResourceData) (client.EnvironmentUpdate, diag.Di pullRequestPlanDeployments == false { return client.EnvironmentUpdate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") } - if *payload.AutoDeployOnPathChangesOnly == true { - return client.EnvironmentUpdate{}, diag.Errorf("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only") + if *payload.AutoDeployOnPathChangesOnly == false { + return client.EnvironmentUpdate{}, diag.Errorf("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled") } } diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 5dde4175..5160fc33 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -532,7 +532,7 @@ func TestUnitEnvironmentResource(t *testing.T) { BlueprintId: "template-id", }, - AutoDeployOnPathChangesOnly: &falsey, + AutoDeployOnPathChangesOnly: &truthyFruity, ContinuousDeployment: &truthyFruity, RequiresApproval: &falsey, PullRequestPlanDeployments: &truthyFruity, @@ -572,16 +572,17 @@ func TestUnitEnvironmentResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "template_id", environment.LatestDeploymentLog.BlueprintId), resource.TestCheckResourceAttr(accessor, "approve_plan_automatically", "true"), resource.TestCheckResourceAttr(accessor, "run_plan_on_pull_requests", "true"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "false"), + resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "true"), resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", environment.AutoDeployByCustomGlob), ), }, { Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ - "name": environment.Name, - "project_id": environment.ProjectId, - "template_id": environment.LatestDeploymentLog.BlueprintId, - "force_destroy": true, + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "force_destroy": true, + "auto_deploy_on_path_changes_only": false, }), Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(accessor, "id", environment.Id), @@ -590,7 +591,7 @@ func TestUnitEnvironmentResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "template_id", environment.LatestDeploymentLog.BlueprintId), resource.TestCheckResourceAttr(accessor, "approve_plan_automatically", "false"), resource.TestCheckResourceAttr(accessor, "run_plan_on_pull_requests", "false"), - resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "true"), + resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "false"), resource.TestCheckResourceAttr(accessor, "auto_deploy_by_custom_glob", autoDeployByCustomGlobDefault), ), }, @@ -603,7 +604,7 @@ func TestUnitEnvironmentResource(t *testing.T) { Name: environment.Name, ContinuousDeployment: &falsey, - AutoDeployOnPathChangesOnly: &truthyFruity, + AutoDeployOnPathChangesOnly: &falsey, RequiresApproval: &truthyFruity, PullRequestPlanDeployments: &falsey, AutoDeployByCustomGlob: autoDeployByCustomGlobDefault, @@ -673,23 +674,50 @@ func TestUnitEnvironmentResource(t *testing.T) { }) }) - t.Run("Failure in validation while both glob and pathChanges are enabled (pathChanges is enabled by default)", func(t *testing.T) { + t.Run("Failure in validation while glob is enabled and pathChanges no", func(t *testing.T) { autoDeployWithCustomGlobEnabled := resource.TestCase{ Steps: []resource.TestStep{ { Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ - "name": environment.Name, - "project_id": environment.ProjectId, - "template_id": environment.LatestDeploymentLog.BlueprintId, - "force_destroy": true, - "run_plan_on_pull_requests": true, - "auto_deploy_by_custom_glob": "/**", + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "auto_deploy_on_path_changes_only": false, + "force_destroy": true, + "run_plan_on_pull_requests": true, + "auto_deploy_by_custom_glob": "/**", }), - ExpectError: regexp.MustCompile("cannot set both auto_deploy_by_custom_glob and auto_deploy_on_path_changes_only"), + ExpectError: regexp.MustCompile("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled"), }, + { + Destroy: true, + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "auto_deploy_on_path_changes_only": true, + "force_destroy": true, + "run_plan_on_pull_requests": true, + "auto_deploy_by_custom_glob": "/**", + }), + }, + /*{ + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": environment.Name, + "project_id": environment.ProjectId, + "template_id": environment.LatestDeploymentLog.BlueprintId, + "auto_deploy_on_path_changes_only": false, + "force_destroy": true, + "run_plan_on_pull_requests": true, + "auto_deploy_by_custom_glob": "/**", + }), + ExpectError: regexp.MustCompile("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled"), + },*/ }, } - runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) {}) + runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) { + //mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1) + }) }) t.Run("Failure in validation while prPlan and CD are disabled", func(t *testing.T) { From 1247c08523f8fb622539871e3af98dfa038576aa Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Mon, 20 Dec 2021 21:18:16 +0200 Subject: [PATCH 5/9] added failed test --- env0/resource_environment_test.go | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 5160fc33..fc332b7a 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -690,7 +690,6 @@ func TestUnitEnvironmentResource(t *testing.T) { ExpectError: regexp.MustCompile("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled"), }, { - Destroy: true, Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ "name": environment.Name, "project_id": environment.ProjectId, @@ -701,22 +700,10 @@ func TestUnitEnvironmentResource(t *testing.T) { "auto_deploy_by_custom_glob": "/**", }), }, - /*{ - Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ - "name": environment.Name, - "project_id": environment.ProjectId, - "template_id": environment.LatestDeploymentLog.BlueprintId, - "auto_deploy_on_path_changes_only": false, - "force_destroy": true, - "run_plan_on_pull_requests": true, - "auto_deploy_by_custom_glob": "/**", - }), - ExpectError: regexp.MustCompile("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled"), - },*/ }, } runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) { - //mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1) + mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1) }) }) From 9e3efa6ecea89e9e49d7ceb954ede014e736ce84 Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Tue, 21 Dec 2021 11:08:18 +0200 Subject: [PATCH 6/9] fixed UT! --- env0/resource_environment_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index fc332b7a..c49f8d72 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -695,15 +695,24 @@ func TestUnitEnvironmentResource(t *testing.T) { "project_id": environment.ProjectId, "template_id": environment.LatestDeploymentLog.BlueprintId, "auto_deploy_on_path_changes_only": true, - "force_destroy": true, "run_plan_on_pull_requests": true, "auto_deploy_by_custom_glob": "/**", + "force_destroy": true, }), + ExpectNonEmptyPlan: true, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(accessor, "id", environment.Id), + resource.TestCheckResourceAttr(accessor, "name", environment.Name), + resource.TestCheckResourceAttr(accessor, "project_id", environment.ProjectId), + ), }, }, } runUnitTest(t, autoDeployWithCustomGlobEnabled, func(mock *client.MockApiClientInterface) { - mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1) + mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1).Return(environment, nil) + mock.EXPECT().Environment(gomock.Any()).Times(1).Return(environment, nil) + mock.EXPECT().ConfigurationVariables(gomock.Any(), gomock.Any()).Times(1).Return(client.ConfigurationChanges{}, nil) + mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1) }) }) From 1e5ef1ad940ee88f52e9e4fe2a6437b737e88bca Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Tue, 21 Dec 2021 11:13:59 +0200 Subject: [PATCH 7/9] removed comment --- env0/resource_environment_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index c49f8d72..78403219 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -549,7 +549,6 @@ func TestUnitEnvironmentResource(t *testing.T) { ContinuousDeployment: &falsey, RequiresApproval: &truthyFruity, PullRequestPlanDeployments: &falsey, - //AutoDeployByCustomGlob: autoDeployByCustomGlobDefault, } testCase := resource.TestCase{ From 9f47cc35c26541438cc38fb72d37234100de72a7 Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Tue, 21 Dec 2021 14:21:49 +0200 Subject: [PATCH 8/9] refactoring --- env0/resource_environment.go | 39 +++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 68f338f6..67a6d4b0 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -359,8 +359,8 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.WorkspaceName = workspace.(string) } + continuousDeployment := d.Get("deploy_on_push").(bool) if d.HasChange("deploy_on_push") { - continuousDeployment := d.Get("deploy_on_push").(bool) payload.ContinuousDeployment = &continuousDeployment } @@ -369,8 +369,8 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.RequiresApproval = &requiresApproval } + pullRequestPlanDeployments := d.Get("run_plan_on_pull_requests").(bool) if d.HasChange("run_plan_on_pull_requests") { - pullRequestPlanDeployments := d.Get("run_plan_on_pull_requests").(bool) payload.PullRequestPlanDeployments = &pullRequestPlanDeployments } @@ -378,14 +378,9 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.AutoDeployOnPathChangesOnly = &autoDeployOnPathChangesOnly payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) - if payload.AutoDeployByCustomGlob != "" { - if (payload.ContinuousDeployment == nil || *payload.ContinuousDeployment == false) && - (payload.PullRequestPlanDeployments == nil || *payload.PullRequestPlanDeployments == false) { - return client.EnvironmentCreate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") - } - if *payload.AutoDeployOnPathChangesOnly == false { - return client.EnvironmentCreate{}, diag.Errorf("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled") - } + err := assertDeploymentTriggers(payload.AutoDeployByCustomGlob, continuousDeployment, pullRequestPlanDeployments, autoDeployOnPathChangesOnly) + if err != nil { + return client.EnvironmentCreate{}, err } if configuration, ok := d.GetOk("configuration"); ok { @@ -404,6 +399,19 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac return payload, nil } +func assertDeploymentTriggers(autoDeployByCustomGlob string, continuousDeployment bool, pullRequestPlanDeployments bool, autoDeployOnPathChangesOnly bool) diag.Diagnostics { + if autoDeployByCustomGlob != "" { + if (continuousDeployment == false) && + (pullRequestPlanDeployments == false) { + return diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") + } + if autoDeployOnPathChangesOnly == false { + return diag.Errorf("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled") + } + } + return nil +} + func getUpdatePayload(d *schema.ResourceData) (client.EnvironmentUpdate, diag.Diagnostics) { payload := client.EnvironmentUpdate{} @@ -427,14 +435,9 @@ func getUpdatePayload(d *schema.ResourceData) (client.EnvironmentUpdate, diag.Di payload.AutoDeployOnPathChangesOnly = &autoDeployOnPathChangesOnly payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) - if payload.AutoDeployByCustomGlob != "" { - if continuousDeployment == false && - pullRequestPlanDeployments == false { - return client.EnvironmentUpdate{}, diag.Errorf("run_plan_on_pull_requests or deploy_on_push must be enabled for auto_deploy_by_custom_glob") - } - if *payload.AutoDeployOnPathChangesOnly == false { - return client.EnvironmentUpdate{}, diag.Errorf("cannot set auto_deploy_by_custom_glob when auto_deploy_on_path_changes_only is disabled") - } + err := assertDeploymentTriggers(payload.AutoDeployByCustomGlob, continuousDeployment, pullRequestPlanDeployments, autoDeployOnPathChangesOnly) + if err != nil { + return client.EnvironmentUpdate{}, err } return payload, nil From f99f7e403c2f6b5eaf5241afd0619372033577bc Mon Sep 17 00:00:00 2001 From: Liran Farage Date: Tue, 21 Dec 2021 15:34:50 +0200 Subject: [PATCH 9/9] pr-comments --- env0/resource_environment.go | 1 - 1 file changed, 1 deletion(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 67a6d4b0..13b35c76 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -84,7 +84,6 @@ func resourceEnvironment() *schema.Resource { Type: schema.TypeString, Description: "redeploy on file filter pattern", Optional: true, - Default: "", }, "deployment_id": { Type: schema.TypeString,