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

parameter_group_name always shows a diff and causes apply to fail for aws_elasticache_cluster #2468

Closed
brikis98 opened this issue Nov 29, 2017 · 5 comments
Labels
bug Addresses a defect in current functionality.

Comments

@brikis98
Copy link
Contributor

brikis98 commented Nov 29, 2017

Terraform Version

Terraform v0.10.8

Affected Resource(s)

  • aws_elasticache_cluster

Terraform Configuration Files

resource "aws_elasticache_cluster" "memcached" {
  engine = "memcached"

  cluster_id = "my-cluster"
  port = 11211
  parameter_group_name = ""

  num_cache_nodes = 1
  node_type = "cache.t2.micro"
}

Note that this error happens with both redis and memcached.

Expected Behavior

After you run apply, there are no diffs in subsequent plan.

Actual Behavior

After you run apply, the next plan shows the following, even though nothing changed:

 ~ aws_elasticache_cluster.memcached
      parameter_group_name: "default.memcached1.4" => <computed>

Worse yet, if you run apply again, you get an error:

* aws_elasticache_cluster.memcached: [WARN] Error updating ElastiCache cluster (memcached-prod), error: InvalidParameterCombination: No modifications were requested

Steps to Reproduce

  1. terraform apply
  2. terraform plan
  3. terraform apply

Important Factoids

You can work around this by setting the parameter group explicitly:

parameter_group_name = "default.memcached1.4"
@brikis98
Copy link
Contributor Author

Update: this bug also affects aws_db_instance and other related resources with parameter_group_name.

@brikis98
Copy link
Contributor Author

One more update: it looks like the license_model on aws_db_instance has a similar issue, were you always get a diff with the plan command. Fortunately, that one does not cause apply to fail.

But the general pattern seems to be consistent: if you set various params to an empty string in Terraform, AWS fills in some default value itself, and you get a diff on the next run of plan.

@apparentlymart apparentlymart added the bug Addresses a defect in current functionality. label Dec 19, 2017
@apparentlymart
Copy link
Member

Thanks for filing this @brikis98, and sorry for the delay in replying.

The attribute parameter_group_name is flagged as being both Computed and Optional, which is supposed to tell Terraform to expect the remote API to provide a value when one isn't specified, but in this case that mechanism isn't working properly because parameter_group_name is specified, from Terraform's perspective, but it's set to an empty string.

This is, therefore, an inconsistency between Terraform's interpretation and the remote API's interpretation: Terraform considers the empty string to be an explicitly-provided value, while the API considers it to be equivalent to "unset" and it provides a default.

Unfortunately I think properly fixing this will require waiting until we're further along with integrating the new config language parser/evaluator, since that introduces a first-class idea of "null" that will then allow us to distinguish between "unset" and "set to empty". As we currently stand, we can't change the diff behavior to treat empty string as unset because there are some resources for which empty string is a legitimate value.

For now, omitting the attribute entirely is the only way to properly request the server-provided default in a way that Terraform will understand. I assume that in practice you're trying to omit it conditionally in a re-usable module, in which case unfortunately for now I think the only path -- until we have explicit support for null as an expression -- is for the module to duplicate the server-provided default and set it explicitly:

  parameter_group_name = "${var.parameter_group_name != "" ? var.parameter_group_name : "default.memcached1.4"}"

In this particular case I think there is at least a predictable default to use. In other cases it's trickier because the server-provided default varies depending on other settings in the user's account, so this is indeed not a fully-general solution as the forthcoming support for null would be:

# NOT YET IMPLEMENTED
  parameter_group_name = var.parameter_group_name != "" ? var.parameter_group_name : null

# (or indeed perhaps the variable's own default is null and so the conditional isn't needed at all)

@apparentlymart
Copy link
Member

After reading #2451 I realized that this seems to be the same v1.3.0 regression reported in #2348, so I'm going to close this one to consolidate the discussion over there, but I'll capture a summary of what's here in that issue so we don't lose it.

@ghost
Copy link

ghost commented Apr 10, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality.
Projects
None yet
Development

No branches or pull requests

2 participants