-
Notifications
You must be signed in to change notification settings - Fork 14
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why tho? I'm interested only in There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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{}{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this step isn't required here... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it fails without it - care to explain what it does? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It tests for an error. Weird that it would fail without it. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 😅 |
||
"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" | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit test is failing...
There was a problem hiding this comment.
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 😅