Skip to content

Commit

Permalink
cloud/azure: Fix azure schemes
Browse files Browse the repository at this point in the history
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-31120

Release note (bug fix): Fixes azure schemes in storage, kms and external conns.
  • Loading branch information
benbardin committed Sep 25, 2023
1 parent 529843e commit 5cd4c7b
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 11 deletions.
3 changes: 2 additions & 1 deletion pkg/ccl/cloudccl/externalconn/datadriven_test.go
Expand Up @@ -132,7 +132,8 @@ SELECT connection_name,
crdb_internal.pb_to_json('cockroach.cloud.externalconn.connectionpb.ConnectionDetails', connection_details),
owner,
owner_id
FROM system.external_connections;
FROM system.external_connections
ORDER BY connection_name;
`)
output, err := sqlutils.RowsToDataDrivenOutput(rows)
require.NoError(t, err)
Expand Down
Expand Up @@ -229,6 +229,14 @@ exec-sql
CREATE EXTERNAL CONNECTION "foo-azure" AS 'azure-storage://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud'
----

exec-sql
CREATE EXTERNAL CONNECTION "bar-azure" AS 'azure-blob://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud'
----

exec-sql
CREATE EXTERNAL CONNECTION "baz-azure" AS 'azure://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud'
----

# Reject invalid azure external connections.
exec-sql
CREATE EXTERNAL CONNECTION "invalid-param-azure" AS 'azure-storage://bucket/path?INVALIDPARAM=baz'
Expand All @@ -247,13 +255,55 @@ pq: failed to construct External Connection details: failed to create azure exte

inspect-system-table
----
bar-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure-blob://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root 1
baz-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root 1
foo-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure-storage://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-azure";
----

exec-sql
DROP EXTERNAL CONNECTION "bar-azure";
----

exec-sql
DROP EXTERNAL CONNECTION "baz-azure";
----

enable-check-external-storage
----

disable-check-kms
----

exec-sql
CREATE EXTERNAL CONNECTION "foo-kms" AS 'azure-kms:///cmk/id?AUTH=specified&AZURE_VAULT_NAME=vault&AZURE_CLIENT_ID=client&AZURE_CLIENT_SECRET=secret&AZURE_TENANT_ID=tenant';
----

# Reject invalid KMS URIs.
exec-sql
CREATE EXTERNAL CONNECTION "missing-cmk-kms" AS 'azure-kms:///?AUTH=implicit&CREDENTIALS=baz&ASSUME_ROLE=ronaldo,rashford,bruno&BEARER_TOKEN=foo';
----
pq: failed to construct External Connection details: failed to create Azure KMS external connection: path component of the KMS cannot be empty; must contain the Customer Managed Key

exec-sql
CREATE EXTERNAL CONNECTION "invalid-params-kms" AS 'azure-kms:///cmk?AUTH=implicit&INVALIDPARAM=baz';
----
pq: failed to construct External Connection details: failed to create Azure KMS external connection: unknown KMS query parameters: INVALIDPARAM

inspect-system-table
----
foo-kms KMS {"provider": "azure_kms", "simpleUri": {"uri": "azure-kms:///cmk/id?AUTH=specified&AZURE_VAULT_NAME=vault&AZURE_CLIENT_ID=client&AZURE_CLIENT_SECRET=secret&AZURE_TENANT_ID=tenant"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-kms";
----

inspect-system-table
----

enable-check-kms
----

subtest end
4 changes: 4 additions & 0 deletions pkg/cloud/azure/BUILD.bazel
Expand Up @@ -45,7 +45,9 @@ go_library(
go_test(
name = "azure_test",
srcs = [
"azure_connection_test.go",
"azure_file_credentials_test.go",
"azure_kms_connection_test.go",
"azure_kms_test.go",
"azure_storage_test.go",
],
Expand All @@ -57,6 +59,8 @@ go_test(
"//pkg/cloud",
"//pkg/cloud/cloudpb",
"//pkg/cloud/cloudtestutils",
"//pkg/cloud/externalconn",
"//pkg/cloud/externalconn/connectionpb",
"//pkg/security/username",
"//pkg/settings/cluster",
"//pkg/testutils",
Expand Down
17 changes: 9 additions & 8 deletions pkg/cloud/azure/azure_connection.go
Expand Up @@ -30,12 +30,13 @@ func validateAzureConnectionURI(
}

func init() {
externalconn.RegisterConnectionDetailsFromURIFactory(
deprecatedExternalConnectionScheme,
connectionpb.ConnectionProvider_azure_storage,
externalconn.SimpleURIFactory,
)

externalconn.RegisterDefaultValidation(deprecatedExternalConnectionScheme, validateAzureConnectionURI)

for _, s := range []string{scheme, deprecatedScheme, deprecatedExternalConnectionScheme} {
externalconn.RegisterConnectionDetailsFromURIFactory(
s,
connectionpb.ConnectionProvider_azure_storage,
externalconn.SimpleURIFactory,
)

externalconn.RegisterDefaultValidation(s, validateAzureConnectionURI)
}
}
25 changes: 25 additions & 0 deletions pkg/cloud/azure/azure_connection_test.go
@@ -0,0 +1,25 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package azure

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/cloud/externalconn"
"github.com/cockroachdb/cockroach/pkg/cloud/externalconn/connectionpb"
"github.com/stretchr/testify/require"
)

func TestAzureStorageConnection(t *testing.T) {
require.Equal(t, connectionpb.ConnectionProvider_azure_storage, externalconn.ProviderForURI("azure://test"))
require.Equal(t, connectionpb.ConnectionProvider_azure_storage, externalconn.ProviderForURI("azure-storage://test"))
require.Equal(t, connectionpb.ConnectionProvider_azure_storage, externalconn.ProviderForURI("azure-blob://test"))
}
4 changes: 2 additions & 2 deletions pkg/cloud/azure/azure_kms_connection.go
Expand Up @@ -31,12 +31,12 @@ func validateAzureKMSConnectionURI(

func init() {
externalconn.RegisterConnectionDetailsFromURIFactory(
scheme,
kmsScheme,
connectionpb.ConnectionProvider_azure_kms,
externalconn.SimpleURIFactory,
)
externalconn.RegisterDefaultValidation(
scheme,
kmsScheme,
validateAzureKMSConnectionURI,
)
}
23 changes: 23 additions & 0 deletions pkg/cloud/azure/azure_kms_connection_test.go
@@ -0,0 +1,23 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package azure

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/cloud/externalconn"
"github.com/cockroachdb/cockroach/pkg/cloud/externalconn/connectionpb"
"github.com/stretchr/testify/require"
)

func TestAzureKMSConnection(t *testing.T) {
require.Equal(t, connectionpb.ConnectionProvider_azure_kms, externalconn.ProviderForURI("azure-kms://test"))
}
18 changes: 18 additions & 0 deletions pkg/cloud/externalconn/impl_registry.go
Expand Up @@ -140,6 +140,24 @@ func ExternalConnectionFromURI(
return parseAndValidateFn.parseAndValidateURI(ctx, env, externalConnectionURI, defaultValidation)
}

// ProviderForURI returns the provider associated with the scheme of a given URI,
// or UNKNOWN if none found.
// This is useful for testing.
func ProviderForURI(uri string) connectionpb.ConnectionProvider {
externalConnectionURI, err := url.Parse(uri)
if err != nil {
return connectionpb.ConnectionProvider_Unknown
}

// Find the parseAndValidateFn method for the ExternalConnection provider.
parseAndValidateFn, registered := parseAndValidateFns[externalConnectionURI.Scheme]
if !registered {
return connectionpb.ConnectionProvider_Unknown
}

return parseAndValidateFn.ConnectionProvider
}

// ExternalConnEnv contains parameters to be used to validate an external
// connection.
type ExternalConnEnv struct {
Expand Down

0 comments on commit 5cd4c7b

Please sign in to comment.