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

ignore_changes for set arguments #26359

Open
marlenepereira opened this issue Sep 23, 2020 · 23 comments
Open

ignore_changes for set arguments #26359

marlenepereira opened this issue Sep 23, 2020 · 23 comments

Comments

@marlenepereira
Copy link

We are using custom headers to increase security in the communication between cloudfront and origin, as recommended by aws. The custom header is modified by a process outside of Terraform, so we need to add ignore_changes to custom_headers.

Terraform Version

Terraform v0.13.3

Terraform Configuration Files

resource "aws_cloudfront_distribution" "distribution" {
  origin {
    domain_name = var.default_origin_domain
    origin_id   = local.origin_id

    custom_origin_config {
      http_port              = 80
      https_port             = 443
      origin_protocol_policy = "https-only"
      origin_ssl_protocols   = ["TLSv1.2"]
    }
    custom_header {
      name  = local.custom_header_name
      value = "replace-me"
    }
  }

  tags = local.common_tags

  enabled         = true
  is_ipv6_enabled = true
  comment         = local.name_prefix

  lifecycle {
    prevent_destroy = false
    ignore_changes  = [origin.0.custom_header]
  }

  logging_config {
    bucket = aws_s3_bucket.distribution_logs.bucket_domain_name
    prefix = local.name_prefix
  }

  aliases = formatlist("%s.${var.edge_domain}", var.edge_aliases)

  default_cache_behavior {
    allowed_methods  = ["DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"]
    cached_methods   = ["GET", "HEAD"]
    target_origin_id = local.origin_id
    compress         = true

    forwarded_values {
      query_string = true
      headers      = ["*"]

      cookies {
        forward = "all"
      }
    }

    viewer_protocol_policy = "https-only"
    min_ttl                = 0
    default_ttl            = 3600
    max_ttl                = 86400

    lambda_function_association {
      event_type = "origin-request"
      lambda_arn = aws_lambda_function.edge_lambda.qualified_arn
    }
  }

  restrictions {
    geo_restriction {
      restriction_type = "none"
    }
  }

  viewer_certificate {
    acm_certificate_arn      = aws_acm_certificate.distribution.arn
    minimum_protocol_version = "TLSv1"
    ssl_support_method       = "sni-only"
  }
}

Output

Error: Cannot index a set value

  on cloudfront.tf line 31, in resource "aws_cloudfront_distribution" "distribution":
  31:     ignore_changes  = [origin.0.custom_header]

Block type "origin" is represented by a set of objects, and set elements do
not have addressable keys. To find elements matching specific criteria, use a
"for" expression with an "if" clause.

Expected Behavior

Custom header should have been ignored and terraform plan succeeded.

Steps to Reproduce

  1. terraform init
  2. terraform plan

Additional Context

  1. We have tried variations of lifecycle ignored_changes described in the reference below, but none have worked.
  2. We can't use lifecycle {ignore_changes = [origin]} because we still need to modify other properties of the origin (i.e. custom_origin_config).

References

@marlenepereira marlenepereira added bug new new issue not yet triaged labels Sep 23, 2020
@marlenepereira marlenepereira changed the title Cloudfront ignore_changes for set arguments ignore_changes for set arguments Sep 23, 2020
@jbardin jbardin added config enhancement and removed bug new new issue not yet triaged labels Sep 24, 2020
@nexxai
Copy link
Contributor

nexxai commented Sep 28, 2020

I apologize for the stupid question, but shouldn't you just be doing ignore_changes = [ origin.custom_header ]? the .0 is only for when you're creating multiple instances of that block, which it doesn't seem like you are doing.

@jbardin
Copy link
Member

jbardin commented Oct 5, 2020

@nexxai, blocks are always contained within their respective data type, and must be addressed accordingly. This means that a List element would always require and index, and a Set element is not addressable. In older versions of Terraform, individual resources with a count of 1 could be referenced either way, but that behavior was also deprecated to allow for more strict handling of data types.

@nexxai
Copy link
Contributor

nexxai commented Oct 5, 2020

Got it. Makes sense. Thank you for the explanation!

@JoseFMP
Copy link

JoseFMP commented Feb 15, 2021

So, how should it be addressed?

@dmitriy-drenkaliuk
Copy link

Is there a workaround for this issue? When a fix can be expected?

@bflad bflad added the lifecycle label Dec 1, 2021
@apparentlymart
Copy link
Contributor

It seems like this particular problem has a root cause which has no clear solution, but that there are also some specific situations which are in principle simpler to handle but still require being able to describe something that isn't describable in the language today.

The root problem here is that by definition elements of a set doesn't have any key or index to look them up by. Set elements are defined by their contents, but that isn't helpful in a situation like ignore_changes where the whole point is to talk about values changing. Set elements are inherently immutable because what might seem like a change to one of the arguments is really, due to the definition of a set, the removal of the old element and the addition of a new one.

In discussions about this so far I don't think anyone's been able to offer a feasible design for solving this in the general case, but there are two specific situations that are, in theory at least, a bit easier to deal with:

  • When there are no elements of the set, and therefore it's only meaningful to talk about the set as a whole.
  • When there is exactly one element of the set, and therefore there's no real need to be able to identify it; no other one could possibly be intended.

We already have the function one which exploits these two special cases to address a similar problem that arises when looking up values from a set of objects: the author can assert by using this function that there should only ever be zero or one values in the set, and thus Terraform can assume that it doesn't actually matter which element we're talking about because if there were more than one we'd return an error anyway.

In principle we could support ignore_changes for sets that only ever have zero or one elements, but it isn't really clear to me what syntax would make sense to talk about that inside ignore_changes. It's not a situation where we're looking up a value, so arbitrary expressions including function calls are not appropriate there.

It also isn't clear to me that such a solution would be sufficient anyway. This particular example only has one origin block, but I believe this resource type allows any number of origins, and so a solution which only worked for zero or one blocks would not be enough to address a case where there were two origin blocks.

My sense is that the most practical answer here would be that any block type that might potentially need to have traversals into it (whether in ignore_changes or elsewhere in the configuration) should not be represented by a set of objects. Terraform offers two other possibilities for representation of block types that can accept zero or more instances:

  • A list of objects is appropriate if there is an inherent order to the blocks which the remote system will be able to preserve. The remote system being able to preserve it is important because otherwise reading back the same data from the remote system could produce a different order, causing the configuration and remote system to never converge. It would then be possible to write ignore_changes = [ block_type[0] ] to identify the zeroth block in particular.
  • A map of objects could be appropriate if they aren't in any particular order but there's some reasonable key to use to uniquely identify each block. The syntax in that case would be block_type "label" instead of just block_type, and then it would be possible to write ignore_changes = [ block_type["label"] ] to identify a particular block individually.

Both of these are supported by Terraform Core today, but map-based blocks in particular are not supported by the legacy SDK that aws_cloudformation_distribution is currently implemented with, and so it would not be viable for the AWS provider to just trivially switch to using that mechanism. Even if it could, requiring a label on each origin block would be a breaking change and so not something that could be taken lightly.

With all of this said then: for me it seems like the most viable path forward is to move away from using sets of objects to represent multiple blocks in most cases, because that offers a clear way to talk about individual blocks when writing traversal expressions elsewhere in the configuration. However, I don't see any easy direct path to that or any other solution, because changing the data type of an attribute or block in an existing resource type is a breaking change.

@jigfox
Copy link

jigfox commented Mar 11, 2022

We just ran into the same problem, but for our use case it would be totally sufficient if all origins (just one exists) would be ignored, so we tried

resource "aws_cloudfront_distribution" "distro" {
  //
  lifecycle {
    ignore_changes = [origin,etag,last_modified_time]
  }
  //
}

Untitled 5

but we still get changes

@apparentlymart
Copy link
Contributor

ignore_changes is for ignoring changes in the configuration, not for ignoring changes in the remote system. That particular message is reporting that if you apply this change then the Terraform state will be updated to record these updated values. If this is a situation where you are expecting particular values to change outside of Terraform because your Terraform configuration isn't attempting to manage them, then #28803 is the issue to follow for that.

@xremming
Copy link

xremming commented May 4, 2022

Would it not be possible to allow for expression with an if clause when ignoring changes? It could still be limited to not allow function calls or other problematic stuff. Another solution I can see would be to allow using lifecycle directive inside of all blocks, so you could have it inside the origin block and the reference there would be local.

@lra
Copy link

lra commented Jul 8, 2022

For context, we have the exact same issue with the ibm_database resource, IBM decided to use a set for dynamic disc allocation, it changes over time, and we cannot ignore it. See group->disk->allocation_mb.

@a-abella
Copy link

One more joining the party here, affecting aws_lb_listener (and presumably aws_lb_listener_rule)

When configuring the default_action of an aws_lb_listener resource with an action of forward to multiple target_group objects, like so, it becomes impossible to ignore changes to the target group routing weight because the target groups are defined as a set.

resource "aws_lb_listener" "test" {
  (...)
  default_action {
    type = "forward"
    forward {
      target_group {
        arn = foo
        weight = 100
      }
      target_group {
        arn = bar
        weight = 0
      }
    }
  }
}

For blue/green deployments we would like to control these weight values outside Terraform but we cannot ignore them without ignoring the whole default_action[0].forward block. This then prevents us from adding or removing target groups from the forwarding list entirely.

@salvianreynaldi
Copy link

it seems there's a recent change on the said ECS Blue/Green deployments with CodeDeploy.
Previously CodeDeploy will detach the inactive target group so we can just ignore default_action[0].target_group_arn in terraform.

Lately CodeDeploy leaves both target group attached at the end of deployments, so we need to ignore the 2 target group blocks.
How do you ignore the entire forward block @a-abella?

@a-abella
Copy link

@salvianreynaldi I don't know about whatever resource you're using, but on aws_lb_listener to ignore the forward rule default_action I think we did:

    ignore_changes = [
      default_action[0].forward[0]
    ]

@dtheodor
Copy link

dtheodor commented Mar 1, 2023

Sets do not have individually addressable items but it still makes sense to want to ignore changes to all of them. This is also a pattern appearing in list items, wanting to ignore changes to all of them. A syntax such as the following would solve both

ignore_changes = [
    list_or_set[*].attribute_to_ignore
]

@fardarter
Copy link

Sets do not have individually addressable items but it still makes sense to want to ignore changes to all of them. This is also a pattern appearing in list items, wanting to ignore changes to all of them. A syntax such as the following would solve both

ignore_changes = [
    list_or_set[*].attribute_to_ignore
]

This is exactly what is needed.

@fdaugan
Copy link

fdaugan commented Oct 2, 2023

Currently I had to put a rotated secrets in custom header in AWS Cloudfront distribution origin.
Best effort support of * in sets would be nice:

ignore_changes = [
    origin[*].custom_header[*].value
]

@marmol-dev
Copy link

Any news on this? I have the same issue.

@instaclustr-wenbodu
Copy link

It seems like this particular problem has a root cause which has no clear solution, but that there are also some specific situations which are in principle simpler to handle but still require being able to describe something that isn't describable in the language today.

The root problem here is that by definition elements of a set doesn't have any key or index to look them up by. Set elements are defined by their contents, but that isn't helpful in a situation like ignore_changes where the whole point is to talk about values changing. Set elements are inherently immutable because what might seem like a change to one of the arguments is really, due to the definition of a set, the removal of the old element and the addition of a new one.

In discussions about this so far I don't think anyone's been able to offer a feasible design for solving this in the general case, but there are two specific situations that are, in theory at least, a bit easier to deal with:

  • When there are no elements of the set, and therefore it's only meaningful to talk about the set as a whole.
  • When there is exactly one element of the set, and therefore there's no real need to be able to identify it; no other one could possibly be intended.

We already have the function one which exploits these two special cases to address a similar problem that arises when looking up values from a set of objects: the author can assert by using this function that there should only ever be zero or one values in the set, and thus Terraform can assume that it doesn't actually matter which element we're talking about because if there were more than one we'd return an error anyway.

In principle we could support ignore_changes for sets that only ever have zero or one elements, but it isn't really clear to me what syntax would make sense to talk about that inside ignore_changes. It's not a situation where we're looking up a value, so arbitrary expressions including function calls are not appropriate there.

It also isn't clear to me that such a solution would be sufficient anyway. This particular example only has one origin block, but I believe this resource type allows any number of origins, and so a solution which only worked for zero or one blocks would not be enough to address a case where there were two origin blocks.

My sense is that the most practical answer here would be that any block type that might potentially need to have traversals into it (whether in ignore_changes or elsewhere in the configuration) should not be represented by a set of objects. Terraform offers two other possibilities for representation of block types that can accept zero or more instances:

  • A list of objects is appropriate if there is an inherent order to the blocks which the remote system will be able to preserve. The remote system being able to preserve it is important because otherwise reading back the same data from the remote system could produce a different order, causing the configuration and remote system to never converge. It would then be possible to write ignore_changes = [ block_type[0] ] to identify the zeroth block in particular.
  • A map of objects could be appropriate if they aren't in any particular order but there's some reasonable key to use to uniquely identify each block. The syntax in that case would be block_type "label" instead of just block_type, and then it would be possible to write ignore_changes = [ block_type["label"] ] to identify a particular block individually.

Both of these are supported by Terraform Core today, but map-based blocks in particular are not supported by the legacy SDK that aws_cloudformation_distribution is currently implemented with, and so it would not be viable for the AWS provider to just trivially switch to using that mechanism. Even if it could, requiring a label on each origin block would be a breaking change and so not something that could be taken lightly.

With all of this said then: for me it seems like the most viable path forward is to move away from using sets of objects to represent multiple blocks in most cases, because that offers a clear way to talk about individual blocks when writing traversal expressions elsewhere in the configuration. However, I don't see any easy direct path to that or any other solution, because changing the data type of an attribute or block in an existing resource type is a breaking change.

Any update on this? We are facing the same issue.
'map-based blocks in particular are not supported by the legacy SDK that aws_cloudformation_distribution is currently implemented with,'
Are you indicating that the issue will be resolved in MapType of terraform-plugin-framework? Thanks!

@pranit-p
Copy link

Sets do not have individually addressable items but it still makes sense to want to ignore changes to all of them. This is also a pattern appearing in list items, wanting to ignore changes to all of them. A syntax such as the following would solve both

ignore_changes = [
    list_or_set[*].attribute_to_ignore
]

This not work with some resources it showing below error

A single static variable reference is required: only attribute access and
│ indexing with constant keys. No calculations, function calls, template
│ expressions, etc are allowed here.

@medialab-aaron
Copy link

Running into this problem with Fastly provider trying to ignore programmatic changes to a Dictionary done outside of Terraform.

@adv4000
Copy link

adv4000 commented Apr 18, 2024

Same issue when I try to ignore changes on Resource aws_codebuild_project and block file_system_locations mount_options

@tivoodoo
Copy link

I have the same problem for azurerm, trying to ignore notification.contact_groups for Resource azurerm_consumption_budget_subscription

@paololazzari
Copy link

I'm setting up a helm release from terraform and I need this as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests