Skip to content

Commit

Permalink
Merge #131943 #132141 #132194 #132195
Browse files Browse the repository at this point in the history 
131943: server/license: Make enterprise license check a no-op r=rafiss,fqazi,rharding6373 a=spilchen

With the deprecation of the core license, there’s no longer a need to check whether enterprise features are enabled—all features are now always enabled. If a license policy violation occurs, such as an expired license, we will throttle connections, but all enterprise features will remain functional. I’ve left the API for checking if enterprise features are enabled in the code, but these functions are now no-ops and can be removed in the future.

I also had to adjust some tests that were explicitly disabling enterprise features, as that’s no longer possible. Some tests were rewritten to fit the new model, while others were removed entirely because they were no longer relevant.

Note: A few tests in `pkg/sql` had to be updated due to the changes in enterprise checks. With all enterprise features enabled, the default isolation level has also changed. Any test that explicitly disabled leases (using TestingDisableTableLeases) needed to apply this setting _after_ starting up the test server. Applying it earlier, as was originally done, caused the test server to fail during startup.

This change will be backported to versions 24.2, 24.1, 23.2, and 23.1.

Epic: CRDB-39988
Closes: CRDB-41998
Release note: none

132141: kvserver: schedule ready processing when the leaseholder changes r=kvoli a=sumeerbhola

Epic: CRDB-37515

Release note: None

132194: crosscluster: update tests to `BACKUP`/`RESTORE` syntax r=msbutler a=kev-cao

Several tests still use the old `BACKUP`/`RESTORE` syntax. This patch updates some of the tests to the new `BACKUP INTO`/`RESTORE FROM ... IN ...` syntax.

Epic: none

Release note: None


132195: testccl: update tests to `BACKUP INTO` syntax r=msbutler a=kev-cao

Several tests still use the old `BACKUP TO` syntax. This patch updates some of the tests to the new `BACKUP INTO` syntax.

Epic: none

Release note: None

Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Kevin Cao <[email protected]>
  • Loading branch information
@spilchen @sumeerbhola @kev-cao
4 people committed Oct 9, 2024
5 parents 8c0f678 + c5a45a6 + 84c1dc4 + 625abbd + 5950824 commit c104fb1
Show file tree
Hide file tree
Showing 33 changed files with 195 additions and 582 deletions.
2 changes: 1 addition & 1 deletion pkg/acceptance/compose/gss/docker-compose-python.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ services:
timeout: 10s
retries: 25
python:
image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-python:20221214-141947
image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-python:20241004-141441
user: "${UID}:${GID}"
depends_on:
cockroach:
Expand Down
2 changes: 1 addition & 1 deletion pkg/acceptance/compose/gss/docker-compose.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ services:
timeout: 10s
retries: 25
psql:
image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-psql:20230907-113902
image: us-east1-docker.pkg.dev/crl-ci-images/cockroach/acceptance-gss-psql:20241004-140930
user: "${UID}:${GID}"
depends_on:
cockroach:
Expand Down
16 changes: 8 additions & 8 deletions pkg/acceptance/compose/gss/psql/gss_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ func TestGSS(t *testing.T) {
{
conf: `host all all all gss include_realm=0`,
user: "tester",
gssErr: `GSS authentication requires an enterprise license`,
gssErr: "",
},
{
conf: `host all tester all gss include_realm=0`,
user: "tester",
gssErr: `GSS authentication requires an enterprise license`,
gssErr: "",
},
{
conf: `host all nope all gss include_realm=0`,
Expand All @@ -89,7 +89,7 @@ func TestGSS(t *testing.T) {
{
conf: `host all all all gss include_realm=0 krb_realm=MY.EX`,
user: "tester",
gssErr: `GSS authentication requires an enterprise license`,
gssErr: "",
},
{
conf: `host all all all gss include_realm=0 krb_realm=NOPE.EX`,
Expand All @@ -99,7 +99,7 @@ func TestGSS(t *testing.T) {
{
conf: `host all all all gss include_realm=0 krb_realm=NOPE.EX krb_realm=MY.EX`,
user: "tester",
gssErr: `GSS authentication requires an enterprise license`,
gssErr: "",
},
// Validate that we can use the "map" option to strip the realm
// data. Note that the system-identity value will have been
Expand All @@ -108,7 +108,7 @@ func TestGSS(t *testing.T) {
conf: `host all all all gss map=demo`,
identMap: `demo /^(.*)@my.ex$ \1`,
user: "tester",
gssErr: `GSS authentication requires an enterprise license`,
gssErr: "",
},
// Verify case-sensitivity.
{
Expand All @@ -129,7 +129,7 @@ func TestGSS(t *testing.T) {
conf: `host all all all gss include_realm=0 map=demo`,
identMap: `demo tester remapped`,
user: "remapped",
gssErr: `GSS authentication requires an enterprise license`,
gssErr: "",
},
}
for i, tc := range tests {
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestGSS(t *testing.T) {
if !IsError(err, tc.gssErr) {
t.Errorf("expected err %v, got %v", tc.gssErr, err)
}
if tc.gsErr == "" {
if tc.gssErr == "" {
if !strings.Contains(string(out), "gss") {
t.Errorf("expected authentication_method=gss, got %s", out)
}
Expand Down Expand Up @@ -214,7 +214,7 @@ func TestGSSFileDescriptorCount(t *testing.T) {
for i := 0; i < 1000; i++ {
fmt.Println(i, time.Since(start))
out, err := exec.Command("psql", "-c", "SELECT 1", "-U", user).CombinedOutput()
if IsError(err, "GSS authentication requires an enterprise license") {
if err != nil {
t.Log(string(out))
t.Fatal(err)
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/acceptance/compose/gss/python/start.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,4 @@ unset PGSSLKEY
unset PGSSLCERT
export PGUSER=tester

# Exit with error unless we find the expected error message.
python manage.py inspectdb 2>&1 | grep 'use of GSS authentication requires an enterprise license'
python manage.py inspectdb
1 change: 0 additions & 1 deletion pkg/ccl/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ go_library(
"//pkg/ccl/storageccl/engineccl",
"//pkg/ccl/utilccl",
"//pkg/ccl/workloadccl",
"//pkg/server",
"//pkg/server/license",
],
)
1 change: 0 additions & 1 deletion pkg/ccl/auditloggingccl/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ go_test(
deps = [
"//pkg/base",
"//pkg/ccl",
"//pkg/ccl/utilccl",
"//pkg/security/securityassets",
"//pkg/security/securitytest",
"//pkg/security/username",
Expand Down
78 changes: 0 additions & 78 deletions pkg/ccl/auditloggingccl/audit_logging_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
Expand All @@ -27,83 +26,6 @@ import (
"github.com/stretchr/testify/require"
)

func TestRoleBasedAuditEnterpriseGated(t *testing.T) {
defer leaktest.AfterTest(t)()
sc := log.ScopeWithoutShowLogs(t)
defer sc.Close(t)

cleanup := logtestutils.InstallLogFileSink(sc, t, logpb.Channel_SENSITIVE_ACCESS)
defer cleanup()

s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
rootRunner := sqlutils.MakeSQLRunner(sqlDB)
defer s.Stopper().Stop(context.Background())

// Disable enterprise.
enableEnterprise := utilccl.TestingDisableEnterprise()

// Enable auditing.
rootRunner.Exec(t, `SET CLUSTER SETTING sql.log.user_audit = 'ALL ALL'`)

testutils.SucceedsSoon(t, func() error {
var currentVal string
rootRunner.QueryRow(t,
"SHOW CLUSTER SETTING sql.log.user_audit",
).Scan(&currentVal)
if currentVal == "" {
return errors.Newf("waiting for cluster setting to be set")
}
return nil
})

// Run a test query.
rootRunner.Exec(t, `SHOW CLUSTER SETTING sql.log.user_audit`)

log.FlushFiles()

entries, err := log.FetchEntriesFromFiles(
0,
math.MaxInt64,
10000,
regexp.MustCompile(`"EventType":"role_based_audit_event"`),
log.WithMarkedSensitiveData,
)

if err != nil {
t.Fatal(err)
}

// Enterprise is disabled, expect the number of entries to be 0.
if len(entries) != 0 {
t.Fatal(errors.Newf("enterprise is disabled, found unexpected entries"))
}

// Enable enterprise
enableEnterprise()

// Run a test query.
rootRunner.Exec(t, `SHOW CLUSTER SETTING sql.log.user_audit`)

log.FlushFiles()

entries, err = log.FetchEntriesFromFiles(
0,
math.MaxInt64,
10000,
regexp.MustCompile(`"Statement":"SHOW CLUSTER SETTING \\"sql.log.user_audit\\""`),
log.WithMarkedSensitiveData,
)

if err != nil {
t.Fatal(err)
}

// Enterprise is enabled, expect an audit log.
if len(entries) != 1 {
t.Fatal(errors.Newf("enterprise is enabled, expected 1 entry, got %d", len(entries)))
}
}

func TestSingleRoleAuditLogging(t *testing.T) {
defer leaktest.AfterTest(t)()
sc := log.ScopeWithoutShowLogs(t)
Expand Down
81 changes: 43 additions & 38 deletions pkg/ccl/backupccl/create_scheduled_backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,57 +280,57 @@ CREATE TABLE other_db.t1(a int);
{
name: "fully-qualified-table-name",
query: "CREATE SCHEDULE FOR BACKUP mydb.public.t1 INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "schema-qualified-table-name",
query: "CREATE SCHEDULE FOR BACKUP public.t1 INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "uds-qualified-table-name",
query: "CREATE SCHEDULE FOR BACKUP myschema.mytbl INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.myschema.mytbl INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.myschema.mytbl INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "db-qualified-table-name",
query: "CREATE SCHEDULE FOR BACKUP mydb.t1 INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "unqualified-table-name",
query: "CREATE SCHEDULE FOR BACKUP t1 INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1 INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "unqualified-table-name-with-symbols",
query: `CREATE SCHEDULE FOR BACKUP "my.tbl" INTO $1 RECURRING '@hourly'`,
expectedBackupStmt: `BACKUP TABLE mydb.public."my.tbl" INTO '%s' WITH OPTIONS (detached)`,
expectedBackupStmt: `BACKUP TABLE mydb.public."my.tbl" INTO %s'%s' WITH OPTIONS (detached)`,
},
{
name: "table-names-from-different-db",
query: "CREATE SCHEDULE FOR BACKUP t1, other_db.t1 INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1, other_db.public.t1 INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.t1, other_db.public.t1 INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "unqualified-all-tables-selectors",
query: "CREATE SCHEDULE FOR BACKUP * INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.* INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.* INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "all-tables-selectors-with-user-defined-schema",
query: "CREATE SCHEDULE FOR BACKUP myschema.* INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.myschema.* INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.myschema.* INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "partially-qualified-all-tables-selectors-with-different-db",
query: "CREATE SCHEDULE FOR BACKUP other_db.* INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE other_db.public.* INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE other_db.public.* INTO %s'%s' WITH OPTIONS (detached)",
},
{
name: "fully-qualified-all-tables-selectors-with-multiple-dbs",
query: "CREATE SCHEDULE FOR BACKUP *, other_db.* INTO $1 RECURRING '@hourly'",
expectedBackupStmt: "BACKUP TABLE mydb.public.*, other_db.public.* INTO '%s' WITH OPTIONS (detached)",
expectedBackupStmt: "BACKUP TABLE mydb.public.*, other_db.public.* INTO %s'%s' WITH OPTIONS (detached)",
},
}

Expand All @@ -343,9 +343,13 @@ CREATE TABLE other_db.t1(a int);
schedules, err := th.createBackupSchedule(t, tc.query, destination)
require.NoError(t, err)

for _, s := range schedules {
for i, s := range schedules {
var latest string
if i == 0 {
latest = "LATEST IN "
}
stmt := getScheduledBackupStatement(t, s.ExecutionArgs())
require.Equal(t, fmt.Sprintf(tc.expectedBackupStmt, destination), stmt)
require.Equal(t, fmt.Sprintf(tc.expectedBackupStmt, latest, destination), stmt, "schedule %d", i)
}
})
}
Expand Down Expand Up @@ -387,10 +391,20 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) {
user: freeUser,
expectedSchedules: []expectedSchedule{
{
nameRe: "BACKUP .+",
backupStmt: "BACKUP INTO 'nodelocal://1/backup?AWS_SECRET_ACCESS_KEY=neverappears' WITH OPTIONS (detached)",
shownStmt: "BACKUP INTO 'nodelocal://1/backup?AWS_SECRET_ACCESS_KEY=redacted' WITH OPTIONS (detached)",
period: time.Hour,
nameRe: "BACKUP .*",
backupStmt: "BACKUP INTO LATEST IN 'nodelocal://1/backup?AWS_SECRET_ACCESS_KEY=neverappears' WITH OPTIONS (detached)",
shownStmt: "BACKUP INTO LATEST IN 'nodelocal://1/backup?AWS_SECRET_ACCESS_KEY=redacted' WITH OPTIONS (detached)",
period: time.Hour,
paused: true,
chainProtectedTimestampRecord: true,
},
{
nameRe: "BACKUP .+",
backupStmt: "BACKUP INTO 'nodelocal://1/backup?AWS_SECRET_ACCESS_KEY=neverappears' WITH OPTIONS (detached)",
shownStmt: "BACKUP INTO 'nodelocal://1/backup?AWS_SECRET_ACCESS_KEY=redacted' WITH OPTIONS (detached)",
period: 24 * time.Hour,
runsNow: true,
chainProtectedTimestampRecord: true,
},
},
},
Expand All @@ -400,9 +414,18 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) {
user: freeUser,
expectedSchedules: []expectedSchedule{
{
nameRe: "my-backup",
backupStmt: "BACKUP INTO 'nodelocal://1/backup' WITH OPTIONS (detached)",
period: time.Hour,
nameRe: "my-backup",
backupStmt: "BACKUP INTO LATEST IN 'nodelocal://1/backup' WITH OPTIONS (detached)",
period: time.Hour,
paused: true,
chainProtectedTimestampRecord: true,
},
{
nameRe: "my-backup",
backupStmt: "BACKUP INTO 'nodelocal://1/backup' WITH OPTIONS (detached)",
period: 24 * time.Hour,
runsNow: true,
chainProtectedTimestampRecord: true,
},
},
},
Expand Down Expand Up @@ -565,24 +588,6 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) {
},
},
},
{
name: "enterprise-license-required-for-incremental",
query: "CREATE SCHEDULE FOR BACKUP INTO 'nodelocal://1/backup' RECURRING '@hourly' FULL BACKUP '@weekly'",
user: freeUser,
errMsg: "use of BACKUP INTO LATEST requires an enterprise license",
},
{
name: "enterprise-license-required-for-revision-history",
query: "CREATE SCHEDULE FOR BACKUP INTO 'nodelocal://1/backup' WITH revision_history RECURRING '@hourly'",
user: freeUser,
errMsg: "use of BACKUP with revision_history requires an enterprise license",
},
{
name: "enterprise-license-required-for-encryption",
query: "CREATE SCHEDULE FOR BACKUP INTO 'nodelocal://1/backup' WITH encryption_passphrase = 'secret' RECURRING '@hourly'",
user: freeUser,
errMsg: "use of BACKUP with encryption requires an enterprise license",
},
{
name: "full-cluster-with-name-arg",
query: `CREATE SCHEDULE $1 FOR BACKUP INTO 'nodelocal://1/backup' WITH revision_history, detached RECURRING '@hourly'`,
Expand Down
5 changes: 0 additions & 5 deletions pkg/ccl/ccl_init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,17 @@ import (
_ "github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
_ "github.com/cockroachdb/cockroach/pkg/ccl/workloadccl"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/server/license"
)

func init() {
// Let CheckEnterpriseEnabled know that all the ccl code is linked in.
utilccl.AllCCLCodeImported = true

// Set up license-related hooks from OSS to CCL. The implementation of the
// functions we bind is in utilccl, but license checks only work once
// utilccl.AllCCLCodeImported is set, above; that's why this hookup is done in
// this `ccl` pkg.
base.CheckEnterpriseEnabled = utilccl.CheckEnterpriseEnabled
base.LicenseType = utilccl.GetLicenseType
base.GetLicenseTTL = utilccl.GetLicenseTTL
server.ApplyTenantLicense = utilccl.ApplyTenantLicense
license.RegisterCallbackOnLicenseChange = utilccl.RegisterCallbackOnLicenseChange
}

Expand Down
11 changes: 0 additions & 11 deletions pkg/ccl/changefeedccl/changefeed_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5190,17 +5190,6 @@ func TestChangefeedErrors(t *testing.T) {
`CREATE CHANGEFEED FOR foo INTO $1`, ``,
)

enableEnterprise := utilccl.TestingDisableEnterprise()
sqlDB.ExpectErrWithTimeout(
t, `CHANGEFEED requires an enterprise license`,
`CREATE CHANGEFEED FOR foo INTO $1`, `kafka://nope`,
)
sqlDB.ExpectErrWithTimeout(
t, `use of AS SELECT requires an enterprise license`,
`CREATE CHANGEFEED AS SELECT * FROM foo`,
)
enableEnterprise()

// Watching system.jobs would create a cycle, since the resolved timestamp
// high-water mark is saved in it.
sqlDB.ExpectErrWithTimeout(
Expand Down
Loading

0 comments on commit c104fb1

Please sign in to comment.