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

provider: Stabilization of singular data source id attributes with clear identifiers #15399

Merged
merged 1 commit into from
Oct 5, 2020

Commits on Sep 30, 2020

  1. provider: Stabilization of singular data source id attributes with cl…

    …ear identifiers
    
    Reference: #14579
    Reference: https://github.com/bflad/tfproviderlint/tree/master/passes/R015
    Reference: https://github.com/bflad/tfproviderlint/tree/master/passes/R016
    Reference: https://github.com/bflad/tfproviderlint/tree/master/passes/R017
    Reference: https://www.terraform.io/docs/extend/best-practices/versioning.html#example-major-number-increments
    Reference: https://registry.terraform.io/providers/hashicorp/random/
    Reference: https://registry.terraform.io/providers/hashicorp/time/
    
    Terraform 0.13 reworked data source reads into the plan graph, which had some unintentional consequences with Terraform Plugin SDK and provider behaviors that were previously ignored when displaying plan differences. While some of these differences  dealing with empty and missing blocks could be addressed with extra graph logic, there remains problematic behaviors that will not be addressable in the near future in core or the SDK.
    
    This change set uses the `tfproviderlint` R015, R016, and R017 checks to find `(*schema.ResourceData).SetId()` usage with unstable values such as the current time and per-execution random identifiers and where the identifier can be stablized based on the data source purpose and surrounding context. For example, singular data sources that represent an analogous managed resource can use the same identifier (e.g. `aws_acm_certificate`, `aws_autoscaling_group`). Other cases where the data source represents information from an AWS Partition or AWS Region are stabilized with those as identifiers (e.g. `aws_regions`).
    
    Importantly, its worth noting that while the unstable `id` attribute is the most visible in the plan difference output, it does not necessarily represent the underlying issue that is causing the output to show. There are two known cases, first with providers unexpectedly writing values to unconfigured and uncomputed attributes and second with Default usage in data source schemas, that are the real triggers of the unexpected difference output. Additional upstream bug reports to properly show difference sigils in the plan difference output are likely necessary, since in many of the real world cases of this particular issue they are missing. Potential future bug reports containing these data sources may help guide those.
    
    Another important note here is that per the Extending Terraform documentation for versioning, that resource identifier changes typically fall under a best practice of a major version increment for Terraform Providers. Given the widespread reports of unexpected behavior as practitioners are upgrading to Terraform 0.13 and since the old identifiers did not represent meaningful information for the lookup context, these changes are not lightly being considered a bug fix. If the usage of a changing time stamp is necessary, the `timestamp()` function and Terraform Time Provider are recommended methodologies for this information. If the usage of a changing random identifier is necessary, the Terraform Random Provider is the recommended methodology.
    
    There still remains other data sources and resources that suffer from known unstable identifiers, such as many plural data sources. Determination of the path forward for these is still undetermined, since ideally Terraform Providers should no longer need to set an `id` attribute after Terraform 0.12, however the Terraform Plugin SDK does not provide functionality to avoid this yet. Future changes may involve a standardized pattern for these within the provider itself.
    
    Changes:
    
    ```
    NOTES:
    
    * data-source/aws_acm_certificate: The `id` attribute has changed to the ARN of the ACM Certificate. The first apply of this updated data source may show this difference.
    * data-source/aws_autoscaling_group: The `id` attribute has changed to the name of the Auto Scaling Group. The first apply of this updated data source may show this difference.
    * data-source/aws_availability_zones: The `id` attribute has changed to the name of the AWS Region. The first apply of this updated data source may show this difference.
    * data-source/aws_db_event_categories: The `id` attribute has changed to the name of the AWS Region. The first apply of this updated data source may show this difference.
    * data-source/aws_ebs_default_kms_key: The `id` attribute has changed to the name of the AWS Region. The first apply of this updated data source may show this difference.
    * data-source/aws_ebs_encryption_by_default: The `id` attribute has changed to the name of the AWS Region. The first apply of this updated data source may show this difference.
    * data-source/aws_ec2_instance_type_offering: The `id` attribute has changed to the EC2 Instance Type. The first apply of this updated data source may show this difference.
    * data-source/aws_ecr_authorization_token: The `id` attribute has changed to the AWS Region. The first apply of this updated data source may show this difference.
    * data-source/aws_ecr_image: The `id` attribute has changed to the SHA256 digest of the ECR Image. The first apply of this updated data source may show this difference.
    * data-source/aws_eks_cluster_auth: The `id` attribute has changed to the name of the EKS Cluster. The first apply of this updated data source may show this difference.
    * data-source/aws_iam_account_alias: The `id` attribute has changed to the AWS Account Alias. The first apply of this updated data source may show this difference.
    * data-source/aws_kms_alias: The `id` attribute has changed to the ARN of the KMS Alias. The first apply of this updated data source may show this difference.
    * data-source/aws_partition: The `id` attribute has changed to the identifier of the AWS Partition. The first apply of this updated data source may show this difference.
    * data-source/aws_regions: The `id` attribute has changed to the identifier of the AWS Partition. The first apply of this updated data source may show this difference.
    * data-source/aws_sns_topic: The `id` attribute has changed to the ARN of the SNS Topic. The first apply of this updated data source may show this difference.
    
    FIXES:
    
    * data-source/aws_acm_certificate: Prevent plan differences with the `id` attribute
    * data-source/aws_autoscaling_group: Prevent plan differences with the `id` attribute
    * data-source/aws_availability_zones: Prevent plan differences with the `id` attribute
    * data-source/aws_db_event_categories: Prevent plan differences with the `id` attribute
    * data-source/aws_ebs_default_kms_key: Prevent plan differences with the `id` attribute
    * data-source/aws_ebs_encryption_by_default: Prevent plan differences with the `id` attribute
    * data-source/aws_ec2_instance_type_offering: Prevent plan differences with the `id` attribute
    * data-source/aws_ecr_authorization_token: Prevent plan differences with the `id` attribute
    * data-source/aws_ecr_image: Prevent plan differences with the `id` attribute
    * data-source/aws_eks_cluster_auth: Prevent plan differences with the `id` attribute
    * data-source/aws_iam_account_alias: Prevent plan differences with the `id` attribute
    * data-source/aws_kms_alias: Prevent plan differences with the `id` attribute
    * data-source/aws_partition: Prevent plan differences with the `id` attribute
    * data-source/aws_regions: Prevent plan differences with the `id` attribute
    * data-source/aws_sns_topic: Prevent plan differences with the `id` attribute
    ```
    
    Output from acceptance testing:
    
    ```
    --- PASS: TestAccAwsAutoScalingGroupDataSource_basic (38.65s)
    
    --- PASS: TestAccAWSAvailabilityZones_AllAvailabilityZones (23.55s)
    --- PASS: TestAccAWSAvailabilityZones_basic (23.40s)
    --- PASS: TestAccAWSAvailabilityZones_ExcludeNames (14.32s)
    --- PASS: TestAccAWSAvailabilityZones_ExcludeZoneIds (24.34s)
    --- PASS: TestAccAWSAvailabilityZones_Filter (23.32s)
    --- PASS: TestAccAWSAvailabilityZones_stateFilter (23.23s)
    
    --- PASS: TestAccAWSDbEventCategories_basic (15.76s)
    --- PASS: TestAccAWSDbEventCategories_sourceType (14.71s)
    
    --- PASS: TestAccAWSEc2InstanceTypeOfferingDataSource_Filter (23.91s)
    --- PASS: TestAccAWSEc2InstanceTypeOfferingDataSource_LocationType (24.65s)
    --- PASS: TestAccAWSEc2InstanceTypeOfferingDataSource_PreferredInstanceTypes (23.58s)
    
    --- PASS: TestAccAWSEcrAuthorizationTokenDataSource_basic (23.96s)
    
    --- PASS: TestAccAWSEcrDataSource_ecrImage (24.22s)
    
    --- PASS: TestAccAWSEksClusterAuthDataSource_basic (19.59s)
    
    --- PASS: TestAccDataSourceAwsEBSDefaultKmsKey_basic (20.11s)
    
    --- PASS: TestAccDataSourceAwsEBSEncryptionByDefault_basic (23.26s)
    
    --- PASS: TestAccDataSourceAwsKmsAlias_AwsService (24.27s)
    --- PASS: TestAccDataSourceAwsKmsAlias_CMK (28.03s)
    
    --- PASS: TestAccDataSourceAwsRegions_AllRegions (23.30s)
    --- PASS: TestAccDataSourceAwsRegions_basic (23.44s)
    --- PASS: TestAccDataSourceAwsRegions_Filter (22.16s)
    
    --- PASS: TestAccDataSourceAwsSnsTopic_basic (26.41s)
    ```
    bflad committed Sep 30, 2020
    Configuration menu
    Copy the full SHA
    c8f82a0 View commit details
    Browse the repository at this point in the history