From cd1c80c23dc33c39fa68f36154a88a0c6cbf556f Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 17 Oct 2022 13:57:48 -0400 Subject: [PATCH] acl: make version check for one-time tokens specific to region One-time tokens are not replicated between regions, so we don't want to enforce that the version check across all of serf, just members in the same region. Adds a new helper function that specifically calls out the regional filter. --- .changelog/14910.txt | 3 +++ nomad/acl_endpoint.go | 6 +++--- nomad/util.go | 24 +++++++++++++++++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 .changelog/14910.txt diff --git a/.changelog/14910.txt b/.changelog/14910.txt new file mode 100644 index 000000000000..bccb174388eb --- /dev/null +++ b/.changelog/14910.txt @@ -0,0 +1,3 @@ +```release-note:bug +acl: Fixed a bug where Nomad version checking for one-time tokens was enforced across regions +``` diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index 46aaf8d3ef18..efd8b0ffcc31 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -944,7 +944,7 @@ func (a *ACL) UpsertOneTimeToken(args *structs.OneTimeTokenUpsertRequest, reply defer metrics.MeasureSince( []string{"nomad", "acl", "upsert_one_time_token"}, time.Now()) - if !ServersMeetMinimumVersion(a.srv.Members(), minOneTimeAuthenticationTokenVersion, false) { + if !RegionServersMeetMinimumVersion(a.srv.Members(), a.srv.Region(), minOneTimeAuthenticationTokenVersion, false) { return fmt.Errorf("All servers should be running version %v or later to use one-time authentication tokens", minAutopilotVersion) } @@ -996,7 +996,7 @@ func (a *ACL) ExchangeOneTimeToken(args *structs.OneTimeTokenExchangeRequest, re defer metrics.MeasureSince( []string{"nomad", "acl", "exchange_one_time_token"}, time.Now()) - if !ServersMeetMinimumVersion(a.srv.Members(), minOneTimeAuthenticationTokenVersion, false) { + if !RegionServersMeetMinimumVersion(a.srv.Members(), a.srv.Region(), minOneTimeAuthenticationTokenVersion, false) { return fmt.Errorf("All servers should be running version %v or later to use one-time authentication tokens", minAutopilotVersion) } @@ -1053,7 +1053,7 @@ func (a *ACL) ExpireOneTimeTokens(args *structs.OneTimeTokenExpireRequest, reply defer metrics.MeasureSince( []string{"nomad", "acl", "expire_one_time_tokens"}, time.Now()) - if !ServersMeetMinimumVersion(a.srv.Members(), minOneTimeAuthenticationTokenVersion, false) { + if !RegionServersMeetMinimumVersion(a.srv.Members(), a.srv.Region(), minOneTimeAuthenticationTokenVersion, false) { return fmt.Errorf("All servers should be running version %v or later to use one-time authentication tokens", minAutopilotVersion) } diff --git a/nomad/util.go b/nomad/util.go index 5ea347f2b6ba..4eeb00729f3c 100644 --- a/nomad/util.go +++ b/nomad/util.go @@ -148,7 +148,29 @@ func isNomadServer(m serf.Member) (bool, *serverParts) { // for the failed servers should be verified. func ServersMeetMinimumVersion(members []serf.Member, minVersion *version.Version, checkFailedServers bool) bool { for _, member := range members { - if valid, parts := isNomadServer(member); valid && (parts.Status == serf.StatusAlive || (checkFailedServers && parts.Status == serf.StatusFailed)) { + valid, parts := isNomadServer(member) + if valid && (parts.Status == serf.StatusAlive || (checkFailedServers && parts.Status == serf.StatusFailed)) { + // Check if the versions match - version.LessThan will return true for + // 0.8.0-rc1 < 0.8.0, so we want to ignore the metadata + versionsMatch := slicesMatch(minVersion.Segments(), parts.Build.Segments()) + if parts.Build.LessThan(minVersion) && !versionsMatch { + return false + } + } + } + + return true +} + +// RegionServersMeetMinimumVersion returns whether the Nomad servers in this +// region are at least on the given Nomad version. The checkFailedServers +// parameter specifies whether version for the failed servers should be +// verified. +func RegionServersMeetMinimumVersion(members []serf.Member, region string, minVersion *version.Version, checkFailedServers bool) bool { + for _, member := range members { + valid, parts := isNomadServer(member) + if valid && parts.Region == region && + (parts.Status == serf.StatusAlive || (checkFailedServers && parts.Status == serf.StatusFailed)) { // Check if the versions match - version.LessThan will return true for // 0.8.0-rc1 < 0.8.0, so we want to ignore the metadata versionsMatch := slicesMatch(minVersion.Segments(), parts.Build.Segments())