From 5515923bd80ebae71901e70c7eb5529f31aaa5de Mon Sep 17 00:00:00 2001 From: Liran Farage <83922349+liranfarage89@users.noreply.github.com> Date: Tue, 21 Dec 2021 15:40:54 +0200 Subject: [PATCH] Fix: Add glob_pattern validation when cd/PrPlan enabled (#190) * Fix: Add glob_pattern validation when cd/PrPlan enabled * fixed tests * updateResource validation + IT fix * changed auto_deploy_path_changes valiation to check 'false' case because 'true' is needed for the API to enable 'custom glob' * added failed test * fixed UT! * removed comment * refactoring * pr-comments --- env0/resource_environment.go | 62 +++++++++++----- env0/resource_environment_test.go | 87 +++++++++++++++++++---- tests/integration/012_environment/main.tf | 1 + 3 files changed, 120 insertions(+), 30 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index e813f852..13b35c76 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -81,11 +81,9 @@ 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, }, "deployment_id": { Type: schema.TypeString, @@ -220,7 +218,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 { @@ -285,7 +287,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 { @@ -303,7 +305,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) @@ -337,7 +344,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 { @@ -351,8 +358,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 } @@ -361,8 +368,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 } @@ -370,6 +377,10 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.AutoDeployOnPathChangesOnly = &autoDeployOnPathChangesOnly payload.AutoDeployByCustomGlob = d.Get("auto_deploy_by_custom_glob").(string) + err := assertDeploymentTriggers(payload.AutoDeployByCustomGlob, continuousDeployment, pullRequestPlanDeployments, autoDeployOnPathChangesOnly) + if err != nil { + return client.EnvironmentCreate{}, err + } if configuration, ok := d.GetOk("configuration"); ok { configurationChanges := getConfigurationVariables(configuration.([]interface{})) @@ -384,10 +395,23 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac payload.DeployRequest = &deployPayload - return payload + return payload, nil } -func getUpdatePayload(d *schema.ResourceData) client.EnvironmentUpdate { +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{} if name, ok := d.GetOk("name"); ok { @@ -397,19 +421,25 @@ 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 + err := assertDeploymentTriggers(payload.AutoDeployByCustomGlob, continuousDeployment, pullRequestPlanDeployments, autoDeployOnPathChangesOnly) + if err != nil { + return client.EnvironmentUpdate{}, err + } + + return payload, nil } func getDeployPayload(d *schema.ResourceData, apiClient client.ApiClientInterface, isRedeploy bool) client.DeployRequest { diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 68d9bf44..78403219 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", }, - ContinuousDeployment: &truthyFruity, AutoDeployOnPathChangesOnly: &truthyFruity, + ContinuousDeployment: &truthyFruity, RequiresApproval: &falsey, PullRequestPlanDeployments: &truthyFruity, AutoDeployByCustomGlob: ".*", @@ -546,11 +546,9 @@ 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, } testCase := resource.TestCase{ @@ -574,15 +572,16 @@ func TestUnitEnvironmentResource(t *testing.T) { 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_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), @@ -591,8 +590,8 @@ 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_by_custom_glob", ""), + resource.TestCheckResourceAttr(accessor, "auto_deploy_on_path_changes_only", "false"), + 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: &falsey, RequiresApproval: &truthyFruity, PullRequestPlanDeployments: &falsey, AutoDeployByCustomGlob: autoDeployByCustomGlobDefault, @@ -674,6 +673,66 @@ func TestUnitEnvironmentResource(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, + "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"), + }, + { + 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, + "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).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) + }) + }) + + 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{ 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" {