Merge cockroachdb#102772 cockroachdb#102785 cockroachdb#103118 cockro…

…achdb#103240 cockroachdb#103301

102772: pkg/util/log: unify explicit flush of network and file sinks r=knz a=abarganier

Ref: cockroachdb#101562

both for files *and* network sinks, such as fluent-servers.

I received some feedback that we shouldn't divide these flush operations based on sink type, and instead we should unify the flush operation to flush both (as the crash reporter already does).

The existing function to flush just the file sinks is quite widely used. Introducing flushing of network sinks begs the question, "What if this adds to the runtime of code using this explicit flush mechanism?"

Well, the existing FlushFileSinks calls fsync() [1] with F_FULLFSYNC [2]. IIUC, this means that it already is a blocking operation as the fsync() call will wait for the buffered data to be written to permanent storage (not just the disk cache). Given that, I think any caller of this function already assumes it's a blocking operation and therefore should be tolerant of that.

[1]: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fsync.2.html
[2]: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html#//apple_ref/doc/man/2/fcntl

Nonetheless, we implement a timeout mechanism for the flushing of the buffered network sinks as an added protection.

Fixes: cockroachdb#101369

102785: codeowners, roachtest: merge sql-schema, sql-sessions to sql-foundations r=celiala a=celiala

Reflecting merge sql-schema, sql-sessions to sql-foundations in:

- [x] github CODEOWNERS
- [x] pkg/cmd/roachtest owners
- [x] TEAMS.yaml

Tests should pass once new team is created via https://github.com/cockroachlabs/crl-infrastructure/pull/775

This is a task for Part 2, below.
____

### Tasks to update: GitHub Team name + GitHub Projects + Blathers

Part 1
- [x] Create GitHub T- label: https://github.com/cockroachdb/cockroach/labels/T-sql-foundations
- [ ] Create new sql-foundations GitHub team name: https://github.com/cockroachlabs/crl-infrastructure/pull/775

Part 2

- [ ] Update Blathers: https://github.com/cockroachlabs/blathers-bot/pull/123
- [ ] Update CODEOWNERS, roachtest, TEAMS.yaml:
  - [ ] master: cockroachdb#102785
  - [ ] release-23.1: cockroachdb#102924 
  - [ ] release-22.2: cockroachdb#102925
- [ ] [Manual Step] To be done at same time as merging Part 2 PRs:
  - [ ] Manually rename "SQL Schema" Project to "SQL Foundations": https://github.com/cockroachdb/cockroach/projects/47

Part 3

- [ ] Remove old GitHub team names: https://github.com/cockroachlabs/crl-infrastructure/pull/776

____


Partial work for DEVINFHD-867
Release note: None
Epic: DEVINFHD-867


103118: jwtauthccl: allow cluster SSO to pick principals from custom claims r=kpatron-cockroachlabs a=kpatron-cockroachlabs

Previously, cluster SSO (JWT authentication) required user principals to be in the subject field of the JWT, which by the JWT specification required them to be a single string. A customer has a desire have which SQL users a human is allowed to login as be specified by their IDP via a "groups" field in the JWT.

To accomadate this, a new cluster setting has been introduced (server.jwt_authentication.claim), which when populated specifies which claim of the JWT to read as containing the principal. This value can be either a string or an array of strings. The resulting principal(s) are then fed through the identity map as before to produce the set of valid SQL users that this token can login as. As before, humans can specify which SQL user they wish to login with via the username field as long as it matches one of the values the token is a valid authentication method for.

Fixes: CC-24595

Release note (enterprise change): Cluster SSO (JWT authentication) can now read SQL usernames from any JWT claims instead of requiring the subject claim to be used.

103240: ui: no need to refresh page after error r=maryliag a=maryliag

Previously, if one page crashes on DB Console, all other pages would show the same error message and the user had to force a refresh on the browser to be able to see the other pages. Now only the broken page shows the error and all the other pages load as expected. The user still needs to force a refresh on the broken page if they want to retry.

Fixes cockroachdb#97533

https://www.loom.com/share/56a6d811d9604b7abe673c1430ee605e

Release note (ui change): If a page crashed, a force refresh is no longer required to be able to see the other pages on DB Console.

103301: backupccl: disable restore data processor memory monitoring by default r=rhu713 a=rhu713

Temporarily disable memory monitoring for the restore data processor due to current logic not handling deletes correctly.

Epic: none

Co-authored-by: Alex Barganier <[email protected]>
Co-authored-by: Celia La <[email protected]>
Co-authored-by: Kyle Patron <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Rui Hu <[email protected]>

TEAMS.yaml

@@ -21,15 +21,13 @@ cockroachdb/docs:
   triage_column_id: 3971225
   aliases:
     cockroachdb/docs-infra-prs: other
-cockroachdb/sql-sessions:
+cockroachdb/sql-foundations:
   aliases:
     cockroachdb/sql-syntax-prs: other
     cockroachdb/sqlproxy-prs: other
     cockroachdb/sql-api-prs: other
-  triage_column_id: 7259065
-  label: T-sql-sessions
-cockroachdb/sql-schema:
-  triage_column_id: 8946818
+  triage_column_id: 19467489
+  label: T-sql-foundations
 cockroachdb/sql-queries:
   aliases:
     cockroachdb/sql-optimizer: other

pkg/ccl/backupccl/restore_processor_planning.go

@@ -28,7 +28,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
-	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -54,7 +53,7 @@ var memoryMonitorSSTs = settings.RegisterBoolSetting(
 	settings.TenantWritable,
 	"bulkio.restore.memory_monitor_ssts",
 	"if true, restore will limit number of simultaneously open SSTs based on available memory",
-	util.ConstantWithMetamorphicTestBool("restore-memory-monitor-ssts", true),
+	false,
 )
 
 // distRestore plans a 2 stage distSQL flow for a distributed restore. It

pkg/ccl/backupccl/utils_test.go

@@ -663,7 +663,7 @@ func requireRecoveryEvent(
 	expected eventpb.RecoveryEvent,
 ) {
 	testutils.SucceedsSoon(t, func() error {
-		log.FlushFileSinks()
+		log.Flush()
 		entries, err := log.FetchEntriesFromFiles(
 			startTime,
 			math.MaxInt64,

pkg/ccl/changefeedccl/changefeed_test.go

@@ -1796,7 +1796,7 @@ func TestChangefeedSchemaChangeNoBackfill(t *testing.T) {
 
 	cdcTest(t, testFn)
 
-	log.FlushFileSinks()
+	log.Flush()
 	entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1, regexp.MustCompile("cdc ux violation"),
 		log.WithFlattenedSensitiveData)
 	if err != nil {
@@ -2166,7 +2166,7 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 
 	cdcTestWithSystem(t, testFn, feedTestEnterpriseSinks)
 
-	log.FlushFileSinks()
+	log.Flush()
 	entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1,
 		regexp.MustCompile("cdc ux violation"), log.WithFlattenedSensitiveData)
 	if err != nil {
@@ -2352,7 +2352,7 @@ func TestChangefeedSchemaChangeAllowBackfill(t *testing.T) {
 
 	cdcTestWithSystem(t, testFn)
 
-	log.FlushFileSinks()
+	log.Flush()
 	entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1,
 		regexp.MustCompile("cdc ux violation"), log.WithFlattenedSensitiveData)
 	if err != nil {
@@ -2404,7 +2404,7 @@ func TestChangefeedSchemaChangeBackfillScope(t *testing.T) {
 	}
 
 	cdcTestWithSystem(t, testFn)
-	log.FlushFileSinks()
+	log.Flush()
 	entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1,
 		regexp.MustCompile("cdc ux violation"), log.WithFlattenedSensitiveData)
 	if err != nil {
@@ -2511,7 +2511,7 @@ func TestChangefeedAfterSchemaChangeBackfill(t *testing.T) {
 	}
 
 	cdcTest(t, testFn)
-	log.FlushFileSinks()
+	log.Flush()
 	entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1,
 		regexp.MustCompile("cdc ux violation"), log.WithFlattenedSensitiveData)
 	if err != nil {

pkg/ccl/changefeedccl/helpers_test.go

@@ -1141,7 +1141,7 @@ var cmLogRe = regexp.MustCompile(`event_log\.go`)
 func checkStructuredLogs(t *testing.T, eventType string, startTime int64) []string {
 	var matchingEntries []string
 	testutils.SucceedsSoon(t, func() error {
-		log.FlushFileSinks()
+		log.Flush()
 		entries, err := log.FetchEntriesFromFiles(startTime,
 			math.MaxInt64, 10000, cmLogRe, log.WithMarkedSensitiveData)
 		if err != nil {

pkg/ccl/changefeedccl/nemeses_test.go

@@ -47,7 +47,7 @@ func TestChangefeedNemeses(t *testing.T) {
 	// nemeses_test.go:39: pq: unimplemented: operation is
 	// unsupported in multi-tenancy mode
 	cdcTest(t, testFn, feedTestNoTenants)
-	log.FlushFileSinks()
+	log.Flush()
 	entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1,
 		regexp.MustCompile("cdc ux violation"), log.WithFlattenedSensitiveData)
 	if err != nil {

pkg/ccl/jwtauthccl/authentication_jwt.go

@@ -10,6 +10,7 @@ package jwtauthccl
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
@@ -64,6 +65,7 @@ type jwtAuthenticatorConf struct {
 	enabled  bool
 	issuers  []string
 	jwks     jwk.Set
+	claim    string
 }
 
 // reloadConfig locks mutex and then refreshes the values in conf from the cluster settings.
@@ -82,6 +84,7 @@ func (authenticator *jwtAuthenticator) reloadConfigLocked(
 		enabled:  JWTAuthEnabled.Get(&st.SV),
 		issuers:  mustParseValueOrArray(JWTAuthIssuers.Get(&st.SV)),
 		jwks:     mustParseJWKS(JWTAuthJWKS.Get(&st.SV)),
+		claim:    JWTAuthClaim.Get(&st.SV),
 	}
 
 	if !authenticator.mu.conf.enabled && conf.enabled {
@@ -147,23 +150,63 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin(
 			"token issued by %s", parsedToken.Issuer())
 	}
 
-	users, err := authenticator.mapUsername(parsedToken.Subject(), parsedToken.Issuer(), identMap)
-	if err != nil || len(users) == 0 {
+	// Extract all requested principals from the token. By default, we take it from the subject unless they specify
+	// an alternate claim to pull from.
+	var tokenPrincipals []string
+	if authenticator.mu.conf.claim == "" || authenticator.mu.conf.claim == "sub" {
+		tokenPrincipals = []string{parsedToken.Subject()}
+	} else {
+		claimValue, ok := parsedToken.Get(authenticator.mu.conf.claim)
+		if !ok {
+			return errors.WithDetailf(
+				errors.Newf("JWT authentication: missing claim"),
+				"token does not contain a claim for %s", authenticator.mu.conf.claim)
+		}
+		switch castClaimValue := claimValue.(type) {
+		case string:
+			// Accept a single string value.
+			tokenPrincipals = []string{castClaimValue}
+		case []interface{}:
+			// Iterate over the slice and add all string values to the tokenPrincipals.
+			for _, maybePrincipal := range castClaimValue {
+				tokenPrincipals = append(tokenPrincipals, fmt.Sprint(maybePrincipal))
+			}
+		case []string:
+			// This case never seems to happen but is included in case an implementation detail changes in the library.
+			tokenPrincipals = castClaimValue
+		default:
+			tokenPrincipals = []string{fmt.Sprint(castClaimValue)}
+		}
+	}
+
+	// Take the principals from the token and send each of them through the identity map to generate the
+	// list of usernames that this token is valid authentication for.
+	var acceptedUsernames []username.SQLUsername
+	for _, tokenPrincipal := range tokenPrincipals {
+		mappedUsernames, err := authenticator.mapUsername(tokenPrincipal, parsedToken.Issuer(), identMap)
+		if err != nil {
+			return errors.WithDetailf(
+				errors.Newf("JWT authentication: invalid claim value"),
+				"the value %s for the issuer %s is invalid", tokenPrincipal, parsedToken.Issuer())
+		}
+		acceptedUsernames = append(acceptedUsernames, mappedUsernames...)
+	}
+	if len(acceptedUsernames) == 0 {
 		return errors.WithDetailf(
-			errors.Newf("JWT authentication: invalid subject"),
-			"the subject %s for the issuer %s is invalid", parsedToken.Subject(), parsedToken.Issuer())
+			errors.Newf("JWT authentication: invalid principal"),
+			"the value %s for the issuer %s is invalid", tokenPrincipals, parsedToken.Issuer())
 	}
-	subjectMatch := false
-	for _, subject := range users {
-		if subject.Normalized() == user.Normalized() {
-			subjectMatch = true
+	principalMatch := false
+	for _, username := range acceptedUsernames {
+		if username.Normalized() == user.Normalized() {
+			principalMatch = true
 			break
 		}
 	}
-	if !subjectMatch {
+	if !principalMatch {
 		return errors.WithDetailf(
-			errors.Newf("JWT authentication: invalid subject"),
-			"token issued for %s and login was for %s", parsedToken.Subject(), user.Normalized())
+			errors.Newf("JWT authentication: invalid principal"),
+			"token issued for %s and login was for %s", tokenPrincipals, user.Normalized())
 	}
 	if user.IsRootUser() || user.IsReserved() {
 		return errors.WithDetailf(
@@ -216,6 +259,9 @@ var ConfigureJWTAuth = func(
 	JWTAuthJWKS.SetOnChange(&st.SV, func(ctx context.Context) {
 		authenticator.reloadConfig(ambientCtx.AnnotateCtx(ctx), st)
 	})
+	JWTAuthClaim.SetOnChange(&st.SV, func(ctx context.Context) {
+		authenticator.reloadConfig(ambientCtx.AnnotateCtx(ctx), st)
+	})
 	return &authenticator
 }