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

Support nullable flag for variables with default values #63

Closed
JasP19 opened this issue Jan 5, 2022 · 2 comments · Fixed by #81
Closed

Support nullable flag for variables with default values #63

JasP19 opened this issue Jan 5, 2022 · 2 comments · Fixed by #81

Comments

@JasP19
Copy link

JasP19 commented Jan 5, 2022

Describe the Feature

Add nullable flag (nullable = false) for module input variables which have a default value and where null is not a sensible/handled value for the variable.

This should not be a breaking change since, as far as I can tell, the module does not currently handle null values for variables.

I am happy to tackle this feature request if you are happy with it.

Expected Behavior

Module input variables (satisfying the description, above) use the nullable flag and set the value to false which makes them assume their default value when a null input value is supplied.

Use Case

This is useful when consuming the module generically (e.g. using Terragrunt for different environments):

terraform {
  source = "${local.base_source_url}?ref=vx.x.x"
}

locals {
  # Automatically load variables
  account_vars = read_terragrunt_config(find_in_parent_folders("account.hcl"))
  service_vars = read_terragrunt_config(find_in_parent_folders("service.hcl"))

  # Extract out common variables for reuse
  account_name = local.account_vars.locals.account_name

  base_source_url = "<some-source-url>"
}


# ---------------------------------------------------------------------------------------------------------------------
# MODULE PARAMETERS
# These are the variables we have to pass in to use the module. This defines the parameters that are common across all
# environments.
# ---------------------------------------------------------------------------------------------------------------------  
inputs = {
  namespace = "${local.account_name}"
  stage = local.service_vars.locals.s3_log_stage
  name = local.service_vars.locals.s3_log_name
  acl = local.service_vars.locals.s3_log_acl
  standard_transition_day = local.service_vars.locals.s3_log_standard_transition_day
  glacier_transition_days = local.service_vars.locals.s3_log_glacier_transition_days
  expiration_days = local.service_vars.locals.s3_log_expiration_days
  allow_encrypted_uploads_only = local.service_vars.locals.s3_log_allow_encrypted_uploads_only
  allow_ssl_requests_only = local.service_vars.locals.s3_log_allow_ssl_requests_only
  tags = local.service_vars.locals.s3_log_tags
  policy = local.service_vars.locals.s3_log_policy
}

By making the input variables nullable, we can set ours to null and get the default values without having any knowledge of the default values. We can also easily override the default values on other environments, where needed.

Describe Ideal Solution

Add nullable flag (nullable = false) for module input variables which have a default value and where null is not a sensible/handled value for the variable. This will include a Terraform required version >= 1.1.

Additional Context

When calling a module and supplying null for one of the input parameters, Terraform's default behaviour was always to set the actual value of the variable to null. There is a thread where many people have contested this behaviour: hashicorp/terraform#24142

Terraform 1.1 has released a nullable flag which allows module creators to choose whether null is a valid value for an input variable, or whether the variable should take on the default value when set to null: hashicorp/terraform#29832

@Nuru
Copy link
Sponsor Contributor

Nuru commented Feb 23, 2022

I believe this is a breaking change in the sense that it would require Terraform version >= 1.1, right? We are still, at the moment, allowing Terraform 0.14 for most of our modules, and only just starting to require Terraform 1.0. So this is something we would not want to do now, but of course it is a fine suggestion for when we get to requiring Terraform version >= 1.1.

@JasP19
Copy link
Author

JasP19 commented Feb 23, 2022

Yes, that's a fair point. Thanks for the response. We will wait until the decision is made to require Terraform >= 1.1 for the Cloud Posse modules.

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

Successfully merging a pull request may close this issue.

2 participants