From cbfa63e9916557a4e3f6f9483fc8793a6847ce53 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 25 Mar 2024 14:21:17 -0400 Subject: [PATCH 1/2] namespace/node_pool: forward RPCs cross-region if ACLs are enabled 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 00000000000..e4df9136fba --- /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 ae557cc6ee7..a1929614c9b 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 { + // 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 { + // 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 ac71ba72102..d8a9dc54d11 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 { + // 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 { + // 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 } From ce7c020a8671e59fc4d63e0357a7ebe5a5a29495 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 25 Mar 2024 14:58:23 -0400 Subject: [PATCH 2/2] fix test when region is empty --- nomad/namespace_endpoint.go | 4 ++-- nomad/node_pool_endpoint.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nomad/namespace_endpoint.go b/nomad/namespace_endpoint.go index a1929614c9b..4a6b908ac95 100644 --- a/nomad/namespace_endpoint.go +++ b/nomad/namespace_endpoint.go @@ -30,7 +30,7 @@ func (n *Namespace) UpsertNamespaces(args *structs.NamespaceUpsertRequest, reply *structs.GenericResponse) error { authErr := n.srv.Authenticate(n.ctx, args) - if n.srv.config.ACLEnabled { + 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 @@ -81,7 +81,7 @@ 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) - if n.srv.config.ACLEnabled { + 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 diff --git a/nomad/node_pool_endpoint.go b/nomad/node_pool_endpoint.go index d8a9dc54d11..56e55697120 100644 --- a/nomad/node_pool_endpoint.go +++ b/nomad/node_pool_endpoint.go @@ -170,7 +170,7 @@ 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) - if n.srv.config.ACLEnabled { + 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 @@ -235,7 +235,7 @@ 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) - if n.srv.config.ACLEnabled { + 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