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 precondition and postcondition blocks in module calls #31122

Open
joe-a-t opened this issue May 24, 2022 · 9 comments
Open

Support precondition and postcondition blocks in module calls #31122

joe-a-t opened this issue May 24, 2022 · 9 comments
Labels
custom-conditions Feedback on variable validation, preconditions. postconditions, checks, and test assertions enhancement new new issue not yet triaged

Comments

@joe-a-t
Copy link

joe-a-t commented May 24, 2022

Current Terraform Version

Terraform v1.2.1

Use-cases

I would like to use https://www.terraform.io/language/expressions/custom-conditions#preconditions-and-postconditions when calling into a module to do some validation of multiple variables at the module invocation level instead of at the resource level.

Attempted Solutions

I could do #25609 (comment) or use a null_resource, both of which allow me to meet my requirements for preconditions. As far as postconditions, I might be able to hack something together with a depends_on in the null_resource, but that's even hackier.

Proposal

Add lifecycle block with precondition and postcondition sub-blocks to the module meta-argument options so that people can get a consistent experience raising errors at both the resource and module level.

References

@joe-a-t joe-a-t added enhancement new new issue not yet triaged labels May 24, 2022
@korinne korinne added the custom-conditions Feedback on variable validation, preconditions. postconditions, checks, and test assertions label May 24, 2022
@crw
Copy link
Collaborator

crw commented May 24, 2022

Thanks for this enhancement request!

@apparentlymart
Copy link
Member

Hi @joe-a-t!

Could you show a hypothetical example of a real configuration you might write if we implemented what you are imagining here? That will help attach this request to at least one specific use-case so we can make sure a solution to it will actually meet your need.

As with module-level for_each, count, and depends_on this is a situation where we'd need to carefully decide what it actually means to place a precondition or postcondition on a module, because Terraform allows each input variable and output value of module to have its own separate dependencies and dependents. A collection of specific use-cases will help make that trade-off so that we can design something that will have minimal surprising interactions with other language features.

One initial hypothesis would be that resource preconditions and postconditions act as if they were declared on each resource in the module separately, but that has a few potentially-tricky consequences:

  • If the module precondition has an unknown result during planning, any data resource inside the module must be read only during apply to make sure that the condition holds before reading it.
  • Module-level postconditions implemented in this way would be weird in that presumably self would mean the object containing the module output values, and so treating it as if the postconditions were on the individual resources would make them evaluate too early, before the output values are evaluated.

So it does seem like some further design consideration is warranted here, to find a suitable compromise. To do that will require some concrete use-cases to design around.

Thanks!

@joe-a-t
Copy link
Author

joe-a-t commented May 26, 2022

Hey @apparentlymart, thanks for the quick attention and comprehensive response.

The core of this issue actually relates to #25609 (comment) and if that issue was solved to allow accessing other variables in variable validation blocks, this issue would be entirely unnecessary from my perspective and that is probably the preferred solution instead of directly addressing this issue.

I don't have any use cases that currently require postconditions, I just included it for completeness/consistency.

As far as I am concerned, having preconditions act as if they were declared on each resource in the module separately would work. I would not care if any data sources got read even if the precondition failed but certainly understand that some other people with different use cases might feel differently and that could be very confusing.

For my specific current use case, all of the values are known and we currently do the validation with #25609 (comment). Our situation is that we have a generic module for kubernetes_deployment and a separate generic module for kubernetes_pod_disruption_budget, both of which provide several guardrails and standardization benefits. We then have some "wrapper" modules, one of which allows for creating a K8s deployment with load balancers (for a web app) and another of which allows for creating a K8s deployment without any load balancers (for workers that pull from a queue use cases).

The validation we are doing is:

  validate_min_available = var.replicas == 1 && var.min_available != null ? tobool("min_available is not supported when replicas=1") : true

where we generate an error if someone provided a replicas value of 1 (used in the kubernetes_deployment module) and specified the min_available variable (which controls whether or not we create a kubernetes_pod_disruption_budget (via a count) with the specified value). The reason we want this validation is that K8s can get into a stuck state if you only have 1 pod and also don't allow K8s to move or replace that pod at all.

The kubernetes_deployment module feels like the wrong place to do this validation since it does not need to be aware of min_available for kubernetes_pod_disruption_budget at all and the kubernetes_pod_disruption_budget module also feels like the wrong place to do the validation since does not need to be aware of the replicas value for the deployment. However, we could pass the replicas value into the kubernetes_pod_disruption_budget module and do the precondition on the kubernetes_pod_disruption_budget resource as a viable work-around, it just seems strange given that many usages of kubernetes_pod_disruption_budget will not have replicas since they instead use autoscaling.

@apparentlymart
Copy link
Member

Thanks for that additional context, @joe-a-t.

I think then we need to be careful here to distinguish between conditions written by a module author to check the correctness of an external call to that module, vs. conditions written by the caller of the module that describe additional assumptions/guarantees the calling module is making about the module call.

For the case of a module containing rules to check its caller, I think #25609 remains the best place to discuss that and as I noted over there our intent would be to make validation for variables work the way originally intended, rather than to introduce an additional similar syntax based on postconditions to do it. While it is true that preconditions inside a module can presently serve as a sort of workaround for the current limitations of variable validation, I don't consider that to be the solution to #25609, which still remains to be solved by reworking how Terraform Core represents input variables internally.

I do expect that we'll hear from others who have use-cases that fit into the other category of the caller themselves making additional assumptions/guarantees about the module call that the module itself isn't aware of, just because in the past we've always heard feedback about anything that a resource can do which a module cannot. For that reason, I'll leave this open as a place for those people to potentially share their use-cases, but it does seem to me like the use-case you shared is more one for #25609 than for this issue, and so it could be helpful to share your same use-case text in a comment there too just to make sure that we'll see it when revisiting that issue in the future.

Thanks!

@apparentlymart apparentlymart changed the title Support pre and post conditions for modules Support precondition and postcondition blocks in module calls May 26, 2022
@rquadling
Copy link

rquadling commented Jun 1, 2022

I have an example use case for having postconditions for module calls.

My use case isn't about the module validating or ensuring the supplied parameters are accurate/appropriate.

It's the case that I want to use this module to retrieve data and ensure it matches MY expectation for the remainder of the configuration.

Using the AWS CLI module for terraform, I can get a list of (for example) AWS SSO Permission Sets. I need to use this module as the AWS Terraform Provider does not currently have a native way to retrieve ALL AWS SSO Permission Sets.

data "aws_ssoadmin_instances" "our_sso" {}

locals {
  sso_instance_arn      = tolist(data.aws_ssoadmin_instances.our_sso.arns)[0]
  sso_identity_store_id = tolist(data.aws_ssoadmin_instances.our_sso.identity_store_ids)[0]
}

module "sso_permission_sets_retrieval" {
  source             = "digitickets/cli/aws"
  version            = "~> 5.0.2"
  role_session_name  = "GetSsoPermissionSets"
  aws_cli_commands   = ["sso-admin", "list-permission-sets", "--instance-arn", local.sso_instance_arn]
  aws_cli_query      = "PermissionSets"
  debug_log_filename = "test-reports/GetSsoPermissionSets.debug.log"
}

locals {
  sso_permission_sets = {
    Support = {
      # Though the arn is recorded here, it is not required once all the required Terraform imports are done for the AWS SSO Permission Sets.
      arn              = "arn:aws:sso:::permissionSet/ssoins-xxxx/ps-yyyy"
      description      = "Support"
      session_duration = "PT1H"
    }
  }
}

I want to make sure that ALL of the SSO Permission Sets are known to Terraform.

So, a postcondition on the module of

  lifecycle {
    postcondition {
      # The condition may not be correct ... but I hope you get the idea.
      condition     = length(setintersection(self.result, keys(local.sso_permission_sets))) == length(self.result)
      error_message = format("Different number of permission sets.")
    }
  }

would allow me to know if someone has created an entirely new permission set without our knowledge.

This is just one example of validating/verifying the pre-existing infrastructure against the Terraform configuration. Many other use cases could exist.

@ascopes
Copy link

ascopes commented May 11, 2023

is there any update on this?

@bt-macole
Copy link

Came here wondering if this is possible. Here is a workaround in >= tf 1.4

resource "terraform_data" "validator" {
    triggers_replace = [
        var.a,
        var.b
  ]

  lifecycle {
    precondition {
        condition = !(var.a == null && var.b == null) && !(var.a != null && var.b != null)
        error_message = "Invalid inputs. Either `var.a` or `var.b` must be a non-null value."
    }
  }
}

full example you can plan locally:

example module

main.tf

resource "terraform_data" "validator" {
    triggers_replace = [
        var.a,
        var.b
  ]

  lifecycle {
    precondition {
        condition = !(var.a == null && var.b == null) && !(var.a != null && var.b != null)
        error_message = "Invalid inputs. One and only one of `var.a` or `var.b` must be a non-null value."
    }
  }
}

variable "a" {
    default = null
}

variable "b" {
    default = null
}

@ascopes
Copy link

ascopes commented Jun 30, 2023

@bt-macole a solution that I use is this:

variable "foo" {
  type = string
  default = null
}

variable "bar" {
  type = string
  default = null
}

resource "terraform_data" "baz" {
  input = timestamp()

  lifecycle {
    precondition {
      // foo || bar
      check = can(coalesce(var.foo)) || can(coalesce(var.bar))
      error_message = "foo or bar must be specified"
    }

    precondition {
      // foo ^ bar
      check = can(coalesce(var.foo)) != can(coalesce(var.bar))
      error_message = "foo or bar must be specified, but not both"
    }
  }
}

Using coalesce will also treat empty strings as being "unfilled values", and using can will hide other erroneous cases you probably don't ever want to cater for.

If you have collections, you can check they are non-empty and contain at least one non null value by unpacking:

variable "foo" {
  type = list(string)
  default = null
}

resource "terraform_data" "bar" {
  input = timestamp()

  lifecycle {
    precondition {
      // any(foo)
      check = can(coalesce(var.foo...))
      error_message = "foo must be a list with at least one non null value"
    }
  }
}

If the list is null, unpacking it will fail, causing can to return false. If it is an empty list, the call to coalesce will fail as it needs at least one parameter, so can will return false. If there is a null value unpacked, coalesce will ignore them until it finds a non-null value.

Exploiting can feels a bit dirty to me but it has worked me around having these short-circuiting issues.

HashiCorp could consider allowing the use of the & and | and ^ operators to allow non-short-circuiting evaluations going forwards in TF 1.6, and make && and || strictly short circuiting.

@duxbuse
Copy link

duxbuse commented Nov 29, 2023

My use case is I have a module that can handle a few types of input.

But at one stage when I call this module I want to ensure that only one type of input is provided.

eg.

locals {
  people = {"tom": "project", "dick": "project", "harry": "project"}
 }

module "iam_binding" {
  for_each = local.people
  member = each.key
  role  = "roles/viewer"
  type = "project" # could be [project | org | folder | vm]
  
  lifecycle {
     precondition {
       condition = each.value == "project"
       error_message = "Can only bind to project level here"
      }
  }
 
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
custom-conditions Feedback on variable validation, preconditions. postconditions, checks, and test assertions enhancement new new issue not yet triaged
Projects
None yet
Development

No branches or pull requests

8 participants