Skip to content

Commit

Permalink
acl: make version check for one-time tokens specific to region
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgross committed Oct 17, 2022
1 parent f6838f6 commit cd1c80c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/14910.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
acl: Fixed a bug where Nomad version checking for one-time tokens was enforced across regions
```
6 changes: 3 additions & 3 deletions nomad/acl_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down
24 changes: 23 additions & 1 deletion nomad/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit cd1c80c

Please sign in to comment.