feat: set default connection limit to unlimited (-1) instead of 1 (#2234)

This avoids the confusion behavior of a session terminating as soon
as a single connection using that session is closed.

CHANGELOG.md

@@ -20,6 +20,13 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
 * The plugin execution_dir configuration parameter is now respected by kms plugins too
   [PR](https://github.com/hashicorp/boundary/pull/2239).
 
+### Deprecations/Changes
+
+* sessions: The default connect limit for new sessions changed from 1 to unlimited (-1).
+  Specific connection limits is an advanced feature of Boundary and this setting is
+  more friendly for new users.
+  ([PR](https://github.com/hashicorp/boundary/pull/2234))
+
 ## 0.9.0 (2022/06/20)
 
 ### Known Issues

internal/daemon/controller/handlers/targets/tcp/target_service_test.go

@@ -133,7 +133,7 @@ func TestGet(t *testing.T) {
 		HostSourceIds:          []string{hs[0].GetPublicId(), hs[1].GetPublicId()},
 		Attrs:                  &pb.Target_TcpTargetAttributes{},
 		SessionMaxSeconds:      wrapperspb.UInt32(28800),
-		SessionConnectionLimit: wrapperspb.Int32(1),
+		SessionConnectionLimit: wrapperspb.Int32(-1),
 		AuthorizedActions:      testAuthorizedActions,
 	}
 	for _, ihs := range hs {
@@ -227,7 +227,7 @@ func TestList(t *testing.T) {
 			Type:                   tcp.Subtype.String(),
 			Attrs:                  &pb.Target_TcpTargetAttributes{},
 			SessionMaxSeconds:      wrapperspb.UInt32(28800),
-			SessionConnectionLimit: wrapperspb.Int32(1),
+			SessionConnectionLimit: wrapperspb.Int32(-1),
 			AuthorizedActions:      testAuthorizedActions,
 		})
 		totalTars = append(totalTars, wantTars[i])
@@ -243,7 +243,7 @@ func TestList(t *testing.T) {
 			Type:                   tcp.Subtype.String(),
 			Attrs:                  &pb.Target_TcpTargetAttributes{},
 			SessionMaxSeconds:      wrapperspb.UInt32(28800),
-			SessionConnectionLimit: wrapperspb.Int32(1),
+			SessionConnectionLimit: wrapperspb.Int32(-1),
 			AuthorizedActions:      testAuthorizedActions,
 		})
 	}
@@ -466,7 +466,7 @@ func TestCreate(t *testing.T) {
 						},
 					},
 					SessionMaxSeconds:      wrapperspb.UInt32(28800),
-					SessionConnectionLimit: wrapperspb.Int32(1),
+					SessionConnectionLimit: wrapperspb.Int32(-1),
 					AuthorizedActions:      testAuthorizedActions,
 					WorkerFilter:           wrapperspb.String(`type == "bar"`),
 				},

internal/db/schema/migrations/oss/postgres/0/41_targets.up.sql

@@ -105,6 +105,7 @@ create table target_tcp (
     constraint session_max_seconds_must_be_greater_than_0
     check(session_max_seconds > 0),
   -- limit on number of session connections allowed. -1 equals no limit
+  -- The default was updated in 37/01_set_unlimited_connections_limit.up.sql.
   session_connection_limit int not null default 1
     constraint session_connection_limit_must_be_greater_than_0_or_negative_1
     check(session_connection_limit > 0 or session_connection_limit = -1),

internal/db/schema/migrations/oss/postgres/37/01_set_unlimited_connections_limit.up.sql

@@ -0,0 +1,8 @@
+begin;
+
+-- Changes the default that was set in 0/41_targets.up.sql
+alter table target_tcp
+    alter column session_connection_limit
+        set default -1;
+
+commit;

internal/db/sqltest/tests/wh/credential_dimension_views/source.sql

@@ -36,7 +36,7 @@ begin;
     'None',                     -- target_description,
     0,                          -- target_default_port_number,
     28800,                      -- target_session_max_seconds,
-    1,                          -- target_session_connection_limit,
+    -1,                         -- target_session_connection_limit,
 
     'p____bwidget',             -- project_id,
     'Big Widget Factory',       -- project_name,

internal/db/sqltest/tests/wh/host_dimension_views/source.sql

@@ -8,7 +8,7 @@ begin;
     'h_____wb__01', 'static host',         'None',                      'None',
     's___2wb-sths', 'static host set',     'Big Widget Static Set 2',   'None',
     'c___wb-sthcl', 'static host catalog', 'Big Widget Static Catalog', 'None',
-    't_________wb', 'tcp target',          'Big Widget Target',         'None', 0,              28800, 1,
+    't_________wb', 'tcp target',          'Big Widget Target',         'None', 0,              28800, -1,
     'p____bwidget', 'Big Widget Factory',  'None',
     'o_____widget', 'Widget Inc',          'None'
   )::whx_host_dimension_source)
@@ -22,7 +22,7 @@ begin;
     'h_____wb__01-plgh',  'plugin host',         'None',                      'None',
     's___2wb-plghs',      'plugin host set',     'Big Widget Plugin Set 2',   'None',
     'c___wb-plghcl',      'plugin host catalog', 'Big Widget Plugin Catalog', 'None',
-    't_________wb',       'tcp target',          'Big Widget Target',         'None', 0,              28800, 1,
+    't_________wb',       'tcp target',          'Big Widget Target',         'None', 0,              28800, -1,
     'p____bwidget',       'Big Widget Factory',  'None',
     'o_____widget',       'Widget Inc',          'None'
     )::whx_host_dimension_source)

internal/session/repository_session_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/boundary/internal/auth/password"
 	"github.com/hashicorp/boundary/internal/authtoken"
 	authtokenStore "github.com/hashicorp/boundary/internal/authtoken/store"
 	cred "github.com/hashicorp/boundary/internal/credential"
@@ -891,9 +892,10 @@ func TestRepository_CancelSession(t *testing.T) {
 	rw := db.New(conn)
 	wrapper := db.TestWrapper(t)
 	iamRepo := iam.TestRepo(t, conn, wrapper)
-	kms := kms.TestKms(t, conn, wrapper)
-	repo, err := NewRepository(rw, rw, kms)
-	connRepo, err := NewConnectionRepository(ctx, rw, rw, kms)
+	testKms := kms.TestKms(t, conn, wrapper)
+	repo, err := NewRepository(rw, rw, testKms)
+	require.NoError(t, err)
+	connRepo, err := NewConnectionRepository(ctx, rw, rw, testKms)
 	require.NoError(t, err)
 	setupFn := func() *Session {
 		session := TestDefaultSession(t, conn, wrapper, iamRepo)
@@ -917,7 +919,47 @@ func TestRepository_CancelSession(t *testing.T) {
 		{
 			name: "already-terminated",
 			session: func() *Session {
-				session := TestDefaultSession(t, conn, wrapper, iamRepo)
+				future := timestamppb.New(time.Now().Add(time.Hour))
+				exp := &timestamp.Timestamp{Timestamp: future}
+				org, proj := iam.TestScopes(t, iamRepo)
+
+				cats := static.TestCatalogs(t, conn, proj.PublicId, 1)
+				hosts := static.TestHosts(t, conn, cats[0].PublicId, 1)
+				sets := static.TestSets(t, conn, cats[0].PublicId, 1)
+				_ = static.TestSetMembers(t, conn, sets[0].PublicId, hosts)
+
+				// We need to set the session connection limit to 1 so that the session
+				// is terminated when the one connection is closed.
+				tcpTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, "test target", target.WithSessionConnectionLimit(1))
+
+				targetRepo, err := target.NewRepository(rw, rw, testKms)
+				require.NoError(t, err)
+				_, _, _, err = targetRepo.AddTargetHostSources(ctx, tcpTarget.GetPublicId(), tcpTarget.GetVersion(), []string{sets[0].PublicId})
+				require.NoError(t, err)
+
+				authMethod := password.TestAuthMethods(t, conn, org.PublicId, 1)[0]
+				acct := password.TestAccount(t, conn, authMethod.GetPublicId(), "name1")
+				user := iam.TestUser(t, iamRepo, org.PublicId, iam.WithAccountIds(acct.PublicId))
+
+				authTokenRepo, err := authtoken.NewRepository(rw, rw, testKms)
+				require.NoError(t, err)
+				at, err := authTokenRepo.CreateAuthToken(ctx, user, acct.GetPublicId())
+				require.NoError(t, err)
+
+				expTime := timestamppb.Now()
+				expTime.Seconds += int64(tcpTarget.GetSessionMaxSeconds())
+				composedOf := ComposedOf{
+					UserId:          user.PublicId,
+					HostId:          hosts[0].PublicId,
+					TargetId:        tcpTarget.GetPublicId(),
+					HostSetId:       sets[0].PublicId,
+					AuthTokenId:     at.PublicId,
+					ScopeId:         tcpTarget.GetScopeId(),
+					Endpoint:        "tcp://127.0.0.1:22",
+					ExpirationTime:  &timestamp.Timestamp{Timestamp: expTime},
+					ConnectionLimit: tcpTarget.GetSessionConnectionLimit(),
+				}
+				session := TestSession(t, conn, wrapper, composedOf, WithExpirationTime(exp))
 				c := TestConnection(t, conn, session.PublicId, "127.0.0.1", 22, "127.0.0.1", 2222, "127.0.0.1")
 				cw := CloseWith{
 					ConnectionId: c.PublicId,

internal/session/service_authorize_connection_test.go

@@ -5,7 +5,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/boundary/internal/auth/password"
+	"github.com/hashicorp/boundary/internal/authtoken"
+	"github.com/hashicorp/boundary/internal/host/static"
 	"github.com/hashicorp/boundary/internal/server"
+	"github.com/hashicorp/boundary/internal/target"
+	"github.com/hashicorp/boundary/internal/target/tcp"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/hashicorp/boundary/internal/db"
@@ -23,10 +28,10 @@ func TestService_AuthorizeConnection(t *testing.T) {
 	rw := db.New(conn)
 	wrapper := db.TestWrapper(t)
 	iamRepo := iam.TestRepo(t, conn, wrapper)
-	kms := kms.TestKms(t, conn, wrapper)
-	repo, err := NewRepository(rw, rw, kms)
+	testKms := kms.TestKms(t, conn, wrapper)
+	repo, err := NewRepository(rw, rw, testKms)
 	require.NoError(t, err)
-	connRepo, err := NewConnectionRepository(ctx, rw, rw, kms)
+	connRepo, err := NewConnectionRepository(ctx, rw, rw, testKms)
 	require.NoError(t, err)
 
 	var testServer string
@@ -56,7 +61,7 @@ func TestService_AuthorizeConnection(t *testing.T) {
 			name:    "valid",
 			session: testSession,
 			wantAuthzInfo: AuthzSummary{
-				ConnectionLimit:        1,
+				ConnectionLimit:        -1,
 				CurrentConnectionCount: 1,
 				ExpirationTime:         testSession.ExpirationTime,
 			},
@@ -72,7 +77,49 @@ func TestService_AuthorizeConnection(t *testing.T) {
 		{
 			name: "exceeded-connection-limit",
 			session: func() *Session {
-				session := setupFn(nil)
+				future := timestamppb.New(time.Now().Add(time.Hour))
+				exp := &timestamp.Timestamp{Timestamp: future}
+				org, proj := iam.TestScopes(t, iamRepo)
+
+				cats := static.TestCatalogs(t, conn, proj.PublicId, 1)
+				hosts := static.TestHosts(t, conn, cats[0].PublicId, 1)
+				sets := static.TestSets(t, conn, cats[0].PublicId, 1)
+				_ = static.TestSetMembers(t, conn, sets[0].PublicId, hosts)
+
+				// We need to set the session connection limit to 1 so that the session
+				// is terminated when the one connection is closed.
+				tcpTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, "test target", target.WithSessionConnectionLimit(1))
+
+				targetRepo, err := target.NewRepository(rw, rw, testKms)
+				require.NoError(t, err)
+				_, _, _, err = targetRepo.AddTargetHostSources(ctx, tcpTarget.GetPublicId(), tcpTarget.GetVersion(), []string{sets[0].PublicId})
+				require.NoError(t, err)
+
+				authMethod := password.TestAuthMethods(t, conn, org.PublicId, 1)[0]
+				acct := password.TestAccount(t, conn, authMethod.GetPublicId(), "name1")
+				user := iam.TestUser(t, iamRepo, org.PublicId, iam.WithAccountIds(acct.PublicId))
+
+				authTokenRepo, err := authtoken.NewRepository(rw, rw, testKms)
+				require.NoError(t, err)
+				at, err := authTokenRepo.CreateAuthToken(ctx, user, acct.GetPublicId())
+				require.NoError(t, err)
+
+				expTime := timestamppb.Now()
+				expTime.Seconds += int64(tcpTarget.GetSessionMaxSeconds())
+				composedOf := ComposedOf{
+					UserId:          user.PublicId,
+					HostId:          hosts[0].PublicId,
+					TargetId:        tcpTarget.GetPublicId(),
+					HostSetId:       sets[0].PublicId,
+					AuthTokenId:     at.PublicId,
+					ScopeId:         tcpTarget.GetScopeId(),
+					Endpoint:        "tcp://127.0.0.1:22",
+					ExpirationTime:  &timestamp.Timestamp{Timestamp: expTime},
+					ConnectionLimit: tcpTarget.GetSessionConnectionLimit(),
+				}
+				session := TestSession(t, conn, wrapper, composedOf, WithExpirationTime(exp))
+
+				// Create connection against the session so that any further attempts are declined
 				_ = TestConnection(t, conn, session.PublicId, "127.0.0.1", 22, "127.0.0.1", 2222, "127.0.0.1")
 				return session
 			}(),

internal/session/service_close_connections_test.go

@@ -3,12 +3,20 @@ package session
 import (
 	"context"
 	"testing"
+	"time"
 
+	"github.com/hashicorp/boundary/internal/auth/password"
+	"github.com/hashicorp/boundary/internal/authtoken"
 	"github.com/hashicorp/boundary/internal/db"
+	"github.com/hashicorp/boundary/internal/db/timestamp"
+	"github.com/hashicorp/boundary/internal/host/static"
 	"github.com/hashicorp/boundary/internal/iam"
 	"github.com/hashicorp/boundary/internal/kms"
+	"github.com/hashicorp/boundary/internal/target"
+	"github.com/hashicorp/boundary/internal/target/tcp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 func TestServiceCloseConnections(t *testing.T) {
@@ -18,9 +26,10 @@ func TestServiceCloseConnections(t *testing.T) {
 	rw := db.New(conn)
 	wrapper := db.TestWrapper(t)
 	iamRepo := iam.TestRepo(t, conn, wrapper)
-	kms := kms.TestKms(t, conn, wrapper)
-	repo, err := NewRepository(rw, rw, kms)
-	connRepo, err := NewConnectionRepository(ctx, rw, rw, kms)
+	testKms := kms.TestKms(t, conn, wrapper)
+	repo, err := NewRepository(rw, rw, testKms)
+	require.NoError(t, err)
+	connRepo, err := NewConnectionRepository(ctx, rw, rw, testKms)
 	require.NoError(t, err)
 
 	type sessionAndCloseWiths struct {
@@ -29,7 +38,47 @@ func TestServiceCloseConnections(t *testing.T) {
 	}
 
 	setupFn := func(cnt int, addtlConn int) sessionAndCloseWiths {
-		s := TestDefaultSession(t, conn, wrapper, iamRepo)
+		future := timestamppb.New(time.Now().Add(time.Hour))
+		exp := &timestamp.Timestamp{Timestamp: future}
+		org, proj := iam.TestScopes(t, iamRepo)
+
+		cats := static.TestCatalogs(t, conn, proj.PublicId, 1)
+		hosts := static.TestHosts(t, conn, cats[0].PublicId, 1)
+		sets := static.TestSets(t, conn, cats[0].PublicId, 1)
+		_ = static.TestSetMembers(t, conn, sets[0].PublicId, hosts)
+
+		// We need to set the session connection limit to 1 so that the session
+		// is terminated when the one connection is closed.
+		tcpTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, "test target", target.WithSessionConnectionLimit(1))
+
+		targetRepo, err := target.NewRepository(rw, rw, testKms)
+		require.NoError(t, err)
+		_, _, _, err = targetRepo.AddTargetHostSources(ctx, tcpTarget.GetPublicId(), tcpTarget.GetVersion(), []string{sets[0].PublicId})
+		require.NoError(t, err)
+
+		authMethod := password.TestAuthMethods(t, conn, org.PublicId, 1)[0]
+		acct := password.TestAccount(t, conn, authMethod.GetPublicId(), "name1")
+		user := iam.TestUser(t, iamRepo, org.PublicId, iam.WithAccountIds(acct.PublicId))
+
+		authTokenRepo, err := authtoken.NewRepository(rw, rw, testKms)
+		require.NoError(t, err)
+		at, err := authTokenRepo.CreateAuthToken(ctx, user, acct.GetPublicId())
+		require.NoError(t, err)
+
+		expTime := timestamppb.Now()
+		expTime.Seconds += int64(tcpTarget.GetSessionMaxSeconds())
+		composedOf := ComposedOf{
+			UserId:          user.PublicId,
+			HostId:          hosts[0].PublicId,
+			TargetId:        tcpTarget.GetPublicId(),
+			HostSetId:       sets[0].PublicId,
+			AuthTokenId:     at.PublicId,
+			ScopeId:         tcpTarget.GetScopeId(),
+			Endpoint:        "tcp://127.0.0.1:22",
+			ExpirationTime:  &timestamp.Timestamp{Timestamp: expTime},
+			ConnectionLimit: tcpTarget.GetSessionConnectionLimit(),
+		}
+		s := TestSession(t, conn, wrapper, composedOf, WithExpirationTime(exp))
 		tofu := TestTofu(t)
 		s, _, err = repo.ActivateSession(context.Background(), s.PublicId, s.Version, tofu)
 		require.NoError(t, err)

internal/target/options.go

@@ -54,7 +54,7 @@ func getDefaultOptions() options {
 		WithCredentialLibraries:    nil,
 		WithStaticCredentials:      nil,
 		WithSessionMaxSeconds:      uint32((8 * time.Hour).Seconds()),
-		WithSessionConnectionLimit: 1,
+		WithSessionConnectionLimit: -1,
 		WithPublicId:               "",
 		WithWorkerFilter:           "",
 	}

internal/target/tcp/target_test.go

@@ -55,7 +55,7 @@ func TestTarget_Create(t *testing.T) {
 					prj.PublicId,
 					target.WithName("valid-proj-scope"),
 					target.WithSessionMaxSeconds(uint32((8 * time.Hour).Seconds())),
-					target.WithSessionConnectionLimit(1),
+					target.WithSessionConnectionLimit(-1),
 				)
 				return t
 			}(),

website/content/docs/concepts/domain-model/targets.mdx

@@ -48,9 +48,9 @@ TCP targets have the following additional attributes:
 
 - `session_connection_limit` - (required)
   The cumulative number of TCP connections allowed during a session.
-  The default is 1.
   -1 means no limit.
-  The value must be greater than 0 or -1.
+  The default is -1.
+  The value must be greater than 0 or exactly -1.
 
 ## Referenced By