Only synthesize anonymous token in primary DC #17231
Conversation
|
I think this is the correct fix. In WAN federation, the primary dc manages all ACLs, so we should not be creating tokens in the secondary DC (especially when the primary DC has not yet completed bootstrapping the ACL system, and is not ready to be the ACL manager). |
pglass
force-pushed
the
pglass/synth-anon-token-wan-fed-fix
branch
from
6e343a3 to
ab66027
Compare
pglass
added
backport/1.15
pglass
changed the title
pglass
force-pushed
the
pglass/synth-anon-token-wan-fed-fix
branch
from
9b80620 to
d2f4fd7
Compare
There was a problem hiding this comment.
I ended up adding an integration test for this.
I wanted to test the scenario from the github issue, but I thought that scenario was perhaps slightly too complex to put in the api/ package, where we are really just testing the api client. I did try incorporating a test into the agent/consul/ package where we have some other acl replication tests, but I had some issues reliably reproducing the issue there (and I opted to not spend more cycles on it).
I feel like this is an important scenario for WAN fed that makes sense for an integration test, and the integration tests are pretty straightforward to write 🙂
| func retryFunc[T any](t *testing.T, f func() (T, error)) T { | ||
| var result T | ||
| retry.RunWith(retryFuncTimer, t, func(r *retry.R) { | ||
| val, err := f() | ||
| require.NoError(r, err) | ||
| result = val | ||
| }) | ||
| return result | ||
| } |
There was a problem hiding this comment.
I added this to try to declutter the retry loops in the test case a little. One gotcha is that the retry.R is not available to the provided function (f), so the caller shouldn't check assertions in f.
| replicationPolicy := retryFunc(t, func() (*api.ACLPolicy, error) { | ||
| t.Logf("Create the replication policy") | ||
| p, _, err := primary.GetClient().ACL().PolicyCreate( | ||
| &api.ACLPolicy{ | ||
| Name: "consul-server-replication", | ||
| Rules: replicationPolicyRules, | ||
| }, | ||
| nil, | ||
| ) | ||
| return p, err | ||
| }) |
There was a problem hiding this comment.
I saw there was a helper to create an agent token. I could move this to a helper alongside that?
| @@ -1,7 +1,7 @@ | |||
| // Copyright (c) HashiCorp, Inc. | |||
| // SPDX-License-Identifier: MPL-2.0 | |||
|
|
|||
| package peering | |||
| package wanfed | |||
There was a problem hiding this comment.
I noticed another package is called peering so I renamed this to match the directory name.
* Only synthesize anonymous token in primary DC * Add integration test for wan fed issue
Description
Prior to #16200, the anonymous token was only inserted in the primary datacenter. This is a potential fix for WAN fed issues seen in: #16620.
PR Checklist