-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
provider/aws: Import OpsWorks Custom Layers #9252
Conversation
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.
Hi @DJRH
thanks for the work here - any reason why the test config had to change in the acceptance tests?
P.
@@ -23,7 +23,7 @@ func TestAccAWSOpsworksCustomLayer(t *testing.T) { | |||
CheckDestroy: testAccCheckAwsOpsworksCustomLayerDestroy, | |||
Steps: []resource.TestStep{ | |||
resource.TestStep{ | |||
Config: testAccAwsOpsworksCustomLayerConfigCreate(stackName), | |||
Config: testAccAwsOpsworksCustomLayerConfigNoVpcCreate(stackName), |
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 has the test config changed?
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.
The previous test (testAccAwsOpsworksCustomLayerConfigCreate - currently in master) didn't work for me as it attempts to create a "No VPC" stack, but then tries to create some security groups to assign to the custom layer within this No VPC stack. The creation of these security groups then fails without a default VPC being set.
It therefore made sense to test for the VPC case, and call the stack creation function that also creates a VPC so that we could also test custom security group functionality. That's why I've branched out the testAccAwsOpsworksCustomLayerConfigCreate function into two separate functions - one that creates a VPC stack, and one that creates a non-VPC stack. The non-VPC stack function is exactly the same as before, and this is called as normal for the aws_opsworks_custom_layer_test config. The new VPC stack function is only called from the import test.
I thought that creating these two separate functions was tidier (and is modelled on the VPC/non-VPC cases from the stack tests). If you'd prefer, we could revert to using the non-VPC test and then set a default VPC for adding the security groups (we'd need a VPC then too which is weird for a non-VPC test)? Alternatively, we could turn off custom security groups in the test and use built-in OpsWorks security groups - a default VPC does not need to be set when these are used. Perhaps we could use built-in SGs for the non-VPC case, and custom SGs for the VPC case.
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.
(Sorry for the essay..)
Hi @stack72, Thanks for looking. Yep, I've added a comment in the inline comment. Cheers, |
Hi @DJRH Thanks for the work - your reasons make sense :) I just renamed a test here and all looks good! Manually merging this as i made sure to change that test casing
Thanks for the work here Paul |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Add ability to import AWS OpsWorks Custom Layers.
Acceptance test passes:
Cheers