consul_acl_role support policies by name

[the-nando]

Terraform Version

Terraform v1.4.6
on darwin_arm64
+ provider registry.terraform.io/hashicorp/consul v2.17.0

Affected Resource(s)

• consul_acl_role

Terraform Configuration Files

resource "consul_acl_role" "team_role" {
  name        ="my-team"
  description = "my-team role"
  policies    = ["my-policy"]
}

Expected Behavior

The my-team role is created / updated with the my-policy policy attached.

Actual Behavior

│ Error: failed to update role 'fd06e56c-1657-8888-fe9f-52221febfab1': Unexpected response code: 500 (Failed to apply role upsert request: failed acl policy lookup: index error: UUID must be 36 characters)

The Consul API for /acl/role expects a list of policies to be applied to the role and such policies can be referenced either by "ID" and/or "Name" via a PolicyLink array.
The Terraform provider however assumes that policies are to be passed only by ID: https://github.com/hashicorp/terraform-provider-consul/blob/v2.17.0/consul/resource_consul_acl_role.go#L195

[remilapeyre]

Hi @the-nando, thanks for reporting this issue. Indeed it was expected to use policies IDs in this parameter. I don't think we can accept both name and IDs in this parameter in a backward compatible manner so I will add a validation and better documentation so that an detailled error can be returned early to the user, and work to support both names and IDs in a later major version of the provider.

[const-tmp]

In Consul API policies by name are documented. Provider documentation says nothing about policy IDs. It looks very confusing.

[jorgemarey]

Hi @remilapeyre, on the ID vs name case, I guess, at the same time you added that the policy should be a valid UUID, maybe we can change that and provide the value as an ID if it's a valid UUID or as a name otherwise. I guess this won't break for anyone, as it would be backwards compatible. Everyone right now should be providing valid UUIDs. So this would only open the scope to set the value as name if it's not a valid UUID. What do you think?

I'm open to making a PR for this if you think it's valid.

[remilapeyre]

Hello @the-nando, @jorgemarey, I have started to look into this issue in more depth in order to support both using an ID or a name.

The solution proposed by @jorgemarey should work and I think it may be a good short-term solution.

I have also started working a release for the provider that should address this and other long-standing issues: https://registry.terraform.io/providers/remilapeyre/consul-next/latest/docs/resources/acl_token.

I am still working on it but you should be able to use either an ID or the name with it:

resource "consul_acl_policy" "test" {
  name  = "test"
  rules = <<-RULE
    node_prefix "" {
      policy = "read"
    }
  RULE
}

resource "consul_acl_policy" "test2" {
  name  = "test2"
  rules = <<-RULE
    node_prefix "" {
      policy = "read"
    }
  RULE
}

resource "consul_acl_token" "test" {
  policies = [
    {
      id = consul_acl_policy.test.id
    },
    {
      name = consul_acl_policy.test2.name
    }
  ]
}

EDIT: if you test here I am very interested by your feedback either here or on the other repo.

[the-nando]

@remilapeyre would it be an option to automatically derive whether the passed value in the policies list is a UUID or a name by parsing it? Something along the lines of:

import "github.com/google/uuid"

func IsValidUUID(u string) bool {
    _, err := uuid.Parse(u)
    return err == nil
 }

What do you think? It doesn't account for the case of someone naming his policies with UUIDs, which is a corner case I'd say.