Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
58379: pgwire: new env var `COCKROACH_ALWAYS_LOG_AUTHN_EVENTS` r=bdarnell a=knz

Requested by @logston . Intended for back-port to v20.2.

This commit adds support for a new env var
`COCKROACH_ALWAYS_LOG_AUTHN_EVENTS`. When set to a true-like value, it
overrides the `server.auth_log.sql_sessions.enabled` cluster setting
and unconditionally enables logging of authentication events.

This is a temporary stop-gap so that it becomes possible to log
authentication events in SQL pods for multi-tenant deployments. This
stop-gap can be removed when multi-tenant CockroachDB learns to
support cluster settings.

Release note: None

58481: roachtest: add expected failure for activerecord for 21.1 r=otan a=rafiss

This test was overriden to pass on 20.2, but the override is no longer
needed for 21.1. Until the override is removed in the activerecord
adapter, this test is expected to fail on 21.1

Release note: none

58485: roachtest: ignore flaky pgjdbc tests r=otan a=rafiss

Release note: none

fixes #57603
fixes #57368

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
3 people committed Jan 6, 2021
4 parents 9fa3e82 + 25f12b2 + 6151696 + 340ed3d commit 42b3e31
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
4 changes: 3 additions & 1 deletion pkg/cmd/roachtest/activerecord_blocklist.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ var activeRecordBlocklists = blocklistsForVersion{
// Please keep these lists alphabetized for easy diffing.
// After a failed run, an updated version of this blocklist should be available
// in the test log.
var activeRecordBlockList21_1 = blocklist{}
var activeRecordBlockList21_1 = blocklist{
"ActiveRecord::CockroachDB::UnloggedTablesTest#test_gracefully_handles_temporary_tables": "modified to pass on 20.2",
}

var activeRecordBlockList20_2 = blocklist{}

Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/roachtest/pgjdbc_blocklist.go
Original file line number Diff line number Diff line change
Expand Up @@ -4426,4 +4426,7 @@ var pgjdbcIgnoreList19_2 = blocklist{
"org.postgresql.test.jdbc2.BatchedInsertReWriteEnabledTest.testBatchWithReWrittenSpaceAfterValues[3: autoCommit=NO, binary=FORCE]": "54477",
"org.postgresql.test.jdbc2.BatchedInsertReWriteEnabledTest.testReWriteDisabledForPlainBatch[2: autoCommit=NO, binary=REGULAR]": "54477",
"org.postgresql.test.jdbc2.BatchedInsertReWriteEnabledTest.testReWriteDisabledForPlainBatch[3: autoCommit=NO, binary=FORCE]": "54477",
"org.postgresql.test.jdbc2.DatabaseEncodingTest.testBadUTF8Decode": "54477",
"org.postgresql.test.jdbc2.DatabaseEncodingTest.testTruncatedUTF8Decode": "54477",
"org.postgresql.test.jdbc2.DatabaseEncodingTest.testUTF8Decode": "54477",
}
18 changes: 12 additions & 6 deletions pkg/sql/pgwire/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/mon"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -89,10 +90,8 @@ type conn struct {

sv *settings.Values

// testingLogEnabled is used in unit tests in this package to
// force-enable auth logging without dancing around the
// asynchronicity of cluster settings.
testingLogEnabled bool
// alwaysLogAuthActivity is used force-enables logging of authn events.
alwaysLogAuthActivity bool
}

// serveConn creates a conn that will serve the netConn. It returns once the
Expand Down Expand Up @@ -147,12 +146,19 @@ func (s *Server) serveConn(
}

c := newConn(netConn, sArgs, &s.metrics, &s.execCfg.Settings.SV)
c.testingLogEnabled = atomic.LoadInt32(&s.testingLogEnabled) > 0
c.alwaysLogAuthActivity = alwaysLogAuthActivity || atomic.LoadInt32(&s.testingLogEnabled) > 0

// Do the reading of commands from the network.
c.serveImpl(ctx, s.IsDraining, s.SQLServer, reserved, authOpt)
}

// alwaysLogAuthActivity makes it possible to unconditionally enable
// authentication logging when cluster settings do not work reliably,
// e.g. in multi-tenant setups in v20.2. This override mechanism
// can be removed after all of CC is moved to use v21.1 or a version
// which supports cluster settings.
var alwaysLogAuthActivity = envutil.EnvOrDefaultBool("COCKROACH_ALWAYS_LOG_AUTHN_EVENTS", false)

func newConn(
netConn net.Conn, sArgs sql.SessionArgs, metrics *ServerMetrics, sv *settings.Values,
) *conn {
Expand Down Expand Up @@ -187,7 +193,7 @@ func (c *conn) GetErr() error {
}

func (c *conn) authLogEnabled() bool {
return c.testingLogEnabled || logSessionAuth.Get(c.sv)
return c.alwaysLogAuthActivity || logSessionAuth.Get(c.sv)
}

// serveImpl continuously reads from the network connection and pushes execution
Expand Down

0 comments on commit 42b3e31

Please sign in to comment.