Also add client_id to make it more explicit

Signed-off-by: DylanGuedes <djmgguedes@gmail.com>
thanos-io · Nov 21, 2024 · 41f4bec · 41f4bec
1 parent 10991db
commit 41f4bec
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,7 +11,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 ## Unreleased
 - [#38](https://github.com/thanos-io/objstore/pull/38) GCS: Upgrade cloud.google.com/go/storage version to `v1.43.0`.
 - [#145](https://github.com/thanos-io/objstore/pull/145) Include content length in the response of Get and GetRange.
-- [#157](https://github.com/thanos-io/objstore/pull/157) Azure: Add `tenant_id` and `client_secret` configs.
+- [#157](https://github.com/thanos-io/objstore/pull/157) Azure: Add `tenant_id`, `client_id` and `client_secret` configs.
 
 ### Fixed
 - [#153](https://github.com/thanos-io/objstore/pull/153) Metrics: Fix `objstore_bucket_operation_duration_seconds_*` for `get` and `get_range` operations.

diff --git a/providers/azure/azure.go b/providers/azure/azure.go
@@ -46,6 +46,7 @@ var DefaultConfig = Config{
 
 // Config Azure storage configuration.
 type Config struct {
+	ClientID                string             `yaml:"client_id"`
 	ClientSecret            string             `yaml:"client_secret"`
 	TenantID                string             `yaml:"tenant_id"`
 	StorageAccountName      string             `yaml:"storage_account"`
@@ -86,8 +87,12 @@ func (conf *Config) validate() error {
 		errMsg = append(errMsg, "user_assigned_id cannot be set when using storage_connection_string authentication")
 	}
 
-	if (conf.TenantID != "" || conf.ClientSecret != "") && (conf.TenantID == "" || conf.ClientSecret == "") {
-		errMsg = append(errMsg, "tenant_id, user_assigned_id, and client_secret must be set together")
+	if conf.UserAssignedID != "" && conf.ClientID != "" {
+		errMsg = append(errMsg, "user_assigned_id cannot be set when using client_id authentication")
+	}
+
+	if (conf.TenantID != "" || conf.ClientSecret != "" || conf.ClientID != "") && (conf.TenantID == "" || conf.ClientSecret == "" || conf.ClientID == "") {
+		errMsg = append(errMsg, "tenant_id, client_id, and client_secret must be set together")
 	}
 
 	if conf.StorageAccountKey != "" && conf.StorageConnectionString != "" {

diff --git a/providers/azure/azure_test.go b/providers/azure/azure_test.go
@@ -149,7 +149,7 @@ container: "MyContainer"`),
 		config: []byte(`storage_account: "myAccount"
 storage_account_key: ""
 tenant_id: "1234-56578678-655"
-user_assigned_id: "1234-56578678-655"
+client_id: "1234-56578678-655"
 client_secret: "1234-56578678-655"
 container: "MyContainer"`),
 		wantFailParse:    false,
@@ -159,7 +159,7 @@ container: "MyContainer"`),
 		name: "Valid ClientID and ClientSecret but missing TenantID",
 		config: []byte(`storage_account: "myAccount"
 storage_account_key: ""
-user_assigned_id: "1234-56578678-655"
+client_id: "1234-56578678-655"
 client_secret: "1234-56578678-655"
 container: "MyContainer"`),
 		wantFailParse:    false,

diff --git a/providers/azure/helpers.go b/providers/azure/helpers.go
@@ -89,8 +89,8 @@ func getTokenCredential(conf Config) (azcore.TokenCredential, error) {
 		return azidentity.NewDefaultAzureCredential(nil)
 	}
 
-	if conf.ClientSecret != "" && conf.TenantID != "" {
-		return azidentity.NewClientSecretCredential(conf.TenantID, conf.UserAssignedID, conf.ClientSecret, &azidentity.ClientSecretCredentialOptions{})
+	if conf.ClientSecret != "" && conf.TenantID != "" && conf.ClientID != "" {
+		return azidentity.NewClientSecretCredential(conf.TenantID, conf.ClientID, conf.ClientSecret, &azidentity.ClientSecretCredentialOptions{})
 	}
 
 	msiOpt := &azidentity.ManagedIdentityCredentialOptions{}