hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.71k stars 9.55k forks source link

Integer variables are not supported #6254

Closed joshuaspence closed 6 years ago

joshuaspence commented 8 years ago

I tried to do something like this:

variable "ctld_zone_count" {
  type = "integer"
  description = "The number of country top-level domains."
  default = 0
}

But it seems that integer is not a valid variable type.

joshuaspence commented 8 years ago

It seems that boolean is also not a valid variable type.

spkane commented 8 years ago

This is particularly odd given that the count parameter requires an integer and not a string. It is not clear how one is supposed to pass an integer through various modules, etc. without it being converted to a string.

spkane commented 8 years ago

It seems a lot better to allow a module integer variable to be defined with no default value like this:

variable "count_master" {
  description = "How many master nodes to create"
  type = "integer"
}

instead of needing to force the variable to be recognized as an integer by giving it a default value like this:

variable "count_master" {
  description = "How many master nodes to create"
  default = 0
}
bilalahmad99 commented 8 years ago

I dont understand if there is a way to pass int variable to a module etc.

thomasbiddle commented 7 years ago

As another added note; I feel like the documentation is lacking in specifically stating a list of variable types. It does show examples of various configurations such as list, map, etc. But It would be great to have a paragraph at the top that simply says "The supported variable types are: x,y,z" as a preface to the rest of it.

apparentlymart commented 7 years ago

There are automatic conversions between int, bool and string, so it works to pass a decimal representation of a number as a string and then assign it to an argument that expects a number. In general Terraform does not make a strong distinction between different scalar types; the type argument on a variable exits to distinguish scalars from lists and maps.

thomasbiddle commented 7 years ago

@apparentlymart Should this be closed as will-not-fix then? Maybe add more documentation on the topic?

apparentlymart commented 7 years ago

It's likely that there will be some changes coming in this area as part of some configuration language improvements we're starting to design now, but this isn't something that will change in the immediate term so I think documenting the current behavior is the right thing to do for the sake of this issue and we'll open other issues and PRs (probably on broader changes than just this fix) later once we have a stronger sense of the future state of this.

(The current idea -- still just a sketch for the moment -- is to eventually stop requiring type altogether and instead just infer the type from what's passed. That way we can more completely support nested data structures such as lists of maps and maps of maps without having to define an arcane micro-syntax for describing the expected type. More on this later, when things are more concrete...)

thomasbiddle commented 7 years ago

@apparentlymart @joshuaspence @bilalahmad99

Copy isn't my forte; does the above PR seem appropriate?

pixelicous commented 6 years ago

@apparentlymart is there any eta on this? for azurerm provider this totally breaks arm templates that rely on int parameters, while you can switch those parameters to strings and pass int(variable) everywhere, it breaks parameter values such as minValue and maxValue properties that expect the parameter to be an int and not a string

apparentlymart commented 6 years ago

Hi @pixelicous! Sorry that things aren't working as expected there.

I'm not sure exactly what problem you are seeing but it should not be required to explicitly use int to convert strings to integer... Terraform should automatically convert when needed. If you're seeing errors when passing a string containing digits to an attribute that expects an integer then I'd encourage you to open an issue with the provider in question (azurerm in your case, it seems) and we can try to figure out what's not working there.


In the current model, the main purpose of this type argument in the variable block is as a hint to Terraform about how to parse variables set from the command line or from environment variables, since primitive values are passed literally while lists and maps are given in HCL syntax.

In the current set of configuration language work we've so far retained that interpretation, but I'm going to investigate what it might look like to expose more complex type constraints here. My concern is that forcing users to describe types in detail will require a lot more exposure to the internals of the type system than we expect today, where usually instead we just automatically convert values where needed and don't require users to think about the difference between true and "true", for example. (some bugs notwithstanding; the type conversion mechanism should be a lot more robust in the new config language interpreter.)

pixelicous commented 6 years ago

@apparentlymart this isn't a provider issue and it isn't going to be solved on the arm template sides, they support types to greater extent..

I hear what you are saying but like any other code language, the option should be there and the operator of the system should know how to handle itself.. if you don't, things break, even in far more complex coding languages.

Regarding our flow and why it break, it is like this: We have a module that receives variables and creates resources, one of the .tf files is to create and arm template resource, basically a json file that receives parameters. When you pass values to the arm templates you pass them as parameters, in the HCL resource for arm template I place the variable name in the parameter field, this part passes the variable as a string to the json file, and the azure parser fails saying you cannot pass string to an integer parameter.

Th other solution i can think of would be to use int function when i pass the variable to the parameter

georgepuckett commented 6 years ago

count = "${var.environment_name == "prd"}"

This returns 'true' rather than 1 if that variable is 'prd'. I can't use this boolean value with AWS resources, and it's not implicitly cast to integer as the comments here seem to suggest

ex:

apparentlymart commented 6 years ago

@georgepuckett strings can convert to and from both numbers and booleans, but booleans do not convert to numbers. For the situation you're describing here, the usual idiom is this:

  count = "${var.environment_name == "prd" ? 1 : 0}"

This makes the conversion explicit, allowing you and future readers to see how the expression relates to the count.


It's been a while since I commented here, so I wanted to share an update: as we continued to iterate on the configuration language improvements project (still in progress, and coming in the next major release) we've settled on changing the type argument for input variables to accept type expressions, which allow a full description of the expected type for each variable:

# Not yet implemented, and details may change before release

variable "boolean_value" {
  type = bool
}

variable "list_of_strings_value" {
  type = list(string)
}

For compatibility with existing configurations, the keywords list and map are still accepted alone and interpreted as "list of any single type" and "map of any single type" respectively, similar to their 0.11 interpretation. However, as shown above it'll be possible to give a specific element type if appropriate.

The other thing here, which I notice I didn't touch on before since this issue dates back to before the language spec had fleshed out, is that the new configuration language interpreter no longer distinguishes between integer and float types, and instead just has a single number type that can represent both whole numbers and fractional numbers to arbitrary precision. Resource attributes and function parameters will still check that that the given value is within the expected range and generate errors if, for example, a fractional number is given where a whole number is expected. As a consequence of this, a number variable would be specified as follows:

# Not yet implemented, and details may change before release

variable "ctld_zone_count" {
  type        = number
  description = "The number of country top-level domains."
  default     = 0
}

We made this tradeoff because we've seen lots of confusion and difficulty around integer vs. float division, float precision, and other similar issues and so unifying the number types allows for better consistency and more predictable behavior. The int function will still be available, redefined as "remove the fractional part of the number" rather than "convert from float to int".

georgepuckett commented 6 years ago

That idiom still produces this:

edit: what a bonehead, i had the closing brace for the conditional statement in the wrong place

Thanks for your help!

jainishshah17 commented 6 years ago

I am facing the same issue. It will great to have variables type number/integer.

pixelicous commented 6 years ago

@apparentlymart Great thanks for the update. Hopefully this "number" type will be passed to other applications as an integer and not a string (for example an Azure arm template), else we still have to modify all our arm templates to receive strings and then convert them to integers

apparentlymart commented 6 years ago

Under v0.12-alpha1 I just tried the following configuration:

variable "example_number" {
  type    = number
  default = 0
}

variable "example_bool" {
  type    = bool
  default = false
}

variable "example_list" {
  type    = list(string)
  default = []
}

With these type declarations in place, Terraform Core will guarantee that the values returned by var.example_number, var.example_bool, and var.example_list are always of the requested type, returning an error to the caller if the given value cannot be converted:

$ terraform apply -var="example_number=hello"

Error: Invalid value for input variable

The argument -var="example_number=..." does not contain a valid value for
variable "example_number": a number is required.

$ terraform apply -var="example_bool=hello"

Error: Invalid value for input variable

The argument -var="example_bool=..." does not contain a valid value for
variable "example_bool": a bool is required.

I used root module variables and the -var option for simplicity here, but a similar error message is returned for a call to a child module, with the added bonus that it can indicate the source code location of the invalid expression to make it easier to find and fix.

If the values of these variables were assigned to a resource argument of the same type then the value will be passed through to the provider itself with no conversions at all. Otherwise, as before Terraform will attempt an automatic conversion, using the same rules as for the module variable values themselves.

It is up to providers to decide which type to use for each argument. With v0.12, the provider protocol uses exactly the same type system as module input variables and output values, so a provider can also declare an attribute as being a number and again Terraform Core will guarantee that it receives a number, or return an error to the user if that is not possible. The provider may in turn choose to apply additional validation to that number, such as requiring it to be an integer, but that additional validation logic belongs to the provider itself (or to the provider SDK).

As I noted in an earlier comment, the int function is still available within the language itself to force a number to be an integer by discarding any fractional part.

So with all of this said, I think v0.12-alpha1 (and thus, by extension, the eventual v0.12.0 final release) address the ideas discussed in this issue. There may be some details to refine in subsequent releases, but we can address those using more focused new issues if needed. For now, I'm going to close this.

Thanks all for the great discussion here about the challenges you faced and ideas for addressing them!

ghost commented 4 years ago

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.