forked from cockroachdb/cockroach
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
…db#85865 cockroachdb#85866 cockroachdb#85885 cockroachdb#85906 cockroachdb#85914 85680: amazon,externalconn: add s3 support to External Connections r=adityamaru a=adityamaru This change registers s3 as a URI that can be represented as an External Connection. Most notably we take a page from the CDC book and switch the s3 parse function to check for invalid parameters, and configurations. This allows us to catch certain misconfiguration at the time we create the external connection. Informs: cockroachdb#84753 Release note (sql change): Users can now `CREATE EXTERNAL CONNECTION` to represent an s3 URI. 85849: sql/catalog/tabledesc: relaxed validation for virtual col in SUFFIX cols r=Xiang-Gu a=Xiang-Gu One of the validation rule says that "we don't allow virtual columns to be in the SUFFIX columns of a secondary index", except for one case: `ALTER PRIMARY KYE USING HASH`, where the implicitly created virtual, shard column, will need to appear in the SUFFIX columns of the implicitly created unique, secondary index of the old PK key columns ( which btw is a CRDB unique feature). The validation has logic to exempt us from this special case but it's written specifically to the legacy schema changer. Namely, it uses the canonical `PrimaryKeySwap` mutation type as the signal but we don't have that in the declarative schema changer. This PR addresses this issue and allows the validation logic to also exempt the exact same case but in the declarative schema changer. Release note: None 85862: ui: new insights table component r=maryliag a=maryliag Creation of the base insights table, that can be used on both schema insights and statement details page. This commit only introduce the table, with the different types, but still needs the proper cell formating for each type. Part of cockroachdb#83783 Release note: None 85865: rowexec: use the sorter's own context r=yuzefovich a=yuzefovich Release note: none 85866: streamingccl, rowexec: correctly mark eventStream as "streaming" r=yuzefovich a=yuzefovich This commit fixes an incomplete solution of 9bb5d30 which attempted to mark `eventStream` generator builtin as "streaming" so that the columnarizer on top of the `projectSetProcessor` would not buffer anything. As found by Steven, the solution in that commit was incomplete since the generators array is not populated at the time where `MustBeStreaming` check is performed. This commit fixes that oversight by using a different way of propagating the property - via the function properties. Release note: None 85885: kvserver: add queue size metric to RaftTransport r=tbg a=pavelkalinnikov Currently, there is a `RaftEnqueuePending` metric in `StoreMetrics` which exposes the `RaftTransport` outgoing queue size. However, this is a per-`Store` metric, so it duplicates the same variable across all the stores. The metric should be tracked on a per-node/transport basis. This PR introduces a per-transport `SendQueueSize` metric to replace the old `RaftEnqueuePending`. It also does the necessary plumbing to include it to the exported metrics, since it's the first metric in `RaftTransport`. <img width="1350" alt="Screenshot 2022-08-10 at 15 37 47" src="https://user-images.githubusercontent.com/3757441/183929666-0f6523d7-5877-4f2f-8460-4f013f87966d.png"> Touches cockroachdb#83917 Release note: None 85906: eval: don't always ignore error from ResolveFunctionByOID r=chengxiong-ruan a=rafiss This addresses a comment from the review of cockroachdb#85656. Release note: None 85914: systemschema_test: fix timestamp stripping regex r=postamar a=postamar The test in this package was flaky because hlc.Timestamp values inside descriptors (modification time, for instance) with a logical component would be improperly redacted. This commit fixes this. Fixes cockroachdb#85799. Release note: None Co-authored-by: Aditya Maru <[email protected]> Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Marylia Gutierrez <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Pavel Kalinnikov <[email protected]> Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Marius Posta <[email protected]>
- Loading branch information
Showing
45 changed files
with
745 additions
and
135 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") | ||
| load("@io_bazel_rules_go//go:def.bzl", "go_test") | ||
|
|
||
| go_test( | ||
| name = "amazon_test", | ||
| srcs = [ | ||
| "main_test.go", | ||
| "s3_connection_test.go", | ||
| ], | ||
| deps = [ | ||
| "//pkg/base", | ||
| "//pkg/ccl", | ||
| "//pkg/ccl/kvccl/kvtenantccl", | ||
| "//pkg/ccl/utilccl", | ||
| "//pkg/cloud", | ||
| "//pkg/cloud/amazon", | ||
| "//pkg/cloud/cloudpb", | ||
| "//pkg/cloud/externalconn/providers", | ||
| "//pkg/security/securityassets", | ||
| "//pkg/security/securitytest", | ||
| "//pkg/server", | ||
| "//pkg/testutils", | ||
| "//pkg/testutils/serverutils", | ||
| "//pkg/testutils/skip", | ||
| "//pkg/testutils/sqlutils", | ||
| "//pkg/testutils/testcluster", | ||
| "//pkg/util/leaktest", | ||
| "//pkg/util/log", | ||
| "//pkg/util/randutil", | ||
| "@com_github_aws_aws_sdk_go//aws/credentials", | ||
| ], | ||
| ) | ||
|
|
||
| get_x_data(name = "get_x_data") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright 2022 The Cockroach Authors. | ||
| // | ||
| // Licensed as a CockroachDB Enterprise file under the Cockroach Community | ||
| // License (the "License"); you may not use this file except in compliance with | ||
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt | ||
|
|
||
| package amazon_test | ||
|
|
||
| import ( | ||
| "os" | ||
| "testing" | ||
|
|
||
| _ "github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvtenantccl" | ||
| "github.com/cockroachdb/cockroach/pkg/ccl/utilccl" | ||
| "github.com/cockroachdb/cockroach/pkg/security/securityassets" | ||
| "github.com/cockroachdb/cockroach/pkg/security/securitytest" | ||
| "github.com/cockroachdb/cockroach/pkg/server" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" | ||
| "github.com/cockroachdb/cockroach/pkg/util/randutil" | ||
| ) | ||
|
|
||
| func TestMain(m *testing.M) { | ||
| defer utilccl.TestingEnableEnterprise()() | ||
|
|
||
| securityassets.SetLoader(securitytest.EmbeddedAssets) | ||
| randutil.SeedForTests() | ||
| serverutils.InitTestServerFactory(server.TestServerFactory) | ||
| serverutils.InitTestClusterFactory(testcluster.TestClusterFactory) | ||
| os.Exit(m.Run()) | ||
| } | ||
|
|
||
| //go:generate ../../../util/leaktest/add-leaktest.sh *_test.go |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| // Copyright 2020 The Cockroach Authors. | ||
| // | ||
| // Licensed as a CockroachDB Enterprise file under the Cockroach Community | ||
| // License (the "License"); you may not use this file except in compliance with | ||
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt | ||
|
|
||
| package amazon | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "net/url" | ||
| "os" | ||
| "testing" | ||
|
|
||
| "github.com/aws/aws-sdk-go/aws/credentials" | ||
| "github.com/cockroachdb/cockroach/pkg/base" | ||
| _ "github.com/cockroachdb/cockroach/pkg/ccl" | ||
| "github.com/cockroachdb/cockroach/pkg/cloud" | ||
| "github.com/cockroachdb/cockroach/pkg/cloud/amazon" | ||
| "github.com/cockroachdb/cockroach/pkg/cloud/cloudpb" | ||
| _ "github.com/cockroachdb/cockroach/pkg/cloud/externalconn/providers" // import External Connection providers. | ||
| "github.com/cockroachdb/cockroach/pkg/testutils" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/skip" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" | ||
| "github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
| "github.com/cockroachdb/cockroach/pkg/util/log" | ||
| ) | ||
|
|
||
| func TestS3ExternalConnection(t *testing.T) { | ||
| defer leaktest.AfterTest(t)() | ||
| defer log.Scope(t).Close(t) | ||
|
|
||
| dir, dirCleanupFn := testutils.TempDir(t) | ||
| defer dirCleanupFn() | ||
|
|
||
| params := base.TestClusterArgs{} | ||
| params.ServerArgs.ExternalIODir = dir | ||
|
|
||
| tc := testcluster.StartTestCluster(t, 1, params) | ||
| defer tc.Stopper().Stop(context.Background()) | ||
|
|
||
| tc.WaitForNodeLiveness(t) | ||
| sqlDB := sqlutils.MakeSQLRunner(tc.Conns[0]) | ||
|
|
||
| // Setup some dummy data. | ||
| sqlDB.Exec(t, `CREATE DATABASE foo`) | ||
| sqlDB.Exec(t, `USE foo`) | ||
| sqlDB.Exec(t, `CREATE TABLE foo (id INT PRIMARY KEY)`) | ||
| sqlDB.Exec(t, `INSERT INTO foo VALUES (1), (2), (3)`) | ||
|
|
||
| createExternalConnection := func(externalConnectionName, uri string) { | ||
| sqlDB.Exec(t, fmt.Sprintf(`CREATE EXTERNAL CONNECTION '%s' AS '%s'`, externalConnectionName, uri)) | ||
| } | ||
| backupAndRestoreFromExternalConnection := func(backupExternalConnectionName string) { | ||
| backupURI := fmt.Sprintf("external://%s", backupExternalConnectionName) | ||
| sqlDB.Exec(t, fmt.Sprintf(`BACKUP DATABASE foo INTO '%s'`, backupURI)) | ||
| sqlDB.Exec(t, fmt.Sprintf(`RESTORE DATABASE foo FROM LATEST IN '%s' WITH new_db_name = bar`, backupURI)) | ||
| sqlDB.CheckQueryResults(t, `SELECT * FROM bar.foo`, [][]string{{"1"}, {"2"}, {"3"}}) | ||
| sqlDB.CheckQueryResults(t, `SELECT * FROM crdb_internal.invalid_objects`, [][]string{}) | ||
| sqlDB.Exec(t, `DROP DATABASE bar CASCADE`) | ||
| } | ||
|
|
||
| // If environment credentials are not present, we want to | ||
| // skip all S3 tests, including auth-implicit, even though | ||
| // it is not used in auth-implicit. | ||
| creds, err := credentials.NewEnvCredentials().Get() | ||
| if err != nil { | ||
| skip.IgnoreLint(t, "No AWS credentials") | ||
| } | ||
| bucket := os.Getenv("AWS_S3_BUCKET") | ||
| if bucket == "" { | ||
| skip.IgnoreLint(t, "AWS_S3_BUCKET env var must be set") | ||
| } | ||
|
|
||
| t.Run("auth-implicit", func(t *testing.T) { | ||
| // You can create an IAM that can access S3 | ||
| // in the AWS console, then set it up locally. | ||
| // https://docs.aws.com/cli/latest/userguide/cli-configure-role.html | ||
| // We only run this test if default role exists. | ||
| credentialsProvider := credentials.SharedCredentialsProvider{} | ||
| _, err := credentialsProvider.Retrieve() | ||
| if err != nil { | ||
| skip.IgnoreLintf(t, "we only run this test if a default role exists, "+ | ||
| "refer to https://docs.aws.com/cli/latest/userguide/cli-configure-role.html: %s", err) | ||
| } | ||
|
|
||
| // Set the AUTH to implicit. | ||
| params := make(url.Values) | ||
| params.Add(cloud.AuthParam, cloud.AuthParamImplicit) | ||
|
|
||
| s3URI := fmt.Sprintf("s3://%s/backup-ec-test-default?%s", bucket, params.Encode()) | ||
| ecName := "auth-implicit-s3" | ||
| createExternalConnection(ecName, s3URI) | ||
| backupAndRestoreFromExternalConnection(ecName) | ||
| }) | ||
|
|
||
| t.Run("auth-specified", func(t *testing.T) { | ||
| s3URI := amazon.S3URI(bucket, "backup-ec-test-default", | ||
| &cloudpb.ExternalStorage_S3{ | ||
| AccessKey: creds.AccessKeyID, | ||
| Secret: creds.SecretAccessKey, | ||
| Region: "us-east-1", | ||
| Auth: cloud.AuthParamSpecified, | ||
| }, | ||
| ) | ||
| ecName := "auth-specified-s3" | ||
| createExternalConnection(ecName, s3URI) | ||
| backupAndRestoreFromExternalConnection(ecName) | ||
| }) | ||
|
|
||
| // Tests that we can put an object with server side encryption specified. | ||
| t.Run("server-side-encryption", func(t *testing.T) { | ||
| // You can create an IAM that can access S3 | ||
| // in the AWS console, then set it up locally. | ||
| // https://docs.aws.com/cli/latest/userguide/cli-configure-role.html | ||
| // We only run this test if default role exists. | ||
| credentialsProvider := credentials.SharedCredentialsProvider{} | ||
| _, err := credentialsProvider.Retrieve() | ||
| if err != nil { | ||
| skip.IgnoreLintf(t, "we only run this test if a default role exists, "+ | ||
| "refer to https://docs.aws.com/cli/latest/userguide/cli-configure-role.html: %s", err) | ||
| } | ||
|
|
||
| s3URI := amazon.S3URI(bucket, "backup-ec-test-sse-256", &cloudpb.ExternalStorage_S3{ | ||
| Region: "us-east-1", | ||
| Auth: cloud.AuthParamImplicit, | ||
| ServerEncMode: "AES256", | ||
| }) | ||
| ecName := "server-side-encryption-s3" | ||
| createExternalConnection(ecName, s3URI) | ||
| backupAndRestoreFromExternalConnection(ecName) | ||
|
|
||
| v := os.Getenv("AWS_KMS_KEY_ARN") | ||
| if v == "" { | ||
| skip.IgnoreLint(t, "AWS_KMS_KEY_ARN env var must be set") | ||
| } | ||
| s3KMSURI := amazon.S3URI(bucket, "backup-ec-test-sse-kms", &cloudpb.ExternalStorage_S3{ | ||
| Region: "us-east-1", | ||
| Auth: cloud.AuthParamImplicit, | ||
| ServerEncMode: "aws:kms", | ||
| ServerKMSID: v, | ||
| }) | ||
| ecName = "server-side-encryption-kms-s3" | ||
| createExternalConnection(ecName, s3KMSURI) | ||
| backupAndRestoreFromExternalConnection(ecName) | ||
| }) | ||
|
|
||
| t.Run("server-side-encryption-invalid-params", func(t *testing.T) { | ||
| // You can create an IAM that can access S3 | ||
| // in the AWS console, then set it up locally. | ||
| // https://docs.aws.com/cli/latest/userguide/cli-configure-role.html | ||
| // We only run this test if default role exists. | ||
| credentialsProvider := credentials.SharedCredentialsProvider{} | ||
| _, err := credentialsProvider.Retrieve() | ||
| if err != nil { | ||
| skip.IgnoreLintf(t, "we only run this test if a default role exists, "+ | ||
| "refer to https://docs.aws.com/cli/latest/userguide/cli-configure-role.html: %s", err) | ||
| } | ||
|
|
||
| // Unsupported server side encryption option. | ||
| invalidS3URI := amazon.S3URI(bucket, "backup-ec-test-sse-256", &cloudpb.ExternalStorage_S3{ | ||
| Region: "us-east-1", | ||
| Auth: cloud.AuthParamImplicit, | ||
| ServerEncMode: "unsupported-algorithm", | ||
| }) | ||
| sqlDB.ExpectErr(t, | ||
| "unsupported server encryption mode unsupported-algorithm. Supported values are `aws:kms` and `AES256", | ||
| fmt.Sprintf(`BACKUP DATABASE foo INTO '%s'`, invalidS3URI)) | ||
|
|
||
| invalidS3URI = amazon.S3URI(bucket, "backup-ec-test-sse-256", &cloudpb.ExternalStorage_S3{ | ||
| Region: "us-east-1", | ||
| Auth: cloud.AuthParamImplicit, | ||
| ServerEncMode: "aws:kms", | ||
| }) | ||
|
|
||
| // Specify aws:kms encryption mode but don't specify kms ID. | ||
| sqlDB.ExpectErr(t, "AWS_SERVER_KMS_ID param must be set when using aws:kms server side encryption mode.", fmt.Sprintf(`BACKUP DATABASE foo INTO '%s'`, | ||
| invalidS3URI)) | ||
| }) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.