jmgreg31 / terraform-aws-cloudfront

AWS Cloudfront Distribution Module
MIT License
27 stars 25 forks source link

Staging #20

Closed schammah closed 4 years ago

jmgreg31 commented 4 years ago

These lgtm, let me know if there are more to add. Having trouble locally trying to test outputs so can't confirm/deny any issues there

schammah commented 4 years ago

sure letting you know only way to test is to create a dist before the changes and after to see if they differed will try to do this later today and then we will merge

jmgreg31 commented 4 years ago

outputs for v4.1.1:

Outputs:

active_trusted_signers = {
  "enabled" = "false"
  "items.#" = "0"
}
arn = arn:aws:cloudfront::894518317441:distribution/EUG63XJ3RWA6B
caller_reference = terraform-20200217172352760900000001
domain_name = dyh7k4bqek212.cloudfront.net
etag = E20FOOCD756396
hosted_zone_id = Z2FDTNDATAQYW2
id = EUG63XJ3RWA6B
in_progress_validation_batches = 0
last_modified_time = 2020-02-17 17:23:52.91 +0000 UTC
name = dyh7k4bqek212.cloudfront.net
status = Deployed

outputs for staging:

Outputs:

active_trusted_signers = [
  {
    "enabled" = "false"
    "items.#" = "0"
  },
]
arn = [
  "arn:aws:cloudfront::894518317441:distribution/E2PKIHDXDMAS8X",
]
caller_reference = [
  "terraform-20200217175629575800000001",
]
domain_name = [
  "d3iwcfmwuvzml8.cloudfront.net",
]
etag = [
  "E28DQ1Z2LBXUTL",
]
hosted_zone_id = [
  "Z2FDTNDATAQYW2",
]
id = [
  "E2PKIHDXDMAS8X",
]
in_progress_validation_batches = [
  0,
]
last_modified_time = [
  "2020-02-17 17:56:29.804 +0000 UTC",
]
name = [
  "d3iwcfmwuvzml8.cloudfront.net",
]
status = [
  "Deployed",
]

outputs for v4.1.1 on same distribution created with staging:

Error: Unsupported attribute

  on .terraform/modules/demo_cf/outputs.tf line 2, in output "id":
   2:   value = aws_cloudfront_distribution.cloudfront_distribution.id
    |----------------
    | aws_cloudfront_distribution.cloudfront_distribution is empty tuple

This value does not have any attributes.

Unfortunately this does look to break the output compatability due to the count of create_cf. My thoughts are to remove create_cf and revert the outputs, or bump to a major version release and remove all the lingering backwards compatibility issues

schammah commented 4 years ago

outputs for v4.1.1:

these outputs are regular variables

outputs for staging:

Outputs:

active_trusted_signers = [
  {
    "enabled" = "false"
    "items.#" = "0"
  },
]
arn = [
  "arn:aws:cloudfront::894518317441:distribution/E2PKIHDXDMAS8X",
]
caller_reference = [
  "terraform-20200217175629575800000001",
]
domain_name = [
  "d3iwcfmwuvzml8.cloudfront.net",
]
etag = [
  "E28DQ1Z2LBXUTL",
]
hosted_zone_id = [
  "Z2FDTNDATAQYW2",
]
id = [
  "E2PKIHDXDMAS8X",
]
in_progress_validation_batches = [
  0,
]
last_modified_time = [
  "2020-02-17 17:56:29.804 +0000 UTC",
]
name = [
  "d3iwcfmwuvzml8.cloudfront.net",
]
status = [
  "Deployed",
]

all outputs have turned into lists because of the create_cf (the count)

outputs for v4.1.1 on same distribution created with staging:

Error: Unsupported attribute

  on .terraform/modules/demo_cf/outputs.tf line 2, in output "id":
   2:   value = aws_cloudfront_distribution.cloudfront_distribution.id
    |----------------
    | aws_cloudfront_distribution.cloudfront_distribution is empty tuple

This value does not have any attributes.

No relevant to do 4.1.1 with staging branch SINCE as soon as you make the first apply then the outputs get updated

Unfortunately this does look to break the output compatability due to the count of create_cf. My thoughts are to remove create_cf and revert the outputs, or bump to a major version release and remove all the lingering backwards compatibility issues

i wouldn't want to revert, got 2 proposed solutions to fix it

i suggest i fix the outputs to first element and we make it either a v5.0.0 or v4.2.0 anyway

i'll start working on this and commit soon

schammah commented 4 years ago

@jmgreg31 please test the outputs now with the same setup you used yesterday

jmgreg31 commented 4 years ago

outputs v4.1.1:

Outputs:

active_trusted_signers = {
  "enabled" = "false"
  "items.#" = "0"
}
arn = arn:aws:cloudfront::894518317441:distribution/E2IZ94CEBHJT2F
caller_reference = terraform-20200218144652428400000001
domain_name = d3ly3299n0khci.cloudfront.net
etag = EAOVR7FRH8T1V
hosted_zone_id = Z2FDTNDATAQYW2
id = E2IZ94CEBHJT2F
in_progress_validation_batches = 0
last_modified_time = 2020-02-18 14:46:52.613 +0000 UTC
name = d3ly3299n0khci.cloudfront.net
status = Deployed

Attempt to plan on new testing/staging branch:

Error: Inconsistent conditional result types

  on .terraform/modules/demo_cf/outputs.tf line 22, in output "active_trusted_signers":
  22:    value       = length(aws_cloudfront_distribution.cloudfront_distribution) > 0 ? aws_cloudfront_distribution.cloudfront_distribution[0].active_trusted_signers : ""
    |----------------
    | aws_cloudfront_distribution.cloudfront_distribution is tuple with 1 element
    | aws_cloudfront_distribution.cloudfront_distribution[0].active_trusted_signers is map of string with 2 elements

The true and false result expressions must have consistent types. The given
expressions are map of string and string, respectively.
schammah commented 4 years ago

i'ved fixed it, since we expect a map on this turnery, need to return now empty map on the condition

schammah commented 4 years ago

@jmgreg31 let me know if you can test once more should be okay now then we can push this for now and we'll see for the refactor rewrite new syntax in a next version

jmgreg31 commented 4 years ago

Yup will test this again. Assuming this works, any other commits before merge to staging?

schammah commented 4 years ago

nope, if it pass we can merge to staging then to prod, all the other stuff i'll prepare for a future version

jmgreg31 commented 4 years ago

These changes look to have resolved the discrepancies and should be compatible.

output testing/staging:

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

Outputs:

active_trusted_signers = {
  "enabled" = "false"
  "items.#" = "0"
}
arn = arn:aws:cloudfront::894518317441:distribution/E1U15GPDES4OG0
caller_reference = terraform-20200219005634645000000001
domain_name = d341zzaq06qi5m.cloudfront.net
etag = E3THG9N5VNRMJM
hosted_zone_id = Z2FDTNDATAQYW2
id = E1U15GPDES4OG0
in_progress_validation_batches = 0
last_modified_time = 2020-02-19 01:08:52.419 +0000 UTC
name = d341zzaq06qi5m.cloudfront.net
status = Deployed

Will merge this PR with staging