Cycle error when removing AWS Lambda functions

[hannes-ucsc]

https:/DataBiosphere/azul/pull/2969/files#r614245068

which is a manifestation of

hashicorp/terraform-provider-aws#11344

and despite my best efforts to follow the best practices

hashicorp/terraform-provider-aws#11344 (comment)

I wasn't able to fix. Here's what I tried:

Index: terraform/api_gateway.tf.json.template.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/terraform/api_gateway.tf.json.template.py b/terraform/api_gateway.tf.json.template.py
--- a/terraform/api_gateway.tf.json.template.py	(revision d744515d69031d45c51bc2698f61b5c1e446a6d5)
+++ b/terraform/api_gateway.tf.json.template.py	(date 1618556411997)
@@ -91,16 +91,17 @@
     # Note that ${} references exist to interpolate a value AND express a dependency.
     "resource": [
         {
-            "aws_api_gateway_deployment": {
+            "aws_api_gateway_stage": {
                 lambda_.name: {
                     "rest_api_id": "${module.chalice_%s.rest_api_id}" % lambda_.name,
+                    "deployment_id": "${module.chalice_%s.rest_api_deployment_id}" % lambda_.name,
                     "stage_name": config.deployment_stage
                 }
             },
             "aws_api_gateway_base_path_mapping": {
                 f"{lambda_.name}_{i}": {
                     "api_id": "${module.chalice_%s.rest_api_id}" % lambda_.name,
-                    "stage_name": "${aws_api_gateway_deployment.%s.stage_name}" % lambda_.name,
+                    "stage_name": "${aws_api_gateway_stage.%s.stage_name}" % lambda_.name,
                     "domain_name": "${aws_api_gateway_domain_name.%s_%i.domain_name}" % (lambda_.name, i)
                 }
                 for i, domain in enumerate(lambda_.domains)
@@ -139,12 +140,12 @@
                 **{
                     f"{lambda_.name}_domain_validation_{i}": {
                         **{
-                            key: "${aws_acm_certificate.%s_%i.domain_validation_options.0.resource_record_%s}"
+                            key: "${tolist(aws_acm_certificate.%s_%i.domain_validation_options)[0].resource_record_%s}"
                                  % (lambda_.name, i, key) for key in ('name', 'type')
                         },
                         "zone_id": "${data.aws_route53_zone.%s.id}" % zones_by_domain[domain].slug,
                         "records": [
-                            "${aws_acm_certificate.%s_%i.domain_validation_options.0.resource_record_value}" % (
+                            "${tolist(aws_acm_certificate.%s_%i.domain_validation_options)[0].resource_record_value}" % (
                                 lambda_.name, i)
                         ],
                         "ttl": 60
Index: scripts/prepare_lambda_deployment.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scripts/prepare_lambda_deployment.py b/scripts/prepare_lambda_deployment.py
--- a/scripts/prepare_lambda_deployment.py	(revision d744515d69031d45c51bc2698f61b5c1e446a6d5)
+++ b/scripts/prepare_lambda_deployment.py	(date 1618588893832)
@@ -35,9 +35,28 @@
         'es_instance_count': {}
     }
 
-    input_json['output']['rest_api_id'] = {
+    output = input_json['output']
+    output['rest_api_id'] = {
         'value': '${aws_api_gateway_rest_api.rest_api.id}'
     }
+    output['rest_api_deployment_id'] = {
+        'value': '${aws_api_gateway_deployment.rest_api.id}'
+    }
+
+    # The Terraform documentation recommends that we don't use the an
+    # implicitly created stage but an explicitly defined one. The explicit
+    # stage is in `api_gateway.tf.json.template.py`. To disable the implicit
+    # stage, we need to remove the `stage_name` and `stage_description`
+    # properties from the Chalice-generted TF config. Chalice hijacks the
+    # `stage_description` to trigger redeployment whenever the REST API
+    # definition changes. The more `triggers` property recently introduced by
+    # the TF AWS provider does the same without any hijacking.
+    #
+    # https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/api_gateway_deployment
+    #
+    deployment = input_json['resource']['aws_api_gateway_deployment']['rest_api']
+    assert deployment.pop('stage_name') == config.deployment_stage
+    deployment['triggers'] = {'redeployment': deployment.pop('stage_description')}
 
     for func in input_json['resource']['aws_lambda_function'].values():
         assert 'layers' not in func
Index: terraform/providers.tf.json.template.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/terraform/providers.tf.json.template.py b/terraform/providers.tf.json.template.py
--- a/terraform/providers.tf.json.template.py	(revision d744515d69031d45c51bc2698f61b5c1e446a6d5)
+++ b/terraform/providers.tf.json.template.py	(date 1618588241560)
@@ -24,7 +24,7 @@
         },
         *({
             "aws": {
-                'version': '2.70.0',
+                'version': '3.36.0',
                 **(
                     {
                         'region': region,
Index: terraform/Makefile
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/terraform/Makefile b/terraform/Makefile
--- a/terraform/Makefile	(revision d744515d69031d45c51bc2698f61b5c1e446a6d5)
+++ b/terraform/Makefile	(date 1618554016988)
@@ -14,9 +14,12 @@
 init: providers config
 	terraform init
 
-.PHONY: rename_resources
-rename_resources: init
+.PHONY: prepare_state
+prepare_state: init
 	python $(project_root)/scripts/rename_resources.py
+	# FIXME: Remove once it has been run on all deployments
+	#        https:/DataBiosphere/azul/issues/2991
+	python $(project_root)/scripts/import_api_gateway_stage.py
 
 .PHONY: validate
 validate: init
@@ -27,14 +30,14 @@
 	terraform plan
 
 .PHONY: apply
-apply: validate rename_resources
+apply: validate prepare_state
 	$(MAKE) unlink_health_checks
 	terraform apply
 	$(MAKE) link_health_checks
 	$(MAKE) grafana
 
 .PHONY: auto_apply
-auto_apply: validate rename_resources
+auto_apply: validate prepare_state
 	$(MAKE) unlink_health_checks
 	terraform apply -auto-approve
 	$(MAKE) link_health_checks
Index: lambdas/service/Makefile
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/lambdas/service/Makefile b/lambdas/service/Makefile
--- a/lambdas/service/Makefile	(revision d744515d69031d45c51bc2698f61b5c1e446a6d5)
+++ b/lambdas/service/Makefile	(date 1618553522983)
@@ -5,7 +5,7 @@
 include ../lambdas.mk
 
 .PHONY: package
-package: check_branch check_python check_aws config compile
+package: check_branch check_python check_aws config compile $(project_root)/scripts/prepare_lambda_deployment.py
 	python -m azul.changelog vendor
 	chalice package --stage $(AZUL_DEPLOYMENT_STAGE) --pkg-format terraform .chalice/terraform
 	python $(project_root)/scripts/prepare_lambda_deployment.py service
Index: scripts/import_api_gateway_stage.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scripts/import_api_gateway_stage.py b/scripts/import_api_gateway_stage.py
new file mode 100644
--- /dev/null	(date 1618553522983)
+++ b/scripts/import_api_gateway_stage.py	(date 1618553522983)
@@ -0,0 +1,46 @@
+import boto3
+from more_itertools import (
+    one,
+)
+
+from azul import (
+    config,
+)
+from azul.logging import (
+    configure_script_logging,
+)
+from azul.terraform import (
+    terraform,
+)
+
+
+def main():
+    def resource(name):
+        return 'aws_api_gateway_stage.' + name
+
+    resources = terraform.run('state', 'list').splitlines()
+    names = {
+        name
+        for name in ('service', 'indexer')
+        if resource(name) not in resources
+    }
+    if names:
+        api_gateway = boto3.client('apigateway')
+        apis = api_gateway.get_rest_apis()
+        for name in names:
+            qname = config.qualified_resource_name(name)
+            matching_apis = (
+                api['id']
+                for api in apis['items']
+                if api['name'] == qname
+            )
+            api_id = one(matching_apis, too_short=None)
+            if api_id is not None:
+                terraform.run('import',
+                              resource(name),
+                              f'{api_id}/{config.deployment_stage}')
+
+
+if __name__ == '__main__':
+    configure_script_logging()
+    main()
Index: lambdas/indexer/Makefile
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/lambdas/indexer/Makefile b/lambdas/indexer/Makefile
--- a/lambdas/indexer/Makefile	(revision d744515d69031d45c51bc2698f61b5c1e446a6d5)
+++ b/lambdas/indexer/Makefile	(date 1618553522983)
@@ -5,7 +5,7 @@
 include ../lambdas.mk
 
 .PHONY: package
-package: check_branch check_python check_aws config compile
+package: check_branch check_python check_aws config compile $(project_root)/scripts/prepare_lambda_deployment.py
 	python -m azul.changelog vendor
 	chalice package --stage $(AZUL_DEPLOYMENT_STAGE) --pkg-format terraform .chalice/terraform
 	python $(project_root)/scripts/prepare_lambda_deployment.py indexer

still getting

Plan: 2 to add, 15 to change, 11 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes


Error: Cycle: module.chalice_service.output.rest_api_deployment_id, module.chalice_service.output.rest_api_id, aws_api_gateway_stage.service, module.chalice_service.aws_api_gateway_deployment.rest_api (destroy deposed 8918b663), module.chalice_service.aws_lambda_function.jwt_auth (destroy), module.chalice_service.aws_api_gateway_rest_api.rest_api, module.chalice_service.aws_api_gateway_deployment.rest_api


make[1]: *** [Makefile:35: apply] Error 1
make[1]: Leaving directory '/home/hannes/workspace/hca/azul/terraform'
make: *** [Makefile:81: deploy] Error 2

during terraform apply against a deployment that's on the base commit of #2969 (064f2c1 on develop) with the PR branch checked out and the above patch applied. The same happens if I apply the patch to the base commit, then deploy (which succeeds), then switch to the PR branch and try to deploy again.

There are some other changes mixed in, most prominently a fix for #1951 which we may fix before this one, so expect conflicts when applying the patch above.

[hannes-ucsc]

#2991 would be a FIXME introduced by the above patch.

[hannes-ucsc]

Examine viability of hashicorp/terraform-provider-aws#11344 (comment) as a workaround.