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

Not possible to disable workload identity /modules/beta-private-cluster v8.1.0 #535

Closed
konstantin-recurly opened this issue May 22, 2020 · 13 comments · Fixed by #542
Closed
Assignees
Labels
bug Something isn't working P2 high priority issues triaged Scoped and ready for work

Comments

@konstantin-recurly
Copy link

konstantin-recurly commented May 22, 2020

This is the variable

variable "identity_namespace" {
  description = "Workload Identity namespace. (Default value of `enabled` automatically sets project based namespace `[project_id].svc.id.goog`)"
  type        = string
  default     = "enabled"
}

This is the local variable

cluster_workload_identity_config = var.identity_namespace == null ? [] : var.identity_namespace == "enabled" ? [{
    identity_namespace = "${var.project_id}.svc.id.goog" }] : [{ identity_namespace = var.identity_namespace
  }]

I would like to use the variable in this way

identity_namespace = null

Expected:
if var.identity_namespace == null it should use an empty list, like so

       workload_identity_config {
           identity_namespace = []
        }

Actual result:

      + workload_identity_config {
          + identity_namespace = "null"
        }

It basically converts null to "null"

@bharathkkb
Copy link
Member

I doubt identity_namespace = [] is possible as identity_namespace takes the identity namespace as a string. Have you tried identity_namespace = ""?

@morgante
Copy link
Contributor

It looks like idenity_namespace might be getting cast to a string.

@morgante morgante self-assigned this May 26, 2020
@morgante morgante added bug Something isn't working P2 high priority issues triaged Scoped and ready for work labels May 26, 2020
@morgante
Copy link
Contributor

morgante commented May 26, 2020

I'm unable to replicate this and successfully disabled workload identity with this config - #542.

Any chance you're using Terragrunt or some other tool which might be incorrectly casting null to "null"?

@konstantin-recurly
Copy link
Author

Yes, I was using terragrunt to run it.

@morgante
Copy link
Contributor

Got it. This is really a bug in Terragrunt (it shouldn't treat null as "null") but we can attempt to work around it.

@konstantin-recurly
Copy link
Author

I think it happens because the variable has type=string
and this

Note that because the values are being passed in with environment variables and json, the type information is lost when crossing the boundary between Terragrunt and Terraform. You must specify the proper type constraint on the variable in Terraform in order for Terraform to process the inputs to the right type.

https://terragrunt.gruntwork.io/docs/reference/config-blocks-and-attributes/#inputs

This is what I am getting if use identity_namespace = ""

+ workload_identity_config {}

Seems that it's what we want.
Thanks

@bharathkkb
Copy link
Member

@morgante since identity_namespace = "" is working do we need to add the guardrail? ihmo either is fine, identity_namespace = "" seems more Terraform native.

@morgante
Copy link
Contributor

I think the guardrail would still be helpful.

@konstantin-recurly
Copy link
Author

Tried to apply what I have posted earlier
identity_namespace = ""
the plan was showing this
+ workload_identity_config {}
and the apply didn't work, I got an error

Error: googleapi: Error 400: Must specify a field to update., badRequest

  on cluster.tf line 22, in resource "google_container_cluster" "primary":
  22: resource "google_container_cluster" "primary" {

Seems that it's not possible to use an empty string.

@morgante
Copy link
Contributor

Can you try setting null explicitly and using the latest module? We don't even generate the workload identity block if it's set to null.

@konstantin-recurly
Copy link
Author

unfortunately, I cannot use the latest version of the module.
with version 8.1.0 if I set it as

  identity_namespace     = null

in the plan it shows null as a string "null"

      + workload_identity_config {
          + identity_namespace = "null"
        }

@morgante
Copy link
Contributor

This is fixed on version 9.2.0, please upgrade to that version.

@konstantin-recurly
Copy link
Author

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2 high priority issues triaged Scoped and ready for work
Projects
None yet
3 participants