From 7e3836273fa635f89639d96ec6c58bd8b3268b85 Mon Sep 17 00:00:00 2001 From: Jeff Swenson Date: Mon, 18 Nov 2024 20:19:04 +0000 Subject: [PATCH] cloud: use uuid as bucket name Previously, the GCS and S3 tests used hard coded bucket names to test the behavior of buckets that do not exist. Now, the tests use a random UUID. This change is necessary because someone appears to have created the GCP bucket which causes the test to fail. Release Justification: Test Only Change Release Note: None Fixes: #135307 Fixes: #135348 Fixes: #135349 Fixes: #135350 Fixes: #135351 Fixes: #135352 --- pkg/cloud/amazon/BUILD.bazel | 1 + pkg/cloud/amazon/s3_storage_test.go | 12 ++++++------ pkg/cloud/gcp/BUILD.bazel | 1 + pkg/cloud/gcp/gcs_storage_test.go | 13 ++++++------- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pkg/cloud/amazon/BUILD.bazel b/pkg/cloud/amazon/BUILD.bazel index d08dc87ee1bf..65850e6e0745 100644 --- a/pkg/cloud/amazon/BUILD.bazel +++ b/pkg/cloud/amazon/BUILD.bazel @@ -66,6 +66,7 @@ go_test( "//pkg/testutils", "//pkg/testutils/skip", "//pkg/util/leaktest", + "//pkg/util/uuid", "@com_github_aws_aws_sdk_go//aws/awserr", "@com_github_aws_aws_sdk_go//aws/request", "@com_github_aws_aws_sdk_go_v2_config//:config", diff --git a/pkg/cloud/amazon/s3_storage_test.go b/pkg/cloud/amazon/s3_storage_test.go index 1ae74f5a66a4..a50d99c3ca03 100644 --- a/pkg/cloud/amazon/s3_storage_test.go +++ b/pkg/cloud/amazon/s3_storage_test.go @@ -28,6 +28,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/cockroachdb/errors" "github.com/stretchr/testify/require" ) @@ -529,11 +530,11 @@ func TestS3BucketDoesNotExist(t *testing.T) { q.Add(cloud.AuthParam, cloud.AuthParamImplicit) q.Add(S3RegionParam, "us-east-1") - // Very long, very random bucket name, that hopefully nobody will ever create. - bucket := "VIBK1H88MOJ665V2RAPVH6X3TWUS0HCWTW5A27AFPPLHMABKH7X445K86K1BP2" u := url.URL{ - Scheme: "s3", - Host: bucket, + Scheme: "s3", + // Use UUID to create a random bucket name that does not exist. If this was + // a constent, someone could create the bucket and break the test. + Host: uuid.NewV4().String(), Path: "backup-test", RawQuery: q.Encode(), } @@ -563,8 +564,7 @@ func TestS3BucketDoesNotExist(t *testing.T) { } _, _, err = s.ReadFile(ctx, "", cloud.ReadOptions{NoFileSize: true}) - require.Error(t, err, "") - require.True(t, errors.Is(err, cloud.ErrFileDoesNotExist), "error is not cloud.ErrFileDoesNotExist: %v", err) + require.ErrorIs(t, err, cloud.ErrFileDoesNotExist) } func TestAntagonisticS3Read(t *testing.T) { diff --git a/pkg/cloud/gcp/BUILD.bazel b/pkg/cloud/gcp/BUILD.bazel index 1cb671beeaf5..04a95c499dd1 100644 --- a/pkg/cloud/gcp/BUILD.bazel +++ b/pkg/cloud/gcp/BUILD.bazel @@ -60,6 +60,7 @@ go_test( "//pkg/testutils/skip", "//pkg/util/ioctx", "//pkg/util/leaktest", + "//pkg/util/uuid", "@com_github_cockroachdb_errors//:errors", "@com_github_googleapis_gax_go_v2//apierror", "@com_github_stretchr_testify//assert", diff --git a/pkg/cloud/gcp/gcs_storage_test.go b/pkg/cloud/gcp/gcs_storage_test.go index e1454f9ee816..ddc7350d7ed3 100644 --- a/pkg/cloud/gcp/gcs_storage_test.go +++ b/pkg/cloud/gcp/gcs_storage_test.go @@ -25,7 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/ioctx" "github.com/cockroachdb/cockroach/pkg/util/leaktest" - "github.com/cockroachdb/errors" + "github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/stretchr/testify/require" "golang.org/x/oauth2/google" ) @@ -355,13 +355,13 @@ func TestFileDoesNotExist(t *testing.T) { ) require.NoError(t, err) _, _, err = s.ReadFile(context.Background(), "", cloud.ReadOptions{NoFileSize: true}) - require.Error(t, err, "") - require.True(t, errors.Is(err, cloud.ErrFileDoesNotExist)) + require.ErrorIs(t, err, cloud.ErrFileDoesNotExist) } { - // Invalid gsBucket. - gsFile := "gs://cockroach-fixtures-us-east1-invalid/tpch-csv/sf-1/region.tbl?AUTH=implicit" + // Use a random UUID as the name of the bucket that does not exist in order + // to avoid name squating. + gsFile := fmt.Sprintf("gs://%s/tpch-csv/sf-1/region.tbl?AUTH=implicit", uuid.NewV4()) conf, err := cloud.ExternalStorageConfFromURI(gsFile, user) require.NoError(t, err) @@ -373,8 +373,7 @@ func TestFileDoesNotExist(t *testing.T) { ) require.NoError(t, err) _, _, err = s.ReadFile(context.Background(), "", cloud.ReadOptions{NoFileSize: true}) - require.Error(t, err, "") - require.True(t, errors.Is(err, cloud.ErrFileDoesNotExist)) + require.ErrorIs(t, err, cloud.ErrFileDoesNotExist) } }