acl-controller: Fix creation of namespaces in a partition in 1.12

[pglass]

Changes proposed in this PR:

When using Consul 1.12, the controller always creates namespaces in the default partition, even when running the controller in the default partition. This seems to be because the partition is passed as a field, rather than as query parameters.

How I've tested this PR:

I added a unit test, which I've run with 1.11.5+ent and 1.12.0+ent (go test -count=1 ./controller/ --tags=enterprise -- -enterprise)

I also manually ran the following script with Consul 1.11.5+ent and 1.12.0+ent:

Expand for script

package main

import (
	"context"
	"log"

	"github.com/hashicorp/consul/api"
)

func main() {
	cfg := api.DefaultConfig()
	client, err := api.NewClient(cfg)

	part, _, err := client.Partitions().Create(context.Background(), &api.Partition{
		Name: "part-1",
	}, nil)
	if err != nil {
		log.Fatal(err)
	}
	log.Printf("created partition %+v", part)

	{
		ns, _, err := client.Namespaces().Create(&api.Namespace{
			Name: "ns-write-options",
		}, &api.WriteOptions{Partition: part.Name})
		if err != nil {
			log.Fatal(err)
		}
		log.Printf("created namespace ns=%v", ns)
	}
	{
		ns, _, err := client.Namespaces().Create(&api.Namespace{
			Partition: part.Name,
			Name:      "ns-partition-field",
		}, nil)
		if err != nil {
			log.Fatal(err)
		}
		log.Printf("created namespace ns=%v", ns)
	}
}

Explanation: This scripts tries to creates two namespaces in partition part-1 by passing the partition as either a field in the namespace object, or as a query parameter. With Consul 1.11.5+ent, it creates both namespaces in part-1, but with Consul 1.12.5+ent it creates only the ns-write-options namespace in part-1.

With Consul 1.12.0+ent,

$ go run .
2022/04/21 19:22:29 created partition &{Name:part-1 Description: DeletedAt:<nil> CreateIndex:20 ModifyIndex:20}
2022/04/21 19:22:29 created namespace ns=&{ns-write-options  <nil> map[] <nil> part-1 21 21}
2022/04/21 19:22:29 created namespace ns=&{ns-partition-field  <nil> map[] <nil> default 23 23}
$ consul-ent namespace list -partition part-1
default:
   Partition:   part-1
ns-write-options:
   Partition:   part-1

With Consul 1.11.5+ent,

$ go run .
2022/04/21 19:22:09 created partition &{Name:part-1 Description: DeletedAt:<nil> CreateIndex:20 ModifyIndex:20}
2022/04/21 19:22:09 created namespace ns=&{ns-write-options  <nil> map[] <nil> part-1 21 21}
2022/04/21 19:22:09 created namespace ns=&{ns-partition-field  <nil> map[] <nil> part-1 23 23}
$ consul-ent namespace list -partition part-1
default:
   Partition:   part-1
ns-partition-field:
   Partition:   part-1
ns-write-options:
   Partition:   part-1

How I expect reviewers to test this PR:

👀

Checklist:

• Tests added
• CHANGELOG entry added

[erichaberkorn]

Is this required?

[pglass]

Yes, though it's because I recently added t.Parallel() to tests in this file (which reduced the test time for this file from ~60s to ~10s on my laptop).

The reason is this gotcha with how Golang closures capture variables (e.g. https://blog.cloudflare.com/a-go-gotcha-when-closures-and-goroutines-collide/). In some cases these tests modify stuff on c, and that's not okay when these run in parallel since they actually share a reference to c when they run.

[erichaberkorn]

Makes sense. What do you think about adding a comment at least the first time?

[pglass]

Yep. There could be other issues with enabling those tests to run in parallel. I'm monitoring it, and if it causes more instability I can revert back to no parallelism there.

[lkysow]

I'd vote for no parallelism and just eat the 60s because guaranteed someone will miss this.