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

Allow warning messages within variable validation #32594

Open
derekwinters opened this issue Jan 27, 2023 · 7 comments
Open

Allow warning messages within variable validation #32594

derekwinters opened this issue Jan 27, 2023 · 7 comments

Comments

@derekwinters
Copy link

Terraform Version

Terraform v1.3.7

Use Cases

I want to be able to raise a warning in a Terraform module, similar to how a provider raises a warning for deprecated arguments.

We focus heavily on maintaining backwards compatibility and giving time to transition how modules are used, so we end up deprecating module variables in favor of new variables from time to time. We document this in variable descriptions and the logic that uses them handles the logic for transitioning, but being able to clearly highlight the deprecations in the plan would be very useful as well.

Attempted Solutions

The way we accomplish this today is simply in descriptions, readme/documentation, and in the implementation of the variables.

As a highly simplified example, we may want to change the name of a variable as we update legacy modules to use standard variable name structures and input types across all our modules.

variable "name" {
  description = "The name of the bucket to create. DEPRECATED: Use bucket_name instead. If bucket_name is provided, this string is ignored."
  type        = string
  default     = ""
}

variable "bucket_name" {
  description = "The unique name of the bucket to create. This will be combined with the environment name to create the final bucket name."
  type        = string
  default     = ""
}

variable "environment" {
  description = "The environment name for this resource."
  type        = string
}

locals {
  # Compare two variables to ensure one was provided. Exit with error if both are empty strings.
  provided_name = coalesce(var.bucket_name, var.name)
  bucket_name   = var.bucket_name != "" ? join("-",[var.environment, local.provided_name]) : local.provided_name
}

Proposal

It would be useful to highlight the deprecated variables of a module within the Terraform plan. For this to work, it would need to be possible to choose warning or error for the validation.

variable "name" {
  description = "The name of the bucket to create. DEPRECATED: Use bucket_name instead. If bucket_name is provided, this string is ignored."
  type        = string
  default     = ""

  validation {
    condition     = var.name != ""
    error_message = "This variable has been deprecated, use bucket_name instead."
    error_level   = "warning"
  }
}

This would then trigger a warning message at the end of the plan, similar to provider warnings or variable validation errors.

Warning: Module Variable Condition

  on ../modules/example/variables.tf line 18:
   18: variable "name" {

This variable has been deprecated, use bucket_name instead.

This was checked by the validation rule at variables.tf:23.

References

No response

@derekwinters derekwinters added enhancement new new issue not yet triaged labels Jan 27, 2023
@crw
Copy link
Contributor

crw commented Feb 1, 2023

Thanks for this feature request! If you are viewing this issue and would like to indicate your interest, please use the 👍 reaction on the issue description to upvote this issue. We also welcome additional use case descriptions. Thanks again!

@kmoe
Copy link
Member

kmoe commented Feb 2, 2023

This sounds reasonable to me, given that the variables for a module form part of its public API.

@kmoe kmoe added modules and removed new new issue not yet triaged labels Feb 2, 2023
@olenm
Copy link

olenm commented Jul 28, 2023

I hope this request gets expanded for all constraints, not just within a module - as vars can come from the environment and having non-error warnings is often far superior than crash-and-burn checks.

@Samreay
Copy link

Samreay commented May 16, 2024

This would be a incredible for us - we're currently trying to standardise naming conventions and being able to add in a simple regex validation on inputs (for example, all lower case and dashes instead of underscores) without requiring everything to update at once would be amazing

@bschaatsbergen
Copy link
Member

bschaatsbergen commented Nov 20, 2024

Hey @derekwinters,

Thanks for raising this issue. It seems you're suggesting two distinct features:

  1. Deprecating input variables: This would allow module authors to gradually phase out old input variables, providing a smoother transition for module callers.
  2. Warning diagnostics for custom conditions: This would enable module authors to issue warnings instead of hard errors for certain validation conditions, giving module authors more flexibility in their module interface.

Both ideas are valuable and worth exploring.


Note

For context, when I refer to a module’s "interface" I’m talking about its input variables and output values.

1. Deprecating input variables

We're currently prototyping support for deprecating input variables and outputs. This would provide a similar experience to deprecating provider resource arguments, allowing module authors to gradually phase out input variables and outputs and guide users towards newer, preferred options. You can track the progress of this feature in #36016.

2. Warning diagnostic for custom conditions

Currently, custom conditions in Terraform (like variable validation, preconditions, and postconditions) can only trigger error diagnostics. This can be limiting, as sometimes a warning might be more appropriate for a less critical validation failure.

To address this, we could introduce a warning_message argument in addition to the existing error_message argument. This would allow module authors to specify a warning message that would be displayed instead of an error, giving users more flexibility in handling validation failures.

Here’s an example:

variable "instance_type" {
  type = string
  validation {
    condition     = !contains(["t2.micro", "t2.small", "t2.medium"], var.instance_type)
    warning_message = "Selected instance type may not handle heavy workloads."
  }
}

Alternatively, in a postcondition:

data "aws_instance" "example" {
  instance_id = "i-1234567890abcdef0"
  lifecycle {
    postcondition {
      condition     = data.aws_instance.example.public_ip != ""
      warning_message = "The instance has a public IP, potentially exposing it to the internet."
    }
  }
}

Drawbacks of warning diagnostic for custom conditions

  • Overusing warnings can desensitize users, making it harder to identify important issues. Practitioners tend to ignore warnings until they turn to errors.

@Audiopolis
Copy link

Warnings are common in all sorts of things. Loggers, missing extras in packages, imports of dependencies of dependencies (not in your pyproject.toml), wet floors...

If you ignore warnings, you get what's coming. I want to warn about unconventional or "could fail in some situations" configurations, or "this configuration won't support xyz functionality" without preventing the user from doing those dangerous things. Maybe they're just setting up a quick, dirty, cheap development environment and don't care.

In my IDE (PyCharm), missing Terraform variables are presented as strong warnings in the code (yellow highlighting), and errors on plan/apply. A warning would ideally result in a weak warning (zigzag line), and on plan/apply, a printed, very visible warning.

It's a big request for sure, but even just printing a yellow warning on plan/apply would be enough to catch or ignore problems based on the strictness of the company/CI.

@bschaatsbergen
Copy link
Member

bschaatsbergen commented Nov 24, 2024

Thank you for sharing your thoughts @Audiopolis!

In addition to my earlier comment, and building on that first iteration of the proposal… It’s important to consider how condition evaluation currently works in Terraform. Today, when defining a condition in a custom condition block, if the condition evaluates to "false", Terraform generates an error diagnostic. This behavior is simply a binary gate: when the condition fails, the user gets immediate feedback during the plan-cycle.

The only challenge or potential complaint I foresee is that, as a module author, I would prefer to write conditions that explicitly catch specific cases (conditions that evaluate to "true") when triggering a warning—e.g., "if the door is red, I'm warning you". This differs from how custom conditions are currently written by module authors to trigger errors, where the logic is typically "if the door isn’t red, I’m presenting you an error.".

If that assumption is correct, this introduces a minor condition design challenge for module authors, as the shift in logic may feel unintuitive. Instead of writing a condition to catch unwanted input (where "false" triggers an error), authors would need to invert the logic to catch the input that triggers a warning. Such inverted conditions could be confusing because it requires thinking in terms of conditions that succeed rather than fail to surface the warning diagnostic.

With the existing condition logic ("false" triggers a diagnostic), module authors can invert the condition themselves using a ! (logical NOT). For example:

 variable "instance_type" {
   type = string
   validation {
-    condition = contains(["t2.micro", "t2.small", "t2.medium"], var.instance_type)
+    condition = !contains(["t2.micro", "t2.small", "t2.medium"], var.instance_type)
     warning_message = "Selected instance type may not handle heavy workloads."
   }
 }

Bear with my regex skills—here’s an example to warn users about allocating too many IP addresses (without an invert).

variable "vpc_cidr" {
  type = string
  validation {
    condition = can(regex("/(2[5-9]|3[0-2])$", var.vpc_cidr))
    warning_message = "CIDR blocks larger than /25 may waste IPs."
  }
}

Then again, module authors may already be implementing conditions using an invert, so this approach might not be as unintuitive or new as it seems. I’d love to hear everyone’s thoughts on how you would prefer to write conditions if they were intended to trigger a warning. Also, please let me know if you think my assumptions about defining warning conditions are incorrect. Thanks!

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

7 participants