From 2fde4a0c93aee9d49322355df887c6ef28508ab7 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Tue, 26 Mar 2024 10:39:37 -0400
Subject: [PATCH] namespace/node pool: forward RPCs cross-region if ACLs aren't
 enabled (#20220)

Although it's not recommended, it's possible to federate regions without ACLs
enabled. In this case, ACL-related objects such as namespaces and node pools can
be written independently in each region and won't be replicated. If you use
commands like `namespace apply` or `node pool delete`, the RPC is supposed to be
forwarded to the authoritative region. But when ACLs are disabled, there is no
authoritative region and so the RPC will always be applied to the local region
even if the `-region` flag is passed.

Remove the change to the RPC region for the namespace and node pool write RPC
whenver ACLs are disabled, so that forwarding works.

Fixes: https://github.com/hashicorp/nomad/issues/20197
Ref: https://github.com/hashicorp/nomad/issues/20128
---
 .changelog/20220.txt        |  3 +++
 nomad/namespace_endpoint.go | 12 ++++++++++--
 nomad/node_pool_endpoint.go | 12 ++++++++++--
 3 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 .changelog/20220.txt

diff --git a/.changelog/20220.txt b/.changelog/20220.txt
new file mode 100644
index 000000000000..e4df9136fba4
--- /dev/null
+++ b/.changelog/20220.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+namespace/node pool: Fixed a bug where the `-region` flag would not be respected for namespace and node pool updates if ACLs were disabled
+```
diff --git a/nomad/namespace_endpoint.go b/nomad/namespace_endpoint.go
index ae557cc6ee70..4a6b908ac95e 100644
--- a/nomad/namespace_endpoint.go
+++ b/nomad/namespace_endpoint.go
@@ -30,7 +30,11 @@ func (n *Namespace) UpsertNamespaces(args *structs.NamespaceUpsertRequest,
 	reply *structs.GenericResponse) error {
 
 	authErr := n.srv.Authenticate(n.ctx, args)
-	args.Region = n.srv.config.AuthoritativeRegion
+	if n.srv.config.ACLEnabled || args.Region == "" {
+		// only forward to the authoritative region if ACLs are enabled,
+		// otherwise we silently write to the local region
+		args.Region = n.srv.config.AuthoritativeRegion
+	}
 	if done, err := n.srv.forward("Namespace.UpsertNamespaces", args, args, reply); done {
 		return err
 	}
@@ -77,7 +81,11 @@ func (n *Namespace) UpsertNamespaces(args *structs.NamespaceUpsertRequest,
 func (n *Namespace) DeleteNamespaces(args *structs.NamespaceDeleteRequest, reply *structs.GenericResponse) error {
 
 	authErr := n.srv.Authenticate(n.ctx, args)
-	args.Region = n.srv.config.AuthoritativeRegion
+	if n.srv.config.ACLEnabled || args.Region == "" {
+		// only forward to the authoritative region if ACLs are enabled,
+		// otherwise we silently write to the local region
+		args.Region = n.srv.config.AuthoritativeRegion
+	}
 	if done, err := n.srv.forward("Namespace.DeleteNamespaces", args, args, reply); done {
 		return err
 	}
diff --git a/nomad/node_pool_endpoint.go b/nomad/node_pool_endpoint.go
index ac71ba721023..56e55697120a 100644
--- a/nomad/node_pool_endpoint.go
+++ b/nomad/node_pool_endpoint.go
@@ -170,7 +170,11 @@ func (n *NodePool) GetNodePool(args *structs.NodePoolSpecificRequest, reply *str
 // cannot be updated.
 func (n *NodePool) UpsertNodePools(args *structs.NodePoolUpsertRequest, reply *structs.GenericResponse) error {
 	authErr := n.srv.Authenticate(n.ctx, args)
-	args.Region = n.srv.config.AuthoritativeRegion
+	if n.srv.config.ACLEnabled || args.Region == "" {
+		// only forward to the authoritative region if ACLs are enabled,
+		// otherwise we silently write to the local region
+		args.Region = n.srv.config.AuthoritativeRegion
+	}
 	if done, err := n.srv.forward("NodePool.UpsertNodePools", args, args, reply); done {
 		return err
 	}
@@ -231,7 +235,11 @@ func (n *NodePool) UpsertNodePools(args *structs.NodePoolUpsertRequest, reply *s
 // deleted.
 func (n *NodePool) DeleteNodePools(args *structs.NodePoolDeleteRequest, reply *structs.GenericResponse) error {
 	authErr := n.srv.Authenticate(n.ctx, args)
-	args.Region = n.srv.config.AuthoritativeRegion
+	if n.srv.config.ACLEnabled || args.Region == "" {
+		// only forward to the authoritative region if ACLs are enabled,
+		// otherwise we silently write to the local region
+		args.Region = n.srv.config.AuthoritativeRegion
+	}
 	if done, err := n.srv.forward("NodePool.DeleteNodePools", args, args, reply); done {
 		return err
 	}