Skip to content

Commit d13e4c3

Browse files
dbanckmagodo
dbanck
and
magodo
authored
Backend/azure: subscription_id infer from Azure CLI & a way to skip *unnecessary* management plane API call (#36623) (#36677)
* backend/azure: Infer `subscription_id` from Azure CLI if not specified * Avoid management plane call if not necessary There are four auth scenarios of the blob/container: 1. User specifies the SAS token 2. User specifies the shared access key 3. User specified to use AAD auth (and credential provided) 4. None of the above, management plane API call needed to list the shared access key and use it to auth For 1, 2 and 3, the management plane API can be skipped in most of the cases, except the target storage account is using private DNS zone. The blob/container data plane client requires the user to specify the base URI, which is not deterministic if the storage account opt in the private DNS zone. In this case, an additional management plane `GET` is required against the storage account, to retrieve the blob endpoint. While if private DNS zone is not used, the base URI can be composed in a fixed pattern, with the storage account name and container&blob name. Hence no management plane API call is needed. The user is expected to use the `subscription_id` and `resource_group_name` to indicate the above intent: Only if both are specified, the additional `GET` call will be invoked to get the accurate blob endpoint. (NOTE: the `subscription_id` can be inferred from the Azure CLI if unspecified) * No need for AAD auth if shared key or sas is specified * update * Add test * update comments * changelog Co-authored-by: magodo <[email protected]>
1 parent 2d42aed commit d13e4c3

File tree

5 files changed

+183
-138
lines changed

5 files changed

+183
-138
lines changed

‎.changes/v1.11/BUG FIXES-20250311-104640.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
kind: BUG FIXES
2+
body: 'Backend/azure: `subscription_id` be optional & skip *unnecessary* management plane API call in some setup'
3+
time: 2025-03-11T10:46:40.000000+11:00
4+
custom:
5+
Issue: "36595"

‎internal/backend/remote-state/azure/api_client.go

+126-131
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,21 @@ import (
2323
)
2424

2525
type Client struct {
26-
// These Clients are only initialized if an Access Key isn't provided
26+
environment environments.Environment
27+
storageAccountName string
28+
29+
// Storage ARM client is used for looking up the blob endpoint, or/and listing access key (if not specified).
2730
storageAccountsClient *storageaccounts.StorageAccountsClient
31+
// This is only non-nil if the config has specified to lookup the blob endpoint
32+
accountDetail *AccountDetails
2833

2934
// Caching
3035
containersClient *containers.Client
3136
blobsClient *blobs.Client
3237

33-
environment environments.Environment
34-
storageAccountName string
35-
36-
accountDetail *AccountDetails
37-
38-
accessKey string
39-
sasToken string
40-
// azureAdStorageAuth is only here if we're using AzureAD Authentication but is an Authorizer for Storage
38+
// Only one of them shall be specified
39+
accessKey string
40+
sasToken string
4141
azureAdStorageAuth auth.Authorizer
4242
}
4343

@@ -47,54 +47,71 @@ func buildClient(ctx context.Context, config BackendConfig) (*Client, error) {
4747
storageAccountName: config.StorageAccountName,
4848
}
4949

50-
// if we have an Access Key - we don't need the other clients
51-
if config.AccessKey != "" {
50+
var armAuthRequired bool
51+
switch {
52+
case config.AccessKey != "":
5253
client.accessKey = config.AccessKey
53-
return &client, nil
54-
}
55-
56-
// likewise with a SAS token
57-
if config.SasToken != "" {
54+
case config.SasToken != "":
5855
sasToken := config.SasToken
5956
if strings.TrimSpace(sasToken) == "" {
6057
return nil, fmt.Errorf("sasToken cannot be empty")
6158
}
6259
client.sasToken = strings.TrimPrefix(sasToken, "?")
63-
64-
return &client, nil
65-
}
66-
67-
if config.UseAzureADAuthentication {
60+
case config.UseAzureADAuthentication:
6861
var err error
6962
client.azureAdStorageAuth, err = auth.NewAuthorizerFromCredentials(ctx, *config.AuthConfig, config.AuthConfig.Environment.Storage)
7063
if err != nil {
7164
return nil, fmt.Errorf("unable to build authorizer for Storage API: %+v", err)
7265
}
66+
default:
67+
// AAD authentication (ARM scope) is required only when no auth method is specified, which falls back to listing the access key via ARM API.
68+
armAuthRequired = true
7369
}
7470

75-
resourceManagerAuth, err := auth.NewAuthorizerFromCredentials(ctx, *config.AuthConfig, config.AuthConfig.Environment.ResourceManager)
76-
if err != nil {
77-
return nil, fmt.Errorf("unable to build authorizer for Resource Manager API: %+v", err)
71+
// If `config.LookupBlobEndpoint` is true, we need to authenticate with ARM to lookup the blob endpoint
72+
if config.LookupBlobEndpoint {
73+
armAuthRequired = true
7874
}
7975

80-
client.storageAccountsClient, err = storageaccounts.NewStorageAccountsClientWithBaseURI(config.AuthConfig.Environment.ResourceManager)
81-
if err != nil {
82-
return nil, fmt.Errorf("building Storage Accounts client: %+v", err)
83-
}
84-
client.configureClient(client.storageAccountsClient.Client, resourceManagerAuth)
76+
if armAuthRequired {
77+
resourceManagerAuth, err := auth.NewAuthorizerFromCredentials(ctx, *config.AuthConfig, config.AuthConfig.Environment.ResourceManager)
78+
if err != nil {
79+
return nil, fmt.Errorf("unable to build authorizer for Resource Manager API: %+v", err)
80+
}
8581

86-
// Populating the storage account detail
87-
storageAccountId := commonids.NewStorageAccountID(config.SubscriptionID, config.ResourceGroupName, client.storageAccountName)
88-
resp, err := client.storageAccountsClient.GetProperties(ctx, storageAccountId, storageaccounts.DefaultGetPropertiesOperationOptions())
89-
if err != nil {
90-
return nil, fmt.Errorf("retrieving %s: %+v", storageAccountId, err)
91-
}
92-
if resp.Model == nil {
93-
return nil, fmt.Errorf("retrieving %s: model was nil", storageAccountId)
94-
}
95-
client.accountDetail, err = populateAccountDetails(storageAccountId, *resp.Model)
96-
if err != nil {
97-
return nil, fmt.Errorf("populating details for %s: %+v", storageAccountId, err)
82+
// When using Azure CLI to auth, the user can leave the "subscription_id" unspecified. In this case the subscription id is inferred from
83+
// the Azure CLI default subscription.
84+
if config.SubscriptionID == "" {
85+
if cachedAuth, ok := resourceManagerAuth.(*auth.CachedAuthorizer); ok {
86+
if cliAuth, ok := cachedAuth.Source.(*auth.AzureCliAuthorizer); ok && cliAuth.DefaultSubscriptionID != "" {
87+
config.SubscriptionID = cliAuth.DefaultSubscriptionID
88+
}
89+
}
90+
}
91+
if config.SubscriptionID == "" {
92+
return nil, fmt.Errorf("subscription id not specified")
93+
}
94+
95+
// Setup the SA client.
96+
client.storageAccountsClient, err = storageaccounts.NewStorageAccountsClientWithBaseURI(config.AuthConfig.Environment.ResourceManager)
97+
if err != nil {
98+
return nil, fmt.Errorf("building Storage Accounts client: %+v", err)
99+
}
100+
client.configureClient(client.storageAccountsClient.Client, resourceManagerAuth)
101+
102+
// Populating the storage account detail
103+
storageAccountId := commonids.NewStorageAccountID(config.SubscriptionID, config.ResourceGroupName, client.storageAccountName)
104+
resp, err := client.storageAccountsClient.GetProperties(ctx, storageAccountId, storageaccounts.DefaultGetPropertiesOperationOptions())
105+
if err != nil {
106+
return nil, fmt.Errorf("retrieving %s: %+v", storageAccountId, err)
107+
}
108+
if resp.Model == nil {
109+
return nil, fmt.Errorf("retrieving %s: model was nil", storageAccountId)
110+
}
111+
client.accountDetail, err = populateAccountDetails(storageAccountId, *resp.Model)
112+
if err != nil {
113+
return nil, fmt.Errorf("populating details for %s: %+v", storageAccountId, err)
114+
}
98115
}
99116

100117
return &client, nil
@@ -111,16 +128,29 @@ func (c *Client) getBlobClient(ctx context.Context) (bc *blobs.Client, err error
111128
}
112129
}()
113130

114-
if c.sasToken != "" {
115-
log.Printf("[DEBUG] Building the Blob Client from a SAS Token")
116-
baseURL, err := naiveStorageAccountBlobBaseURL(c.environment, c.storageAccountName)
131+
var baseUri string
132+
if c.accountDetail != nil {
133+
// Use the actual blob endpoint if available
134+
pBaseUri, err := c.accountDetail.DataPlaneEndpoint(EndpointTypeBlob)
117135
if err != nil {
118-
return nil, fmt.Errorf("build storage account blob base URL: %v", err)
136+
return nil, err
119137
}
120-
blobsClient, err := blobs.NewWithBaseUri(baseURL)
138+
baseUri = *pBaseUri
139+
} else {
140+
baseUri, err = naiveStorageAccountBlobBaseURL(c.environment, c.storageAccountName)
121141
if err != nil {
122-
return nil, fmt.Errorf("new blob client: %v", err)
142+
return nil, err
123143
}
144+
}
145+
146+
blobsClient, err := blobs.NewWithBaseUri(baseUri)
147+
if err != nil {
148+
return nil, fmt.Errorf("new blob client: %v", err)
149+
}
150+
151+
switch {
152+
case c.sasToken != "":
153+
log.Printf("[DEBUG] Building the Blob Client from a SAS Token")
124154
c.configureClient(blobsClient.Client, nil)
125155
blobsClient.Client.AppendRequestMiddleware(func(r *http.Request) (*http.Request, error) {
126156
if r.URL.RawQuery == "" {
@@ -131,59 +161,35 @@ func (c *Client) getBlobClient(ctx context.Context) (bc *blobs.Client, err error
131161
return r, nil
132162
})
133163
return blobsClient, nil
134-
}
135164

136-
if c.accessKey != "" {
165+
case c.accessKey != "":
137166
log.Printf("[DEBUG] Building the Blob Client from an Access Key")
138-
baseURL, err := naiveStorageAccountBlobBaseURL(c.environment, c.storageAccountName)
139-
if err != nil {
140-
return nil, fmt.Errorf("build storage account blob base URL: %v", err)
141-
}
142-
blobsClient, err := blobs.NewWithBaseUri(baseURL)
143-
if err != nil {
144-
return nil, fmt.Errorf("new blob client: %v", err)
145-
}
146-
c.configureClient(blobsClient.Client, nil)
147-
148167
authorizer, err := auth.NewSharedKeyAuthorizer(c.storageAccountName, c.accessKey, auth.SharedKey)
149168
if err != nil {
150169
return nil, fmt.Errorf("new shared key authorizer: %v", err)
151170
}
152171
c.configureClient(blobsClient.Client, authorizer)
153-
154172
return blobsClient, nil
155-
}
156173

157-
// Neither shared access key nor sas token specified, then we have the storage account details populated.
158-
// This detail can be used to get the "most" correct blob endpoint comparing to the naive construction.
159-
baseUri, err := c.accountDetail.DataPlaneEndpoint(EndpointTypeBlob)
160-
if err != nil {
161-
return nil, err
162-
}
163-
blobsClient, err := blobs.NewWithBaseUri(*baseUri)
164-
if err != nil {
165-
return nil, fmt.Errorf("new blob client: %v", err)
166-
}
167-
168-
if c.azureAdStorageAuth != nil {
174+
case c.azureAdStorageAuth != nil:
169175
log.Printf("[DEBUG] Building the Blob Client from AAD auth")
170176
c.configureClient(blobsClient.Client, c.azureAdStorageAuth)
171177
return blobsClient, nil
172-
}
173-
174-
log.Printf("[DEBUG] Building the Blob Client from an Access Token (using user credentials)")
175-
key, err := c.accountDetail.AccountKey(ctx, c.storageAccountsClient)
176-
if err != nil {
177-
return nil, fmt.Errorf("retrieving key for Storage Account %q: %s", c.storageAccountName, err)
178-
}
179178

180-
authorizer, err := auth.NewSharedKeyAuthorizer(c.storageAccountName, *key, auth.SharedKey)
181-
if err != nil {
182-
return nil, fmt.Errorf("new shared key authorizer: %v", err)
179+
default:
180+
// Neither shared access key, sas token, or AAD Auth were specified so we have to call the management plane API to get the key.
181+
log.Printf("[DEBUG] Building the Blob Client from an Access Key (key is listed using client credentials)")
182+
key, err := c.accountDetail.AccountKey(ctx, c.storageAccountsClient)
183+
if err != nil {
184+
return nil, fmt.Errorf("retrieving key for Storage Account %q: %s", c.storageAccountName, err)
185+
}
186+
authorizer, err := auth.NewSharedKeyAuthorizer(c.storageAccountName, *key, auth.SharedKey)
187+
if err != nil {
188+
return nil, fmt.Errorf("new shared key authorizer: %v", err)
189+
}
190+
c.configureClient(blobsClient.Client, authorizer)
191+
return blobsClient, nil
183192
}
184-
c.configureClient(blobsClient.Client, authorizer)
185-
186-
return blobsClient, nil
187193
}
188194

189195
func (c *Client) getContainersClient(ctx context.Context) (cc *containers.Client, err error) {
@@ -197,16 +203,29 @@ func (c *Client) getContainersClient(ctx context.Context) (cc *containers.Client
197203
}
198204
}()
199205

200-
if c.sasToken != "" {
201-
log.Printf("[DEBUG] Building the Container Client from a SAS Token")
202-
baseURL, err := naiveStorageAccountBlobBaseURL(c.environment, c.storageAccountName)
206+
var baseUri string
207+
if c.accountDetail != nil {
208+
// Use the actual blob endpoint if available
209+
pBaseUri, err := c.accountDetail.DataPlaneEndpoint(EndpointTypeBlob)
203210
if err != nil {
204-
return nil, fmt.Errorf("build storage account blob base URL: %v", err)
211+
return nil, err
205212
}
206-
containersClient, err := containers.NewWithBaseUri(baseURL)
213+
baseUri = *pBaseUri
214+
} else {
215+
baseUri, err = naiveStorageAccountBlobBaseURL(c.environment, c.storageAccountName)
207216
if err != nil {
208-
return nil, fmt.Errorf("new container client: %v", err)
217+
return nil, err
209218
}
219+
}
220+
221+
containersClient, err := containers.NewWithBaseUri(baseUri)
222+
if err != nil {
223+
return nil, fmt.Errorf("new container client: %v", err)
224+
}
225+
226+
switch {
227+
case c.sasToken != "":
228+
log.Printf("[DEBUG] Building the Container Client from a SAS Token")
210229
c.configureClient(containersClient.Client, nil)
211230
containersClient.Client.AppendRequestMiddleware(func(r *http.Request) (*http.Request, error) {
212231
if r.URL.RawQuery == "" {
@@ -217,59 +236,35 @@ func (c *Client) getContainersClient(ctx context.Context) (cc *containers.Client
217236
return r, nil
218237
})
219238
return containersClient, nil
220-
}
221239

222-
if c.accessKey != "" {
240+
case c.accessKey != "":
223241
log.Printf("[DEBUG] Building the Container Client from an Access Key")
224-
baseURL, err := naiveStorageAccountBlobBaseURL(c.environment, c.storageAccountName)
225-
if err != nil {
226-
return nil, fmt.Errorf("build storage account blob base URL: %v", err)
227-
}
228-
containersClient, err := containers.NewWithBaseUri(baseURL)
229-
if err != nil {
230-
return nil, fmt.Errorf("new container client: %v", err)
231-
}
232-
c.configureClient(containersClient.Client, nil)
233-
234242
authorizer, err := auth.NewSharedKeyAuthorizer(c.storageAccountName, c.accessKey, auth.SharedKey)
235243
if err != nil {
236244
return nil, fmt.Errorf("new shared key authorizer: %v", err)
237245
}
238246
c.configureClient(containersClient.Client, authorizer)
239-
240247
return containersClient, nil
241-
}
242-
243-
// Neither shared access key nor sas token specified, then we have the storage account details populated.
244-
// This detail can be used to get the "most" correct blob endpoint comparing to the naive construction.
245-
baseUri, err := c.accountDetail.DataPlaneEndpoint(EndpointTypeBlob)
246-
if err != nil {
247-
return nil, err
248-
}
249-
containersClient, err := containers.NewWithBaseUri(*baseUri)
250-
if err != nil {
251-
return nil, fmt.Errorf("new container client: %v", err)
252-
}
253248

254-
if c.azureAdStorageAuth != nil {
249+
case c.azureAdStorageAuth != nil:
255250
log.Printf("[DEBUG] Building the Container Client from AAD auth")
256251
c.configureClient(containersClient.Client, c.azureAdStorageAuth)
257252
return containersClient, nil
258-
}
259-
260-
log.Printf("[DEBUG] Building the Container Client from an Access Token (using user credentials)")
261-
key, err := c.accountDetail.AccountKey(ctx, c.storageAccountsClient)
262-
if err != nil {
263-
return nil, fmt.Errorf("retrieving key for Storage Account %q: %s", c.storageAccountName, err)
264-
}
265253

266-
authorizer, err := auth.NewSharedKeyAuthorizer(c.storageAccountName, *key, auth.SharedKey)
267-
if err != nil {
268-
return nil, fmt.Errorf("new shared key authorizer: %v", err)
254+
default:
255+
// Neither shared access key, sas token, or AAD Auth were specified so we have to call the management plane API to get the key.
256+
log.Printf("[DEBUG] Building the Container Client from an Access Key (key is listed using user credentials)")
257+
key, err := c.accountDetail.AccountKey(ctx, c.storageAccountsClient)
258+
if err != nil {
259+
return nil, fmt.Errorf("retrieving key for Storage Account %q: %s", c.storageAccountName, err)
260+
}
261+
authorizer, err := auth.NewSharedKeyAuthorizer(c.storageAccountName, *key, auth.SharedKey)
262+
if err != nil {
263+
return nil, fmt.Errorf("new shared key authorizer: %v", err)
264+
}
265+
c.configureClient(containersClient.Client, authorizer)
266+
return containersClient, nil
269267
}
270-
c.configureClient(containersClient.Client, authorizer)
271-
272-
return containersClient, nil
273268
}
274269

275270
func (c *Client) configureClient(client client.BaseClient, authorizer auth.Authorizer) {

0 commit comments

Comments
 (0)