Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add prevent_auto_deploy flag to create without deploy #804

Merged
merged 5 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ type EnvironmentCreate struct {
IsRemoteBackend *bool `json:"isRemoteBackend,omitempty" tfschema:"-"`
Type string `json:"type,omitempty"`
DriftDetectionRequest *DriftDetectionRequest `json:"driftDetectionRequest,omitempty" tfschema:"-"`
PreventAutoDeploy *bool `json:"preventAutoDeploy,omitempty" tfschema:"-"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it a pointer, is it okay to always pass false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it won't be a pointer and the user won't specify it - will it evaluate to false? I think we could always pass it but is it really better?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it simplifies the code significantly.

But it depends on the API. Some of the API's params do have "3" values: "pass nothing", "false", "true".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try passing false as default. let me see

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a big deal in this case...
I approved the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it anyway cause it should be simpler - can you take a quick look?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit test is failing...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea I gave up on the bool change, it failing to much tests now 😅

}

// When converted to JSON needs to be flattened. See custom MarshalJSON below.
Expand Down
10 changes: 10 additions & 0 deletions env0/resource_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,11 @@ func resourceEnvironment() *schema.Resource {
Description: "should use remote backend",
Optional: true,
},
"prevent_auto_deploy": {
Type: schema.TypeBool,
Description: "use this flag to prevent auto deploy on environment creation",
Optional: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's okay to always pass false, you can add a default: false.

},
"terragrunt_working_directory": {
Type: schema.TypeString,
Description: "The working directory path to be used by a Terragrunt template. If left empty '/' is used. Note: modifying this field destroys the current environment and creates a new one",
Expand Down Expand Up @@ -799,6 +804,11 @@ func getCreatePayload(d *schema.ResourceData, apiClient client.ApiClientInterfac
payload.PullRequestPlanDeployments = boolPtr(val.(bool))
}

//lint:ignore SA1019 reason: https://github.com/hashicorp/terraform-plugin-sdk/issues/817
if val, exists := d.GetOkExists("prevent_auto_deploy"); exists {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're missing more use-cases.

Search for the string "auto_deploy_on_path_changes_only"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why tho? I'm interested only in Create flow - update and delete are not interesting to me (even if this property is changed - nothing should be sone because its not really persisted at the backend)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood!

payload.PreventAutoDeploy = boolPtr(val.(bool))
}

//lint:ignore SA1019 reason: https://github.com/hashicorp/terraform-plugin-sdk/issues/817
if val, exists := d.GetOkExists("auto_deploy_on_path_changes_only"); exists {
payload.AutoDeployOnPathChangesOnly = boolPtr(val.(bool))
Expand Down
66 changes: 66 additions & 0 deletions env0/resource_environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,72 @@ func TestUnitEnvironmentResource(t *testing.T) {
})
})

t.Run("prevent auto deploy", func(t *testing.T) {
templateId := "template-id"
newTemplateId := "new-template-id"
truthyFruity := true

environment := client.Environment{
Id: uuid.New().String(),
Name: "name",
ProjectId: "project-id",
LatestDeploymentLog: client.DeploymentLog{
BlueprintId: templateId,
},
}

testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"name": environment.Name,
"project_id": environment.ProjectId,
"template_id": templateId,
"force_destroy": true,
"prevent_auto_deploy": true,
}),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(accessor, "id", environment.Id),
resource.TestCheckResourceAttr(accessor, "name", environment.Name),
resource.TestCheckResourceAttr(accessor, "project_id", environment.ProjectId),
resource.TestCheckResourceAttr(accessor, "template_id", templateId),
resource.TestCheckResourceAttr(accessor, "prevent_auto_deploy", "true"),
),
},
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this step isn't required here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it fails without it - care to explain what it does?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It tests for an error. Weird that it would fail without it.
What's the error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry my bad, had to fix the assertions as well - the API of those tests is confusing AF 😅
removed unnecessary step

"name": environment.Name,
"project_id": environment.ProjectId,
"template_id": newTemplateId,
"force_destroy": true,
"prevent_auto_deploy": true,
}),
ExpectError: regexp.MustCompile("template_id may not be modified, create a new environment instead"),
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {
gomock.InOrder(
mock.EXPECT().Template(environment.LatestDeploymentLog.BlueprintId).Times(1).Return(template, nil),
mock.EXPECT().EnvironmentCreate(client.EnvironmentCreate{
Name: environment.Name,
ProjectId: environment.ProjectId,
DeployRequest: &client.DeployRequest{
BlueprintId: templateId,
},

PreventAutoDeploy: &truthyFruity,
}).Times(1).Return(environment, nil),
mock.EXPECT().Environment(environment.Id).Times(1).Return(environment, nil),
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{}, nil),
mock.EXPECT().Environment(environment.Id).Times(1).Return(environment, nil),
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{}, nil),
mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1),
)
})
})

t.Run("avoid modifying template id", func(t *testing.T) {
templateId := "template-id"
newTemplateId := "new-template-id"
Expand Down
Loading