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

Data Source with Unstable id Attribute and Unconfigured Default Attribute Showing Difference Every Apply #586

Closed
bflad opened this issue Sep 18, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@bflad
Copy link
Contributor

bflad commented Sep 18, 2020

SDK version

Terraform CLI: v0.13.3
Terraform Plugin SDK: v2.0.1

Relevant provider source code

func Provider() *schema.Provider {
	provider := &schema.Provider{
// ...
		DataSourcesMap: map[string]*schema.Resource{
// ...
			"aws_s3_bucket_objects": dataSourceAwsS3BucketObjects(),
// ...
	}
}

const keyRequestPageSize = 1000

func dataSourceAwsS3BucketObjects() *schema.Resource {
	return &schema.Resource{
		Read: dataSourceAwsS3BucketObjectsRead,

		Schema: map[string]*schema.Schema{
			"bucket": {
				Type:     schema.TypeString,
				Required: true,
			},
// ...
			"max_keys": {
				Type:     schema.TypeInt,
				Optional: true,
				Default:  1000,
			},
// ...
			"keys": {
				Type:     schema.TypeList,
				Computed: true,
				Elem:     &schema.Schema{Type: schema.TypeString},
			},
// ...
		},
	}
}

func dataSourceAwsS3BucketObjectsRead(d *schema.ResourceData, meta interface{}) error {
	conn := meta.(*AWSClient).s3conn

	bucket := d.Get("bucket").(string)
	prefix := d.Get("prefix").(string)

	d.SetId(resource.UniqueId())

	listInput := s3.ListObjectsV2Input{
		Bucket: aws.String(bucket),
	}
// ...
	// "listInput.MaxKeys" refers to max keys returned in a single request
	// (i.e., page size), not the total number of keys returned if you page
	// through the results. "maxKeys" does refer to total keys returned.
	maxKeys := int64(d.Get("max_keys").(int))
	if maxKeys <= keyRequestPageSize {
		listInput.MaxKeys = aws.Int64(maxKeys)
	}
// ...
	var keys []string
// ...
	err := conn.ListObjectsV2Pages(&listInput, func(page *s3.ListObjectsV2Output, lastPage bool) bool {
// ...
		for _, object := range page.Contents {
			keys = append(keys, aws.StringValue(object.Key))
// ...
		}

		maxKeys = maxKeys - aws.Int64Value(page.KeyCount)

		if maxKeys <= keyRequestPageSize {
			listInput.MaxKeys = aws.Int64(maxKeys)
		}

		return !lastPage
	})

	if err != nil {
		return fmt.Errorf("error listing S3 Bucket (%s) Objects: %s", bucket, err)
	}
// ...
	if err := d.Set("keys", keys); err != nil {
		return fmt.Errorf("error setting keys: %s", err)
	}
// ...
	return nil
}

Important notes:

  • Unstable d.SetId() value (I have not verified if stabilizing the id attribute makes the overall difference go away.)
  • max_keys attribute Default: 1000 (non-zero-value; although issue does apply to zero-value Default as well)
  • No d.Set("max_keys", /*...*/)

Terraform Configuration Files

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "3.7.0"
    }
    random = {
      source  = "hashicorp/random"
      version = "2.3.0"
    }
  }
  required_version = "0.13.3"
}

provider "aws" {
  region = "us-east-2"
}

resource "random_pet" "test" {}

resource "aws_s3_bucket" "test" {
  bucket = "data-source-default-${random_pet.test.id}"
}

resource "aws_s3_bucket_object" "test" {
  bucket  = aws_s3_bucket.test.bucket
  content = "hello world"
  key     = "test"
}

data "aws_s3_bucket_objects" "test" {
  # https://github.com/hashicorp/terraform/issues/25961
  depends_on = [aws_s3_bucket_object.test]

  bucket = aws_s3_bucket_object.test.bucket
  # Uncomment to remove diff in reproduction
  # max_keys = 1000
}

output "keys" {
  value = data.aws_s3_bucket_objects.test.keys
}

Debug Output

Likely relevant output:

2020/09/17 22:08:55 [WARN] Provider "registry.terraform.io/hashicorp/aws" produced an unexpected new value for data.aws_s3_bucket_objects.test.
      - .max_keys: was null, but now cty.NumberIntVal(1000)
      - .id: was cty.StringVal("terraform-20200918020854967400000001"), but now cty.StringVal("terraform-20200918020855726900000001")

Expected Behavior

No data source difference shown:

$ terraform apply
random_pet.test: Refreshing state... [id=optimal-bunny]
aws_s3_bucket.test: Refreshing state... [id=data-source-default-optimal-bunny]
aws_s3_bucket_object.test: Refreshing state... [id=test]
data.aws_s3_bucket_objects.test: Refreshing state... [id=terraform-20200918020855726900000001]

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

keys = [
  "test",
]

Actual Behavior

$ terraform apply
random_pet.test: Refreshing state... [id=optimal-bunny]
aws_s3_bucket.test: Refreshing state... [id=data-source-default-optimal-bunny]
aws_s3_bucket_object.test: Refreshing state... [id=test]
data.aws_s3_bucket_objects.test: Refreshing state... [id=terraform-20200918020836971600000001]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
 <= read (data resources)

Terraform will perform the following actions:

  # data.aws_s3_bucket_objects.test will be read during apply
  # (config refers to values not yet known)
 <= data "aws_s3_bucket_objects" "test"  {
        bucket          = "data-source-default-optimal-bunny"
        common_prefixes = []
      ~ id              = "terraform-20200918020854967400000001" -> "terraform-20200918020855726900000001"
        keys            = [
            "test",
        ]
        max_keys        = 1000
        owners          = []
    }

Plan: 0 to add, 0 to change, 0 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

data.aws_s3_bucket_objects.test: Reading... [id=terraform-20200918020854967400000001]
data.aws_s3_bucket_objects.test: Read complete after 0s [id=terraform-20200918020855726900000001]

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

keys = [
  "test",
]

The unstable data source id attribute issue, new in Terraform 0.13.0, is tracked here:

Terraform 0.13.1 contained a fix to hide the data source difference if the unstable id attribute was the only difference. However, later versions still show this issue when combined with another attribute that has a Default. Somewhere along the line, core or the SDK is not honoring the Computed-ness of Default.

Steps to Reproduce

This configuration is chosen because the setup is quick/easy to replicate and the data source contains a non-zero-value Default. More typical in the AWS Provider is Type: schema.TypeBool attributes with Default: false that also have this issue.

The configuration above is self-contained for Terraform 0.13 reproduction, which requires the depends_on. If you would like to test against Terraform 0.12, comment out the depends_on in the data source otherwise 0.12 will show a perpetual difference anyways due to how data sources previously interacted with the graph.

You can also bring your own S3 bucket with some objects, if you wish to see this occurring without the resource -> data source references. Just comment out the resources and adjust the data source configuration.

  1. terraform init
  2. terraform apply
  3. terraform apply

References

@jbardin
Copy link
Member

jbardin commented Sep 24, 2020

This may require disallowing defaults in data sources through the legacy SDK. It doesn't necessarily mean that there can't be a future mechanism for providing defaults within the provider itself, but they can't be sent back to Terraform through the returned value unless the attribute is Computed.

The general pain of data sources showing up in the plan is something that should only exist through the 0.13 core release cycle. The 0.14 release will no longer have a separate Refresh phase, and data sources will always be read once with the complete, udpate-to-date configuration. It doesn't mean that this issue should be ignored, since we would prefer to be able to someday enforce correctness of the returned values, rather than only log inconsistencies as warnings.

@bflad
Copy link
Contributor Author

bflad commented Dec 2, 2021

I think a lot of this was resolved with fixing unstable attributes in the AWS provider and Terraform CLI has changed immensely since this issue was opened. If there's still reason to track anything, it might be best to open a new issue with a reproduction case using more recent versions of CLI and the SDK.

@bflad bflad closed this as completed Dec 2, 2021
@github-actions
Copy link

github-actions bot commented Jan 2, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants