Remove artificial ACLTokenMaxTTL limit for configuring acl token expi…

…ry (#17066)

* Remove artificial ACLTokenMaxTTL limit for configuring acl token expiry

* Add changelog

* Remove test on default MaxTokenTTL

* Change to imperitive tense for changelog entry

.changelog/17066.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+command: Allow creating ACL Token TTL with greater than 24 hours with the -expires-ttl flag.
+```

agent/consul/acl_endpoint_test.go

@@ -3261,21 +3261,6 @@ func TestACLEndpoint_AuthMethodSet(t *testing.T) {
 		err := aclEp.AuthMethodSet(&req, &resp)
 		testutil.RequireErrorContains(t, err, "MaxTokenTTL 1ms cannot be less than")
 	})
-
-	t.Run("Create with MaxTokenTTL too big", func(t *testing.T) {
-		reqMethod := newAuthMethod("test")
-		reqMethod.MaxTokenTTL = 25 * time.Hour
-
-		req := structs.ACLAuthMethodSetRequest{
-			Datacenter:   "dc1",
-			AuthMethod:   reqMethod,
-			WriteRequest: structs.WriteRequest{Token: TestDefaultInitialManagementToken},
-		}
-		resp := structs.ACLAuthMethod{}
-
-		err := aclEp.AuthMethodSet(&req, &resp)
-		testutil.RequireErrorContains(t, err, "MaxTokenTTL 25h0m0s cannot be more than")
-	})
 }
 
 func TestACLEndpoint_AuthMethodDelete(t *testing.T) {

agent/consul/config.go

@@ -238,20 +238,20 @@ type Config struct {
 	AutoConfigAuthzAllowReuse      bool
 
 	// TombstoneTTL is used to control how long KV tombstones are retained.
-	// This provides a window of time where the X-Consul-Index is monotonic.
+	// This provides a window of time when the X-Consul-Index is monotonic.
 	// Outside this window, the index may not be monotonic. This is a result
-	// of a few trade offs:
+	// of a few trade-offs:
 	// 1) The index is defined by the data view and not globally. This is a
 	// performance optimization that prevents any write from incrementing the
 	// index for all data views.
 	// 2) Tombstones are not kept indefinitely, since otherwise storage required
 	// is also monotonic. This prevents deletes from reducing the disk space
 	// used.
 	// In theory, neither of these are intrinsic limitations, however for the
-	// purposes of building a practical system, they are reasonable trade offs.
+	// purposes of building a practical system, they are reasonable trade-offs.
 	//
 	// It is also possible to set this to an incredibly long time, thereby
-	// simulating infinite retention. This is not recommended however.
+	// simulating infinite retention. This is not recommended, however.
 	//
 	TombstoneTTL time.Duration
 
@@ -524,11 +524,13 @@ func DefaultConfig() *Config {
 		TombstoneTTLGranularity:              30 * time.Second,
 		SessionTTLMin:                        10 * time.Second,
 		ACLTokenMinExpirationTTL:             1 * time.Minute,
-		ACLTokenMaxExpirationTTL:             24 * time.Hour,
+		// Duration is stored as an int64. Setting the default max
+		// to the max possible duration (approx 290 years).
+		ACLTokenMaxExpirationTTL: 1<<63 - 1,
 
 		// These are tuned to provide a total throughput of 128 updates
-		// per second. If you update these, you should update the client-
-		// side SyncCoordinateRateTarget parameter accordingly.
+		// per second. If you update these, you should update the client-side
+		// SyncCoordinateRateTarget parameter accordingly.
 		CoordinateUpdatePeriod:     5 * time.Second,
 		CoordinateUpdateBatchSize:  128,
 		CoordinateUpdateMaxBatches: 5,
@@ -560,7 +562,7 @@ func DefaultConfig() *Config {
 			},
 		},
 
-		// Stay under the 10 second aggregation interval of
+		// Stay under the 10-second aggregation interval of
 		// go-metrics. This ensures we always report the
 		// usage metrics in each cycle.
 		MetricsReportingInterval: 9 * time.Second,

command/acl/token/create/token_create_test.go

@@ -7,12 +7,14 @@ import (
 	"encoding/json"
 	"strings"
 	"testing"
+	"time"
+
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/testrpc"
-	"github.com/mitchellh/cli"
-	"github.com/stretchr/testify/require"
 )
 
 func TestTokenCreateCommand_noTabs(t *testing.T) {
@@ -119,6 +121,34 @@ func TestTokenCreateCommand_Pretty(t *testing.T) {
 		require.Equal(t, "3d852bb8-5153-4388-a3ca-8ca78661889f", token.AccessorID)
 		require.Equal(t, "3a69a8d8-c4d4-485d-9b19-b5b61648ea0c", token.SecretID)
 	})
+
+	// create with an expires-ttl (<24h)
+	t.Run("expires-ttl_short", func(t *testing.T) {
+		token := run(t, []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-policy-name=" + policy.Name,
+			"-description=test token",
+			"-expires-ttl=1h",
+		})
+
+		// check diff between creation and expires time since we
+		// always set the token.ExpirationTTL value to 0 at the moment
+		require.Equal(t, time.Hour, token.ExpirationTime.Sub(token.CreateTime))
+	})
+
+	// create with an expires-ttl long (>24h)
+	t.Run("expires-ttl_long", func(t *testing.T) {
+		token := run(t, []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-policy-name=" + policy.Name,
+			"-description=test token",
+			"-expires-ttl=8760h",
+		})
+
+		require.Equal(t, 8760*time.Hour, token.ExpirationTime.Sub(token.CreateTime))
+	})
 }
 
 func TestTokenCreateCommand_JSON(t *testing.T) {