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

Prefer jsonencode() Over Heredoc Syntax of JSON in Documentation Examples #17714

Open
bflad opened this issue Feb 19, 2021 · 1 comment
Open
Assignees
Labels
documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS.

Comments

@bflad
Copy link
Contributor

bflad commented Feb 19, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue 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 issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Martin's excellent summary in #16792 explains this best:

The existing examples of inline IAM policy documents tend to show them as "heredoc" strings, which was the required style in Terraform v0.11 and earlier.

However, users seem to frequently start from these simple examples and then later need to insert dynamic elements such as ARNs or lists of ARNs from other resources, and because they started with a multi-line string template they then understandably start experimenting with string templating to produce the dynamic JSON, and run into various issues with incorrect quoting and generation of commas.

When helping users in that case (e.g. in the community forum) we typically suggest that they switch to using the jsonencode function for generating that dynamic JSON, because then they can just use normal Terraform language expression features and let Terraform itself worry about making the result valid JSON syntax.

In an attempt to help users discover that solution themselves, rather than fighting with string templates first, I've changed the examples for the resource types I've most commonly seen questions about in order to show generating the JSON string
programmatically using the jsonencode function. Not everyone will necessarily realize right away that they can also use dynamic expressions in there, I hope that this will at least set folks on a better path when they start evaluating possible solutions by making it less likely that string templates will be seen as a viable option.

There are many more heredoc-based IAM policy examples in other resource types -- particularly in various examples that include aws_iam_role helper resources with assume_role_policy arguments -- but I wanted to keep this confined to a frequently-seen subset to start just to avoid this being a huge diff that could be disruptive to merge. Hopefully over time we can update more of these (though there's no particular urgency here), and also maybe write any new examples in this jsonencode style.

The jsonencode style is also typically how Terraform CLI will present the value in the plan diff, in order to potentially produce a structural diff within the JSON data structure, so hopefully this will also help set users up to be less surprised when they encounter that for the first time.

This issue is for tracking and replacing other examples, but does not offer any initial future-proofing enforcement solutions.

Affected Files

Changes to these files should be submitted in batches, to prevent delayed reviews and merge conflicts.

  • website/docs/d/billing_service_account.html.markdown
  • website/docs/d/cloudtrail_service_account.html.markdown
  • website/docs/d/ecs_task_definition.html.markdown
  • website/docs/d/elb_service_account.html.markdown
  • website/docs/d/kms_ciphertext.html.markdown
  • website/docs/d/lambda_invocation.html.markdown
  • website/docs/d/redshift_service_account.html.markdown
  • website/docs/r/api_gateway_account.html.markdown
  • website/docs/r/api_gateway_authorizer.html.markdown
  • website/docs/r/api_gateway_integration.html.markdown
  • website/docs/r/api_gateway_integration_response.html.markdown
  • website/docs/r/api_gateway_model.html.markdown
  • website/docs/r/api_gateway_rest_api_policy.html.markdown
  • website/docs/r/apigatewayv2_model.html.markdown
  • website/docs/r/appsync_datasource.html.markdown
  • website/docs/r/appsync_function.html.markdown
  • website/docs/r/appsync_graphql_api.html.markdown
  • website/docs/r/appsync_resolver.html.markdown
  • website/docs/r/autoscaling_group.html.markdown
  • website/docs/r/autoscaling_lifecycle_hook.html.markdown
  • website/docs/r/backup_selection.html.markdown
  • website/docs/r/backup_vault_policy.html.markdown
  • website/docs/r/batch_compute_environment.html.markdown
  • website/docs/r/batch_job_definition.html.markdown
  • website/docs/r/cloudformation_stack.html.markdown
  • website/docs/r/cloudformation_stack_set.html.markdown
  • website/docs/r/cloudfront_realtime_log_config.html.markdown
  • website/docs/r/cloudtrail.html.markdown
  • website/docs/r/cloudwatch_composite_alarm.html.markdown
  • website/docs/r/cloudwatch_dashboard.html.markdown
  • website/docs/r/cloudwatch_event_archive.html.markdown
  • website/docs/r/cloudwatch_event_rule.html.markdown
  • website/docs/r/cloudwatch_event_target.html.markdown
  • website/docs/r/codeartifact_domain_permissions_policy.html.markdown
  • website/docs/r/codeartifact_repository_permissions_policy.html.markdown
  • website/docs/r/codebuild_project.html.markdown
  • website/docs/r/codebuild_report_group.html.markdown
  • website/docs/r/codedeploy_deployment_group.html.markdown
  • website/docs/r/codepipeline.markdown
  • website/docs/r/cognito_identity_pool_roles_attachment.markdown
  • website/docs/r/cognito_user_group.html.markdown
  • website/docs/r/cognito_user_pool_client.markdown
  • website/docs/r/config_config_rule.html.markdown
  • website/docs/r/config_configuration_aggregator.html.markdown
  • website/docs/r/config_configuration_recorder.html.markdown
  • website/docs/r/config_configuration_recorder_status.html.markdown
  • website/docs/r/config_conformance_pack.html.markdown
  • website/docs/r/config_delivery_channel.html.markdown
  • website/docs/r/dlm_lifecycle_policy.markdown
  • website/docs/r/dynamodb_table_item.html.markdown
  • website/docs/r/ecr_lifecycle_policy.html.markdown
  • website/docs/r/ecr_repository_policy.html.markdown
  • website/docs/r/ecs_task_definition.html.markdown
  • website/docs/r/efs_file_system_policy.html.markdown
  • website/docs/r/eks_cluster.html.markdown
  • website/docs/r/elasticsearch_domain.html.markdown
  • website/docs/r/elasticsearch_domain_policy.html.markdown
  • website/docs/r/emr_cluster.html.markdown
  • website/docs/r/emr_instance_group.html.markdown
  • website/docs/r/emr_security_configuration.html.markdown
  • website/docs/r/flow_log.html.markdown
  • website/docs/r/fms_policy.html.markdown
  • website/docs/r/glacier_vault.html.markdown
  • website/docs/r/glue_crawler.html.markdown
  • website/docs/r/iam_access_key.html.markdown
  • website/docs/r/iam_instance_profile.html.markdown
  • website/docs/r/iam_policy_attachment.html.markdown
  • website/docs/r/iam_role_policy_attachment.markdown
  • website/docs/r/iam_server_certificate.html.markdown
  • website/docs/r/iam_user.html.markdown
  • website/docs/r/iot_policy_attachment.html.markdown
  • website/docs/r/iot_role_alias.html.markdown
  • website/docs/r/iot_topic_rule.html.markdown
  • website/docs/r/kinesis_firehose_delivery_stream.html.markdown
  • website/docs/r/kms_ciphertext.html.markdown
  • website/docs/r/kms_grant.html.markdown
  • website/docs/r/lambda_function.html.markdown
  • website/docs/r/lambda_permission.html.markdown
  • website/docs/r/media_store_container_policy.html.markdown
  • website/docs/r/mq_configuration.html.markdown
  • website/docs/r/msk_cluster.html.markdown
  • website/docs/r/msk_configuration.html.markdown
  • website/docs/r/msk_scram_secret_association.html.markdown
  • website/docs/r/opsworks_stack.html.markdown
  • website/docs/r/organizations_policy.html.markdown
  • website/docs/r/pinpoint_email_channel.markdown
  • website/docs/r/pinpoint_event_stream.markdown
  • website/docs/r/resourcegroups_group.html.markdown
  • website/docs/r/s3_bucket.html.markdown
  • website/docs/r/s3_bucket_notification.html.markdown
  • website/docs/r/secretsmanager_secret_policy.html.markdown
  • website/docs/r/sfn_state_machine.html.markdown
  • website/docs/r/sns_topic.html.markdown
  • website/docs/r/sqs_queue_policy.html.markdown
  • website/docs/r/ssm_activation.html.markdown
  • website/docs/r/ssm_document.html.markdown
  • website/docs/r/ssm_patch_baseline.html.markdown
  • website/docs/r/ssm_resource_data_sync.html.markdown
  • website/docs/r/transfer_server.html.markdown
  • website/docs/r/transfer_ssh_key.html.markdown
  • website/docs/r/transfer_user.html.markdown
  • website/docs/r/vpc_endpoint_connection_notification.html.markdown
  • website/docs/r/xray_encryption_config.html.markdown

References

@mattburgess
Copy link
Collaborator

Given the advice at https://developer.hashicorp.com/terraform/tutorials/aws/aws-iam-policy?in=terraform%2Faws#refactor-your-policy, I'm wondering whether this ticket should skip the middle step of heredoc -> jsonencode() and instead convert all heredocs into aws_iam_policy_documents? I'm happy to tackle this one once we've reached agreement on approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS.
Projects
None yet
Development

No branches or pull requests

2 participants