Skip to content

Commit 9354d98

Browse files
authored
Adding custom cloud configuration while creating Azure credentials
1 parent be44237 commit 9354d98

File tree

3 files changed

+228
-15
lines changed

3 files changed

+228
-15
lines changed

storage_drivers/azure/api/azure.go

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,20 @@ type Client struct {
233233

234234
// ValidateCloudConfiguration validates the cloud configuration and returns a cloud.Configuration.
235235
func ValidateCloudConfiguration(cloudConfig *CloudConfiguration) (*cloud.Configuration, error) {
236+
ctx := context.Background()
237+
238+
// Log input configuration
239+
if cloudConfig == nil {
240+
Logc(ctx).Debug("ValidateCloudConfiguration called with nil cloudConfig, using default AzurePublic.")
241+
} else {
242+
Logc(ctx).WithFields(LogFields{
243+
"cloudName": cloudConfig.CloudName,
244+
"adAuthorityHost": cloudConfig.ADAuthorityHost,
245+
"audience": cloudConfig.Audience,
246+
"endpoint": cloudConfig.Endpoint,
247+
}).Debug("ValidateCloudConfiguration called with cloudConfig.")
248+
}
249+
236250
// If no cloud config provided, use default (AzurePublic)
237251
if cloudConfig == nil {
238252
return &cloud.AzurePublic, nil
@@ -253,16 +267,25 @@ func ValidateCloudConfiguration(cloudConfig *CloudConfiguration) (*cloud.Configu
253267

254268
// Option 1: Named cloud
255269
if hasCloudName {
270+
var namedConfig *cloud.Configuration
256271
switch cloudConfig.CloudName {
257272
case "AzurePublic":
258-
return &cloud.AzurePublic, nil
273+
namedConfig = &cloud.AzurePublic
259274
case "AzureChina":
260-
return &cloud.AzureChina, nil
275+
namedConfig = &cloud.AzureChina
261276
case "AzureGovernment":
262-
return &cloud.AzureGovernment, nil
277+
namedConfig = &cloud.AzureGovernment
263278
default:
264279
return nil, fmt.Errorf("unknown cloudName: %s (valid values: AzurePublic, AzureChina, AzureGovernment)", cloudConfig.CloudName)
265280
}
281+
282+
// Log resulting configuration
283+
Logc(ctx).WithFields(LogFields{
284+
"cloudName": cloudConfig.CloudName,
285+
"activeDirectoryAuthorityHost": namedConfig.ActiveDirectoryAuthorityHost,
286+
}).Debug("ValidateCloudConfiguration returning named cloud configuration.")
287+
288+
return namedConfig, nil
266289
}
267290

268291
// Option 2: Custom configuration - all three fields must be provided
@@ -282,15 +305,24 @@ func ValidateCloudConfiguration(cloudConfig *CloudConfiguration) (*cloud.Configu
282305
}
283306

284307
// Build custom cloud configuration
285-
return &cloud.Configuration{
308+
customConfig := &cloud.Configuration{
286309
ActiveDirectoryAuthorityHost: cloudConfig.ADAuthorityHost,
287310
Services: map[cloud.ServiceName]cloud.ServiceConfiguration{
288311
cloud.ResourceManager: {
289312
Audience: cloudConfig.Audience,
290313
Endpoint: cloudConfig.Endpoint,
291314
},
292315
},
293-
}, nil
316+
}
317+
318+
// Log resulting configuration
319+
Logc(ctx).WithFields(LogFields{
320+
"activeDirectoryAuthorityHost": customConfig.ActiveDirectoryAuthorityHost,
321+
"resourceManagerEndpoint": customConfig.Services[cloud.ResourceManager].Endpoint,
322+
"resourceManagerAudience": customConfig.Services[cloud.ResourceManager].Audience,
323+
}).Debug("ValidateCloudConfiguration returning custom cloud configuration.")
324+
325+
return customConfig, nil
294326
}
295327

296328
// NewDriver is a factory method for creating a new SDK interface.
@@ -308,7 +340,7 @@ func NewDriver(config ClientConfig) (Azure, error) {
308340
return nil, fmt.Errorf("invalid cloud configuration: %v", err)
309341
}
310342

311-
credential, err := GetAzureCredential(config)
343+
credential, err := GetAzureCredential(config, cloudConfig)
312344
if err != nil {
313345
return nil, err
314346
}
@@ -360,16 +392,47 @@ func NewDriver(config ClientConfig) (Azure, error) {
360392
}, nil
361393
}
362394

363-
func GetAzureCredential(config ClientConfig) (credential azcore.TokenCredential, err error) {
395+
// GetAzureCredential creates an Azure credential with the specified cloud configuration.
396+
// The cloudConfig parameter should be a validated cloud configuration from ValidateCloudConfiguration().
397+
// If cloudConfig is nil, defaults to AzurePublic cloud.
398+
func GetAzureCredential(
399+
config ClientConfig, cloudConfig *cloud.Configuration,
400+
) (credential azcore.TokenCredential, err error) {
401+
// Default to AzurePublic if no cloud config provided
402+
if cloudConfig == nil {
403+
cloudConfig = &cloud.AzurePublic
404+
}
405+
364406
armConfig := azclient.ARMClientConfig{
365407
TenantID: config.TenantID,
366408
}
367409

368-
authProvider, err := azclient.NewAuthProvider(&armConfig, &config.AzureAuthConfig)
410+
// Set cloud-specific ARM configuration
411+
// For named clouds: use CloudName (e.g., "AzurePublic", "AzureChina", "AzureGovernment")
412+
// For custom clouds: use ResourceManagerEndpoint to let azclient build custom configuration
413+
if config.CloudConfig != nil {
414+
if config.CloudConfig.CloudName != "" {
415+
armConfig.Cloud = config.CloudConfig.CloudName
416+
} else if config.CloudConfig.Endpoint != "" {
417+
armConfig.ResourceManagerEndpoint = config.CloudConfig.Endpoint
418+
}
419+
}
420+
421+
// Create auth provider with cloud-aware client options
422+
// The cloud configuration must be set before credentials are created to ensure
423+
// the correct authority host is used (e.g., login.microsoftonline.us for Azure Government)
424+
authProviderOptions := []azclient.AuthProviderOption{
425+
azclient.WithClientOptionsMutFn(func(option *policy.ClientOptions) {
426+
option.Cloud = *cloudConfig
427+
}),
428+
}
429+
430+
authProvider, err := azclient.NewAuthProvider(&armConfig, &config.AzureAuthConfig, authProviderOptions...)
369431
if err != nil {
370432
return nil, fmt.Errorf("error creating azure auth provider: %w", err)
371433
}
372434

435+
// GetAzIdentity may return nil in test environments without actual credentials
373436
return authProvider.GetAzIdentity(), nil
374437
}
375438

storage_drivers/azure/api/azure_test.go

Lines changed: 151 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import (
1010
"testing"
1111

1212
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
13-
1413
"github.com/stretchr/testify/assert"
14+
"sigs.k8s.io/cloud-provider-azure/pkg/azclient"
1515

1616
"github.com/netapp/trident/utils/errors"
1717
)
@@ -1324,3 +1324,153 @@ func TestValidateCloudConfiguration_InvalidURL(t *testing.T) {
13241324
})
13251325
}
13261326
}
1327+
1328+
// ////////////////////////////////////////////////////////////////////////////
1329+
// Tests for GetAzureCredential
1330+
// ////////////////////////////////////////////////////////////////////////////
1331+
1332+
// Helper function to create a test ClientConfig
1333+
func createTestClientConfig(cloudConfig *CloudConfiguration) ClientConfig {
1334+
return ClientConfig{
1335+
TenantID: "test-tenant-id",
1336+
CloudConfig: cloudConfig,
1337+
AzureAuthConfig: azclient.AzureAuthConfig{},
1338+
}
1339+
}
1340+
1341+
func TestGetAzureCredential_NoCloudConfig(t *testing.T) {
1342+
// Test with nil cloud config - should default to AzurePublic
1343+
config := createTestClientConfig(nil)
1344+
1345+
// Validate cloud configuration
1346+
cloudConfig, err := ValidateCloudConfiguration(config.CloudConfig)
1347+
assert.NoError(t, err, "ValidateCloudConfiguration should not error with nil config")
1348+
1349+
_, err = GetAzureCredential(config, cloudConfig)
1350+
// Since we don't have actual Azure credentials, we expect this to succeed in creating
1351+
// the auth provider structure, even though it won't return a valid credential
1352+
// The important thing is it doesn't error on cloud configuration validation
1353+
assert.NoError(t, err, "GetAzureCredential should not error with nil cloud config")
1354+
}
1355+
1356+
func TestGetAzureCredential_EmptyCloudConfig(t *testing.T) {
1357+
// Test with empty cloud config - should default to AzurePublic
1358+
config := createTestClientConfig(&CloudConfiguration{})
1359+
1360+
// Validate cloud configuration
1361+
cloudConfig, err := ValidateCloudConfiguration(config.CloudConfig)
1362+
assert.NoError(t, err, "ValidateCloudConfiguration should not error with empty config")
1363+
1364+
_, err = GetAzureCredential(config, cloudConfig)
1365+
assert.NoError(t, err, "GetAzureCredential should not error with empty cloud config")
1366+
}
1367+
1368+
func TestGetAzureCredential_NamedClouds(t *testing.T) {
1369+
tests := []struct {
1370+
name string
1371+
cloudName string
1372+
}{
1373+
{
1374+
name: "AzurePublic",
1375+
cloudName: "AzurePublic",
1376+
},
1377+
{
1378+
name: "AzureChina",
1379+
cloudName: "AzureChina",
1380+
},
1381+
{
1382+
name: "AzureGovernment",
1383+
cloudName: "AzureGovernment",
1384+
},
1385+
}
1386+
1387+
for _, tt := range tests {
1388+
t.Run(tt.name, func(t *testing.T) {
1389+
config := createTestClientConfig(&CloudConfiguration{
1390+
CloudName: tt.cloudName,
1391+
})
1392+
1393+
// Validate cloud configuration
1394+
cloudConfig, err := ValidateCloudConfiguration(config.CloudConfig)
1395+
assert.NoError(t, err, "ValidateCloudConfiguration should not error with valid cloud name: %s", tt.cloudName)
1396+
1397+
_, err = GetAzureCredential(config, cloudConfig)
1398+
assert.NoError(t, err, "GetAzureCredential should not error with valid named cloud: %s", tt.cloudName)
1399+
})
1400+
}
1401+
}
1402+
1403+
func TestGetAzureCredential_CustomCloud(t *testing.T) {
1404+
config := createTestClientConfig(&CloudConfiguration{
1405+
ADAuthorityHost: "https://login.custom.cloud/",
1406+
Audience: "https://management.custom.cloud",
1407+
Endpoint: "https://management.custom.cloud",
1408+
})
1409+
1410+
// Validate cloud configuration
1411+
cloudConfig, err := ValidateCloudConfiguration(config.CloudConfig)
1412+
assert.NoError(t, err, "ValidateCloudConfiguration should not error with valid custom config")
1413+
1414+
// Note: This test will fail with a network error because the custom cloud URL is fake.
1415+
// This is expected behavior - validation of custom cloud URLs happens when the auth
1416+
// provider attempts to connect to the endpoint.
1417+
_, err = GetAzureCredential(config, cloudConfig)
1418+
// We expect an error due to network failure, not a validation error
1419+
assert.Error(t, err, "GetAzureCredential should error when custom cloud endpoint is unreachable")
1420+
assert.Contains(t, err.Error(), "error creating azure auth provider")
1421+
}
1422+
1423+
func TestGetAzureCredential_InvalidCloudName(t *testing.T) {
1424+
config := createTestClientConfig(&CloudConfiguration{
1425+
CloudName: "InvalidCloudName",
1426+
})
1427+
1428+
// Validate cloud configuration - should fail for invalid cloud name
1429+
_, err := ValidateCloudConfiguration(config.CloudConfig)
1430+
assert.Error(t, err, "ValidateCloudConfiguration should error with invalid cloud name")
1431+
assert.Contains(t, err.Error(), "unknown cloudName")
1432+
}
1433+
1434+
func TestGetAzureCredential_IncompleteCustomConfig(t *testing.T) {
1435+
config := createTestClientConfig(&CloudConfiguration{
1436+
ADAuthorityHost: "https://login.custom.cloud/",
1437+
Audience: "https://management.custom.cloud",
1438+
// Missing Endpoint - should cause validation error
1439+
})
1440+
1441+
// Validate cloud configuration - should fail for incomplete custom config
1442+
_, err := ValidateCloudConfiguration(config.CloudConfig)
1443+
assert.Error(t, err, "ValidateCloudConfiguration should error with incomplete custom config")
1444+
assert.Contains(t, err.Error(), "adAuthorityHost, audience, and endpoint are all required")
1445+
}
1446+
1447+
func TestGetAzureCredential_MutuallyExclusiveConfig(t *testing.T) {
1448+
config := createTestClientConfig(&CloudConfiguration{
1449+
CloudName: "AzurePublic",
1450+
ADAuthorityHost: "https://login.custom.cloud/",
1451+
Audience: "https://management.custom.cloud",
1452+
Endpoint: "https://management.custom.cloud",
1453+
})
1454+
1455+
// Validate cloud configuration - should fail for mutually exclusive config
1456+
_, err := ValidateCloudConfiguration(config.CloudConfig)
1457+
assert.Error(t, err, "ValidateCloudConfiguration should error with mutually exclusive config")
1458+
assert.Contains(t, err.Error(), "mutually exclusive")
1459+
}
1460+
1461+
func TestGetAzureCredential_InvalidURLInCustomConfig(t *testing.T) {
1462+
config := createTestClientConfig(&CloudConfiguration{
1463+
ADAuthorityHost: "https://login.custom.cloud/",
1464+
Audience: "https://management.custom.cloud",
1465+
Endpoint: "https://management.custom.cloud",
1466+
})
1467+
1468+
// Validate cloud configuration - should succeed (url.Parse is permissive)
1469+
cloudConfig, err := ValidateCloudConfiguration(config.CloudConfig)
1470+
assert.NoError(t, err, "ValidateCloudConfiguration accepts syntactically valid URLs")
1471+
1472+
// The actual network error will occur when trying to use the credential
1473+
_, err = GetAzureCredential(config, cloudConfig)
1474+
assert.Error(t, err, "GetAzureCredential should error when custom cloud endpoint is unreachable")
1475+
assert.Contains(t, err.Error(), "error creating azure auth provider")
1476+
}

storage_drivers/azure/azure_anf_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,9 +1190,9 @@ func TestInitialize_WithCloudConfigurationCustom(t *testing.T) {
11901190
"clientID": "deadbeef-784c-4b35-8329-460f52a3ad50",
11911191
"clientSecret": "myClientSecret",
11921192
"cloudConfiguration": {
1193-
"adAuthorityHost": "https://login.microsoftonline.azurestack.contoso.com/",
1194-
"audience": "https://management.azurestack.contoso.com",
1195-
"endpoint": "https://management.azurestack.contoso.com"
1193+
"adAuthorityHost": "https://login.microsoftonline.com/",
1194+
"audience": "https://management.core.windows.net/",
1195+
"endpoint": "https://management.azure.com"
11961196
},
11971197
"serviceLevel": "Premium",
11981198
"debugTraceFlags": {"method": true, "api": true, "discovery": true},
@@ -1217,9 +1217,9 @@ func TestInitialize_WithCloudConfigurationCustom(t *testing.T) {
12171217

12181218
assert.NoError(t, result, "initialize failed")
12191219
assert.NotNil(t, driver.Config.CloudConfiguration, "cloud configuration is nil")
1220-
assert.Equal(t, "https://login.microsoftonline.azurestack.contoso.com/", driver.Config.CloudConfiguration.ADAuthorityHost)
1221-
assert.Equal(t, "https://management.azurestack.contoso.com", driver.Config.CloudConfiguration.Audience)
1222-
assert.Equal(t, "https://management.azurestack.contoso.com", driver.Config.CloudConfiguration.Endpoint)
1220+
assert.Equal(t, "https://login.microsoftonline.com/", driver.Config.CloudConfiguration.ADAuthorityHost)
1221+
assert.Equal(t, "https://management.core.windows.net/", driver.Config.CloudConfiguration.Audience)
1222+
assert.Equal(t, "https://management.azure.com", driver.Config.CloudConfiguration.Endpoint)
12231223
}
12241224

12251225
func TestInitialize_WithCloudConfigurationInvalid(t *testing.T) {

0 commit comments

Comments
 (0)