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

aws_cloudfront_distribution attribute mismatch: custom_header #6527

Closed
pete0emerson opened this issue May 6, 2016 · 17 comments · Fixed by #9171
Closed

aws_cloudfront_distribution attribute mismatch: custom_header #6527

pete0emerson opened this issue May 6, 2016 · 17 comments · Fixed by #9171

Comments

@pete0emerson
Copy link

My attempt at creating an AWS CloudFront distribution fronted by S3 is failing, and while my gut says it's a configuration problem on my part, the terraform output says that it's a bug. So, here's a bug report.

Terraform Version

Terraform v0.6.15

Affected Resource(s)

  • aws_cloudfront_distribution

Terraform Configuration Files

provider "aws" {
  region      = "${var.aws_region}"
  max_retries = 11
}

resource "aws_s3_bucket" "webapp-stage" {
  bucket = "com.mydomain.assets.webapp"
  acl    = "private"

  policy = <<EOF
{
   "Version":"2012-10-17",
   "Id":"PolicyForCloudFrontPrivateContent",
   "Statement":[
     {
       "Sid":" Grant a CloudFront Origin Identity access to support private content",
       "Effect":"Allow",
       "Principal":{"CanonicalUser":"${aws_cloudfront_origin_access_identity.webapp-stage.s3_canonical_user_id}"},
       "Action":"s3:GetObject",
       "Resource":"arn:aws:s3:::com.mydomain.assets.webapp/*"
     }
   ]
}
EOF
}

resource "aws_s3_bucket_object" "index" {
  bucket = "${aws_s3_bucket.webapp-stage.bucket}"
  key    = "index.html"
  source = "index.html"
  etag   = "${md5(file("index.html"))}"
}

resource "aws_cloudfront_origin_access_identity" "webapp-stage" {
  comment = "webapp-stage"
}

resource "aws_cloudfront_distribution" "webapp-stage" {
  origin {
    domain_name = "${aws_s3_bucket.webapp-stage.bucket}.s3.amazonaws.com"
    origin_id   = "S3-${aws_s3_bucket.webapp-stage.bucket}"
    custom_header {
        name = "foo"
        value = "bar"
    }
    s3_origin_config {
      origin_access_identity = "${aws_cloudfront_origin_access_identity.webapp-stage.cloudfront_access_identity_path}"
    }
  }

  enabled             = true
  comment             = "${aws_s3_bucket.webapp-stage.bucket}"
  default_root_object = "index.html"

  aliases = ["webapp-assets.mydomain.com"]

  restrictions {
    geo_restriction {
      restriction_type = "none"
    }
  }

  default_cache_behavior {
    allowed_methods  = ["DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"]
    cached_methods   = ["GET", "HEAD"]
    target_origin_id = "S3-${aws_s3_bucket.webapp-stage.bucket}"

    viewer_protocol_policy = "allow-all"
    min_ttl                = 0
    default_ttl            = 3600
    max_ttl                = 86400

    forwarded_values {
      query_string = false

      cookies {
        forward = "none"
      }
    }
  }

  viewer_certificate {
    iam_certificate_id = "arn:aws:iam::012345678901:server-certificate/cloudfront/mydomain.com/mydomain.com"
  }
}

Debug Output

https://gist.github.com/pete0emerson/d1182c68317adc4d85fdd95379c09728

Panic Output

None

Expected Behavior

AWS Cloudfront Distribution created, connected privately to an S3 bucket

Actual Behavior

Mismatch reason: attribute mismatch: origin.2204279056.custom_header.#

Steps to Reproduce

  1. terraform apply

Important Factoids

The user account used to run terraform has very loose ACLs, I don't think that's in play as a potential problem.
Given that this is the first time using aws_cloudfront_distribution I suspect there's a good chance that PEBKAC is at play, but given that the output thinks it's a terraform bug, I'm submitting this ticket.

References

None

@vancluever
Copy link
Contributor

One thing I'm noticing in the diff is the computed hash for the single origin is different in both diffs, with the only real difference I see being origin_access_identity:

Diff 1 new: ${aws_cloudfront_origin_access_identity.webapp-stage.cloudfront_acc
    ess_identity_path}
Diff 2 new: origin-access-identity/cloudfront/E3D6XQC28JYH8E

Not too sure if this counts for anything - I'm not too sure if since the hash for origin is computed based off the sub-hashes of all complex structures underneath it, maybe the pre-apply diff is breaking because the aws_cloudfront_origin_access_identityhasn't been interpolated yet?

I also notice a difference in the paths on the two attributes:

origin.2204279056.s3_origin_config.~3517799439.origin_access_identity
origin.4136490765.s3_origin_config.140030283.origin_access_identity

Not too sure what the ~ denotes here.

Incidentally, the values for custom_header (quantity, name, value, and hash code) are all identical.

@apparentlymart
Copy link
Contributor

@vancluever, I wasn't able to find the code on a quick search but I believe at some point I found code that suggested that the ~ prefix means "computed", which seems consistent with your theory.

@vancluever
Copy link
Contributor

Thanks @apparentlymart!

So if this is true, then I guess the answer we need is if a child hash is computed, is that communicated to the parent? I'm guessing this gets marked as computed as it's an interpolated value, versus what is in the schema, as none of the values in the s3_origin_config path are marked as computed.

@pete0emerson
Copy link
Author

It looks like you're on the right track; if I hardcode the origin_access_identity inside the aws_cloudfront_distribution.origin stanza it gets rid of the error. I still can't get aws_cloudfront_distribution to succeed with a custom SSL cert, but that's a separate issue that I'll either figure out or file.

@vancluever
Copy link
Contributor

Just a note that I'm unable to re-produce this in a current version of TF (off HEAD). Not too sure if something has changed in core to make this a non-issue now, but just perusing the changelog for cloudfront_distribution_configuration_structure.go I don't see anything that would have changed this specifically. @pete0emerson not too sure if you are around to confirm if this is corrected for you or not?

@vancluever
Copy link
Contributor

I spoke too soon on this one. The viewer_certificate block in this sample config referenced a certificate I obviously don't have, and when the first apply failed it created the origin access identity, basically creating a situation where the data was available to Terraform, ensuring that the second apply succeeded. Diving into this one and #8271 deeper...

@vancluever
Copy link
Contributor

vancluever commented Oct 1, 2016

I'm quite sure now that this is a result of computed state not propagating up the graph. InstanceDiff.Same does have some level of computed hash detection, but it only works for the level that it's inspecting, from what I can see. Basically, it takes the first computed hash element in the resource path, switches that up to a \d+ match, and tries to find an identical attribute off of that regex match. This would work if say origin.2204279056 was marked as computed, but it's not, because the computed value is in a subkey.

So we need to do one of, I think:

  1. Figure out way to calculate interpolated subkeys
  2. Figure out a way to propagate computed hashes to any other parent sets in the config
  3. Broaden InstanceDiff.Same to try and look for child sets that are computed if that's possible.

The latter seems like it'd be the least invasive, I'm thinking it should be safe (I mean if a set in a set is computed then just by nature the hash of that parent should be computed as well by proxy). However, I think the most proper way to do it would be option 2, but I don't know if there's currently a way for the field reader code to propagate that data up the config.

@vancluever
Copy link
Contributor

vancluever commented Oct 2, 2016

Alright, looks like the fix was none of the above here, but kind of in the spirit of option 3.

Code already existed in helper/schema/field_reader_config.go to check subkeys of sets to see if any values were computed. However, this only was applying to the first level of the set, either intentionally or not, as there was some recursion in the the function (hasComputedSubKeys), but the recursion was not taking into account that the subkey could itself be a set or list as well, and not iterating into these keys to get any values to add to the address.

I'm referencing the fix back to this bug. This could possibly fix other issues as well that are kind of related (ie: sub-sets or lists of a set that have computed values where the parent itself has none).

@pete0emerson
Copy link
Author

@vancluever apologies for the delay, I just encountered what I think is this problem again, and I'm on terraform version 0.7.7.

I've created a new gist for you to help out, LMK how I can assist: https://gist.github.com/pete0emerson/a16b3b04adcc3c03646ce81bec170570

@pete0emerson
Copy link
Author

Interesting, I just ran terraform apply a second time (with no modifications to any .tf files) and got different output. I'm trying to grok it now, but am attaching as a new gist here:

https://gist.github.com/pete0emerson/9ba75c42cab074d5a7a81af6072bf8f4

@pete0emerson
Copy link
Author

Okay, I added some more things to my tf files (ssl_support_method = "sni-only" and minimum_protocol_version = "TLSv1") and changed the origin_access_identity to "${aws_cloudfront_origin_access_identity.origin_access_identity.cloudfront_access_identity_path}", and it ran clean.

I'm destroying and recreating now to see if it runs clean from 0 state, will report back in a few.

@pete0emerson
Copy link
Author

Confirmed, I started with a blank slate, the first run created the attribute mismatch as described above, the second run ran clean.

@vancluever
Copy link
Contributor

Thanks @pete0emerson! Can you merge in 0cc7bcb and see if that fixes things on the first shot?

@benlei-neustar
Copy link

I just tried your (@vancluever) fork and it seems to have solved the issue for me.

@vancluever
Copy link
Contributor

Thanks @benlei-neustar! Good to know and should help come review time 👍

@meyerbro
Copy link

meyerbro commented Nov 29, 2016

Not fixed it seems...
Version 0.7.13.

gusmat pushed a commit to gusmat/terraform that referenced this issue Dec 6, 2016
This fixes some edge-ish cases where a set in a config has a set or list
in it that contains computed values, but non-set or list values in the
parent do not.

This can cause "diffs didn't match during apply" errors in a scenario
such as when a set's hash is calculated off of child items (including
any sub-lists or sets, as it should be), and the hash changes between
the plan and apply diffs due to the computed values present in the
sub-list or set items. These will be marked as computed, but due to the
fact that the function was not iterating over the list or set items
properly (ie: not adding the item number to the address, so
set.0.set.foo was being yielded instead of set.0.set.0.foo), these
computed values were not being properly propagated to the parent set to
be marked as computed.

Fixes hashicorp#6527.
Fixes hashicorp#8271.

This possibly fixes other non-CloudFront related issues too.
fatmcgav pushed a commit to fatmcgav/terraform that referenced this issue Feb 27, 2017
This fixes some edge-ish cases where a set in a config has a set or list
in it that contains computed values, but non-set or list values in the
parent do not.

This can cause "diffs didn't match during apply" errors in a scenario
such as when a set's hash is calculated off of child items (including
any sub-lists or sets, as it should be), and the hash changes between
the plan and apply diffs due to the computed values present in the
sub-list or set items. These will be marked as computed, but due to the
fact that the function was not iterating over the list or set items
properly (ie: not adding the item number to the address, so
set.0.set.foo was being yielded instead of set.0.set.0.foo), these
computed values were not being properly propagated to the parent set to
be marked as computed.

Fixes hashicorp#6527.
Fixes hashicorp#8271.

This possibly fixes other non-CloudFront related issues too.
@ghost
Copy link

ghost commented Apr 19, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants