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

Migrate module to terraform 0.13 #677

Closed
ivankorn opened this issue Sep 17, 2020 · 7 comments · Fixed by #777
Closed

Migrate module to terraform 0.13 #677

ivankorn opened this issue Sep 17, 2020 · 7 comments · Fixed by #777
Labels
enhancement New feature or request

Comments

@ivankorn
Copy link
Contributor

0.13 is here, with for_each, depends_on for whole modules and custom variables validation.

While for_each is more about code cleanup, the vars constraints should really speed-up re-using of the module as we can make API design reflected on vars constraints as well (combinations of variables, possible values etc).
F.e. new precise variables validation would help with avoiding such situations

@ivankorn ivankorn added the enhancement New feature or request label Sep 17, 2020
@ivankorn ivankorn changed the title Migrate module to terraform to 0.13 Migrate module to terraform 0.13 Sep 17, 2020
@bharathkkb
Copy link
Member

+1 I think we will be targeting this sometime late Q4

@bharathkkb
Copy link
Member

Also on a different note, the variable validation is scoped to the defined variable. I am wondering how we can use that to validate the situation you mentioned above, i.e prevent user from setting both release_channel and auto_upgrade. I can see this being useful in other modules as well.

@ivankorn
Copy link
Contributor Author

Also on a different note, the variable validation is scoped to the defined variable. I am wondering how we can use that to validate the situation you mentioned above, i.e prevent user from setting both release_channel and auto_upgrade. I can see this being useful in other modules as well.

The new syntax allows calling functions within condition of the validation block and even more cool those functions can accept variables as parameters as well. So we should be able to pretty flexible validate vars against other vars.

@morgante
Copy link
Contributor

I'm not sure we should "upgrade" to Terraform 0.13 for its own sake (we already support 0.13). Instead I'd like to see any upgrade be tied to direct code or feature enhancements.

@bharathkkb
Copy link
Member

@morgante yes I believe we were trying to figure out a way to prevent some GKE misconfigs from occurring by using the new validation feature in 0.13.

@ivankorn I meant that we wont be able to validate across two variables. Consider an example like this

variable "test_var_1" {
  type        = number
  description = "test var that checks if test_var_1 is greater than 1 and test_var_2 is greater than 2"

  validation {
    condition     = var.test_var_2 > 2 && var.test_var_1 > 1
    error_message = "Oops either test_var_1 is less than 1 or test_var_2 is less than 2."
  }
}
variable "test_var_2" {
  type        = number
  description = "a number greater than 2"
}

this throws an error


Error: Invalid reference in variable validation

  on test.tf line 6, in variable "test_var_1":
   6:     condition     = var.test_var_2 > 2 && var.test_var_1 > 1

The condition for variable "test_var_1" can only refer to the variable itself,
using var.test_var_1.

In our case here we would need both the var. release_channel and var. auto_upgrade to make a validation, right?

@DeviaVir
Copy link

Wanted to leave a note here that it looks like this module does not support Terraform 0.13's count as it currently stands:

The Terraform configuration must be valid before initialization so that
Terraform can determine which modules and providers need to be installed.

Error: Module does not support count

  on us.tf line 29, in module "gke":
  29:   count = local.create_us

Module "gke" cannot be used with count because it contains a nested
provider configuration for "kubernetes", at
.terraform/modules/gke/modules/beta-private-cluster/auth.tf:29,10-22.

This module can be made compatible with count by changing it to receive all of
its provider configurations from the calling module, by using the "providers"
argument in the calling module block.

Documentation: https://www.terraform.io/docs/configuration/modules.html#legacy-shared-modules-with-provider-configurations

As far as I am able to interpret, we need to replace/remove the kubernetes provider configuration out of the module and make sure it is passed by the passing module.. which seems hard to do in this case as the provider configuration is based on the module.

@xingao267
Copy link
Member

Wanted to leave a note here that it looks like this module does not support Terraform 0.13's count as it currently stands:

The Terraform configuration must be valid before initialization so that
Terraform can determine which modules and providers need to be installed.

Error: Module does not support count

  on us.tf line 29, in module "gke":
  29:   count = local.create_us

Module "gke" cannot be used with count because it contains a nested
provider configuration for "kubernetes", at
.terraform/modules/gke/modules/beta-private-cluster/auth.tf:29,10-22.

This module can be made compatible with count by changing it to receive all of
its provider configurations from the calling module, by using the "providers"
argument in the calling module block.

Documentation: https://www.terraform.io/docs/configuration/modules.html#legacy-shared-modules-with-provider-configurations

As far as I am able to interpret, we need to replace/remove the kubernetes provider configuration out of the module and make sure it is passed by the passing module.. which seems hard to do in this case as the provider configuration is based on the module.

I'm seeing the same issue with depends_on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants