feat: accept repository-style Azure keys in repo-creds (#29414)

Project: argoproj/argo-cd | Language: Go | Concepts: backwards-compatibility, api-design, testing-strategy | Difficulty: mid
Link: https://github.com/argoproj/argo-cd/pull/29415

The problem

Argo CD supports storing Azure credentials in Kubernetes Secrets for both repositories (repository Secrets) and repository credentials (repo-creds Secrets). However, the two types of Secrets used different key names for Azure client and tenant IDs: repository Secrets accepted both ID and Id spellings (e.g., azureServicePrincipalClientID and azureServicePrincipalClientId), but repo-creds Secrets only accepted the uppercase ID variant. This inconsistency meant the same Azure credentials would work in one Secret type but not the other, causing confusion and deployment failures.

Why it's non-trivial

A naive fix might simply accept both variants everywhere, but that could cause subtle bugs if both keys are present with different values. The system needs to define which key takes precedence to avoid ambiguous or unpredictable behavior. Additionally, the change must not break existing configurations and should be clearly documented and tested.

The approach

1. Accept both key spellings, but preserve precedence.

The core logic change is in secretToRepoCred:

+	azureServicePrincipalClientID := secretCopy.Data["azureServicePrincipalClientID"]
+	if len(azureServicePrincipalClientID) == 0 {
+		azureServicePrincipalClientID = secretCopy.Data["azureServicePrincipalClientId"]
+	}
+	azureServicePrincipalTenantID := secretCopy.Data["azureServicePrincipalTenantID"]
+	if len(azureServicePrincipalTenantID) == 0 {
+		azureServicePrincipalTenantID = secretCopy.Data["azureServicePrincipalTenantId"]
+	}

What this does:

• Tries to read the uppercase ID key first (azureServicePrincipalClientID).
• If that key is missing or empty, falls back to the lowercase Id alias (azureServicePrincipalClientId).
• The same logic applies to the tenant ID.

Why:
This preserves backward compatibility (existing uppercase-ID keys win), but allows new configs to use either spelling.

2. Use the resolved values in the credentials struct.

-		AzureServicePrincipalClientId:     string(secretCopy.Data["azureServicePrincipalClientID"]),
+		AzureServicePrincipalClientId:     string(azureServicePrincipalClientID),
...
-		AzureServicePrincipalTenantId:     string(secretCopy.Data["azureServicePrincipalTenantID"]),
+		AzureServicePrincipalTenantId:     string(azureServicePrincipalTenantID),

What this does:

• Ensures the struct fields receive the resolved value, regardless of which key spelling was used.

3. Add explicit tests for aliasing and precedence.

func TestSecretToRepoCred_AzureServicePrincipalKeyAliases(t *testing.T) {
	tests := []struct {
		name     string
		data     map[string][]byte
		clientID string
		tenantID string
	}{
		{
			name: "repository-style aliases",
			data: map[string][]byte{
				"azureServicePrincipalClientId": []byte("alias-client"),
				"azureServicePrincipalTenantId": []byte("alias-tenant"),
			},
			clientID: "alias-client",
			tenantID: "alias-tenant",
		},
		{
			name: "existing keys take precedence",
			data: map[string][]byte{
				"azureServicePrincipalClientID": []byte("existing-client"),
				"azureServicePrincipalClientId": []byte("alias-client"),
				"azureServicePrincipalTenantID": []byte("existing-tenant"),
				"azureServicePrincipalTenantId": []byte("alias-tenant"),
			},
			clientID: "existing-client",
			tenantID: "existing-tenant",
		},
	}
	...
}

What this does:

• Verifies that the fallback logic works (aliases are accepted).
• Confirms that uppercase-ID keys take precedence when both are present.

4. Update documentation to clarify accepted keys and precedence.

+* `azureServicePrincipalClientID` or `azureServicePrincipalClientId` refers to the client ID.
+* `azureServicePrincipalTenantID` or `azureServicePrincipalTenantId` refers to the tenant ID.
...
+> [!NOTE]
+> If both forms of a client or tenant ID key have values, the uppercase-`ID` form takes precedence.

What this does:

• Documents the accepted key names and the precedence rule for users.

From the review thread

No notable review discussion on this PR.

Takeaways

• When introducing support for multiple key aliases, always define and enforce a clear precedence to avoid ambiguity.
• Backwards compatibility should be preserved by prioritizing existing key names or behaviors.
• Changes to configuration parsing should be accompanied by explicit tests for all supported variants and edge cases.
• Documentation should be updated to reflect any new accepted inputs and the rules governing them.