From 8d6e4c917876e397e87134f74bb97046d37ab905 Mon Sep 17 00:00:00 2001 From: Jay Date: Thu, 3 Aug 2023 08:07:22 -0400 Subject: [PATCH] ccl/sqlproxyccl: avoid tenant lookups if we know the type of connection Previously, we were performing a tenant lookup call before checking on the type of connection. This can be unnecessary (e.g. doing a lookup call for the private endpoints ACL, even if we knew that the connection was a public one). This commit addresses that. Release note: None Epic: none --- pkg/ccl/sqlproxyccl/acl/cidr_ranges.go | 10 +++++----- pkg/ccl/sqlproxyccl/acl/private_endpoints.go | 10 +++++----- pkg/ccl/sqlproxyccl/acl/private_endpoints_test.go | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/ccl/sqlproxyccl/acl/cidr_ranges.go b/pkg/ccl/sqlproxyccl/acl/cidr_ranges.go index 69c8fe573e80..073172486f5b 100644 --- a/pkg/ccl/sqlproxyccl/acl/cidr_ranges.go +++ b/pkg/ccl/sqlproxyccl/acl/cidr_ranges.go @@ -28,16 +28,16 @@ var _ AccessController = &CIDRRanges{} // CheckConnection implements the AccessController interface. func (p *CIDRRanges) CheckConnection(ctx context.Context, conn ConnectionTags) error { - tenantObj, err := p.LookupTenantFn(ctx, conn.TenantID) - if err != nil { - return err - } - // Private connections. This ACL is only responsible for public CIDR ranges. if conn.EndpointID != "" { return nil } + tenantObj, err := p.LookupTenantFn(ctx, conn.TenantID) + if err != nil { + return err + } + // Cluster allows public connections, so we'll check allowed CIDR ranges. if tenantObj.AllowPublicConn() { ip := net.ParseIP(conn.IP) diff --git a/pkg/ccl/sqlproxyccl/acl/private_endpoints.go b/pkg/ccl/sqlproxyccl/acl/private_endpoints.go index 7e4e1fe55b0a..8665e26544cd 100644 --- a/pkg/ccl/sqlproxyccl/acl/private_endpoints.go +++ b/pkg/ccl/sqlproxyccl/acl/private_endpoints.go @@ -37,16 +37,16 @@ var _ AccessController = &PrivateEndpoints{} // CheckConnection implements the AccessController interface. func (p *PrivateEndpoints) CheckConnection(ctx context.Context, conn ConnectionTags) error { - tenantObj, err := p.LookupTenantFn(ctx, conn.TenantID) - if err != nil { - return err - } - // Public connections. This ACL is only responsible for private endpoints. if conn.EndpointID == "" { return nil } + tenantObj, err := p.LookupTenantFn(ctx, conn.TenantID) + if err != nil { + return err + } + // Cluster allows private connections, so we'll check allowed endpoints. if tenantObj.AllowPrivateConn() { for _, endpoints := range tenantObj.AllowedPrivateEndpoints { diff --git a/pkg/ccl/sqlproxyccl/acl/private_endpoints_test.go b/pkg/ccl/sqlproxyccl/acl/private_endpoints_test.go index d986b558a13b..888528b3ecd2 100644 --- a/pkg/ccl/sqlproxyccl/acl/private_endpoints_test.go +++ b/pkg/ccl/sqlproxyccl/acl/private_endpoints_test.go @@ -41,7 +41,7 @@ func TestPrivateEndpoints(t *testing.T) { return nil, errors.New("foo") }, } - err := p.CheckConnection(ctx, makeConn("")) + err := p.CheckConnection(ctx, makeConn("foo")) require.EqualError(t, err, "foo") })