r/synthetics_canary - new resource

[DrFaust92]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #11145

Release note for CHANGELOG:

resource_aws_synthetics_canary: add new service

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSSyntheticsCanary_'
--- PASS: TestAccAWSSyntheticsCanary_tags (349.62s)
--- PASS: TestAccAWSSyntheticsCanary_disappears (146.87s)
--- PASS: TestAccAWSSyntheticsCanary_runConfig (213.57s)
--- PASS: TestAccAWSSyntheticsCanary_basic (86.61s)
--- PASS: TestAccAWSSyntheticsCanary_runtimeVersion (208.80s)
--- PASS: TestAccAWSSyntheticsCanary_runConfigTracing (208.81s)

[DrFaust92]

basic use case test now passing

need to add the following:

• more tests (VPC and other configs)
• add logic to get code from s3 (only arguments are added)
• wait for creation/update logic
• wait for deletion logic

[DrFaust92]

When a canary is deleted, the Lambda that is created by it is not deleted...
and when that is coupled with a VPC config there is an endless delete loop for VPC resources as the lambdas ENIs are also not deleted

a work around is to delete the function as well but it requires calling the lambda from the canary resource for a resource thats not managed by terraform (albeit it is a byproduct)

@bflad / @ewbankkit , thoughts?

edit: according to the docs, none of the resources created by the canary are destroyed automatically. so there is more junk from other services as well

[bflad]

@DrFaust92 sounds less than ideal. I think first things first we should split this PR into the service client and tagging parts (which we can merge immediately) versus the problematic resource implementation (which I'm not sure when we'll have more time to look into this issue).

[DrFaust92]

Will split these

[DrFaust92]

opened for only service and tag implementation #13186

[tdmalone]

for a resource thats not managed by terraform (albeit it is a byproduct)

@DrFaust92 This sounds similar - although admittedly, more complex - to eg. a CloudWatch log group being created the first time a Lambda is run and not being deleted when the Lambda is deleted. The general workaround for that with Terraform is to also define the log group, and ensure it's created before the function runs for the first time; the function then uses that log group and if you delete everything from Terraform, everything is deleted.

I haven't played around with Synthetics in anger yet so I'm not sure if a similar workflow would work (particularly given you probably wouldn't want to manage the entire Lambda from within Terraform... unless the code can be easily referenced from an S3 object? 🤔) but perhaps that workflow ^^ can offer inspiration towards a solution here.

[immo-huneke-zuhlke]

I've used the temporary provider based on this pull request with good results. I'd be keen to see this merged ASAP because the temporary provider will not pass the security review before production release, being considered SOUP (software of unknown provenance). Any idea when it might happen?

[DrFaust92]

hey @immo-huneke-zuhlke, i wasn't looking at this lately but at the time i was having issues with canary resource deletion as there are resource that are implicitly created by canary and create annoying dependencies. if you are satisfied with a fork of this branch im guessing some changes were made to it.

i'll be happy receive a review of what changes are required to make this work or merge change from the fork.

[immo-huneke-zuhlke]

Hi Ilia, Thanks for getting back to me so quickly. The truth is that I don’t yet know if there are any resources left behind that have to be cleaned up manually – maybe there are some that I have not noticed. I create the Lambda execution role and the associated policy explicitly, so those get deleted when the canary is destroyed. However, the role has permission to create Cloudwatch log groups and log streams, so any that it creates on the fly will not be deleted automatically. I think that’s ok, because logs should be accessible even after their originator has been deleted. Similarly the outputs generated by the synthetic (HAR files, screenshots and stdout/stderr) are placed in the S3 bucket created for the synthetic canary by the Terraform script – but since this bucket is deleted when the canary is destroyed, the outputs disappear with it. I think the documentation should warn users about these behaviours, but they are quite acceptable. Best regards, Immo From: Ilia Lazebnik <notifications@github.com> Reply to: terraform-providers/terraform-provider-aws <reply@reply.github.com> Date: Thursday, 16 July 2020 at 11:33 To: terraform-providers/terraform-provider-aws <terraform-provider-aws@noreply.github.com> Cc: Immo Huneke <immo.huneke@zuhlke.com>, Mention <mention@noreply.github.com> Subject: Re: [terraform-providers/terraform-provider-aws] [WIP]r/synthetics_canary - new resource (#13140) hey @immo-huneke-zuhlke<https://github.com/immo-huneke-zuhlke>, i wasn't looking at this lately but at the time i was having issues with canary resource deletion as there are resource that are implicitly created by canary and create annoying dependencies. if you are satisfied with a fork of this branch im guessing some changes were made to it. i'll be happy receive a review of what changes are required to make this work or merge change from the fork. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#13140 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACHOGYF2U22VFUQMA2MLENLR33JRHANCNFSM4MYIXA5Q>.

[ewbankkit]

@DrFaust92 I have been testing with what we have in this PR 👏 and it seems like once the Synthetics Canary has been created I can manually delete the Lambda that was implicitly created in the console. A terraform destroy of the in-VPC acceptance test configuration then completes cleanly.
The answer may then be to add an acceptance test step that deletes the lambda (there is a standard naming pattern documented).
There is still the problem that in a pure IaC world that lambda can't be deleted via this provider. The user will have to shell out to the AWS CLI or something.
As Amazon do, we could add the list of implicitly created resources to the documentation and even return the Lambda function and layer ARNs as Computed attributes.

[DrFaust92]

Added some docs regarding implicit resources + some added tests (disappears)
added support for setting memory for canary lambda.

still experimenting with lambda deletion but all else seems to be working after a few tweaks.

[MaksymBilenko]

I think that canary start option is missing here.

[DrFaust92]

@MaksymBilenko, not sure i want to mix canary execution and canary start in a single resource. id split it it of to a different data source maybe, like lambda execution.

[brntsllvn]

@DrFaust92 Looking at the PR, this looks mostly done...? I'm happy to beta test or contribute if that moves things forward. Let me know how I can assist. Thanks for your effort here.

[YakDriver]

Since the modern documentation has links on the right side of the page to all the sections, internal links to the subsections aren't needed.

[YakDriver]

Looks great! 🎉

GovCloud:

--- PASS: TestAccAWSSyntheticsCanary_disappears (36.45s)
--- PASS: TestAccAWSSyntheticsCanary_s3 (58.41s)
--- PASS: TestAccAWSSyntheticsCanary_runConfigTracing (86.62s)
--- PASS: TestAccAWSSyntheticsCanary_basic (92.23s)
--- PASS: TestAccAWSSyntheticsCanary_runtimeVersion (92.29s)
--- PASS: TestAccAWSSyntheticsCanary_tags (97.04s)
--- PASS: TestAccAWSSyntheticsCanary_runConfig (98.37s)
--- PASS: TestAccAWSSyntheticsCanary_startCanary (101.86s)
--- PASS: TestAccAWSSyntheticsCanary_vpc (477.29s)

us-west-2:

--- PASS: TestAccAWSSyntheticsCanary_basic (86.26s)
--- PASS: TestAccAWSSyntheticsCanary_disappears (40.82s)
--- PASS: TestAccAWSSyntheticsCanary_runConfig (80.27s)
--- PASS: TestAccAWSSyntheticsCanary_runConfigTracing (78.04s)
--- PASS: TestAccAWSSyntheticsCanary_runConfigTracing (80.96s)
--- PASS: TestAccAWSSyntheticsCanary_runtimeVersion (75.07s)
--- PASS: TestAccAWSSyntheticsCanary_s3 (45.80s)
--- PASS: TestAccAWSSyntheticsCanary_startCanary (92.94s)
--- PASS: TestAccAWSSyntheticsCanary_tags (76.34s)
--- PASS: TestAccAWSSyntheticsCanary_vpc (655.78s)

[ghost]

This has been released in version 3.28.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!