Different BSL credentials may conflict when deploying on Azure

[ywk253100]

Currently, Velero populates the storage credentials into the environment variables and Azure SDK reads them to do the auth.
Similar logic appears in two places of Velero:

• Azure plugin

  • Populate the crednetial
  • Azure SDK read from the env

• Repository code

  • Populate the credential
  • Azure SDK read from the env

However, different BSLs may have different credentials which could be set by the Credential field, the credential of one BSL may be overridden by others when dealing the BSLs concurrently, e.g. do the backup and restore with different BSL in the same time.

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

• 👍 for "I would like to see this bug fixed as soon as possible"
• 👎 for "There are more important bugs to focus on right now"

[anshulahuja98]

So overall how I see this is that we wish to replace NewDefaultAzureCredentialto something which can take the values in function inputs.
NewDefaultAzureCredential - comes with it's own set of benefits since it tries various auth approaches - ServicePrincipal / MSI /Workload Identity etc.

There is no equivalent function which takes these values as inputs and tries all auth mechanisms.

One route I see by which we can achieve this is by somehow bringing the logic of NewDefaultAzureCredential in our impl and create a ChainedTokenCredential, then use that further.

func AzureCredential(config map[]){
// Try to create NewClientSecretCredential with secret, principal ID, client ID
NewClientSecretCredential () // This does not have dependency on environment variables.
ChainedTokenCredential.Append()

WorkloadIdentity()
ChainedTokenCredential.Append()

MSICredential()
ChainedTokenCredential.Append()

}

Code ref : https://github.com/Azure/azure-sdk-for-go/blob/a98f12ec13aeff1991f2343db112f0d89bf1cdae/sdk/azidentity/default_azure_credential.go#L60

NewClientSecretCredential atleast will solve for environment dependence of NewEnvironmentCredential

for other routes, we'll have to slightly deeper into handling, but hopefully they should be solvable.

If this approach seems messy and indeterministic to an extent, we can also evaluate adding another field in the BSL which says which auth mode to use - MSI/ SP/ WKLD IDentity etc.
That could translate into an if-else tree to choose which auth mode to use when.

[ywk253100]

@anshulahuja98 That's exactly what I'm thinking of.

The change should appear in three places: Azure plugin, Velero repository code and Kopia, we need to maintain duplicated code if we implement the logic separately.
Is it possible that we implement a ConfigCredential which is like the EnvironmentCredential but reads the credentials from a map in upstream Azure SDK? Seems that's pretty reasonable for the code that uses Azure SDK and runs concurrently.

[anshulahuja98]

EnvironmentCredential is just a wrapper on top of NewClientSecretCredential . We can simply use that.

Are you talking about an implementation similar to NewDefaultAzureCredential instead?

[ywk253100]

Two levels for the implementation of Azure SDK:

The first one is implementing a ConfigCredential which is like the EnvironmentCredential but reads the credentials from a map. Besides the ClientSecretCredential we also need the ClientCertificateCredential and UsernamePasswordCredential as the current code supports all of them.

The second level is implementing a new function like the NewDefaultAzureCredential but wraps the ConfigCredential instead(make ConfigCredential, WorkloadIdentityCredential, ManagedIdentityCredential and CLICredential as a chain).

With these two new implementations, developers can use them to interact with Azure in concurrent code.

[anshulahuja98]

Hmm, it does make sense, but might be slightly difficult to convince the SDK team.
Might need to start a bit of code to see how the interfaces will end up looking like and then we can create an Issue in their repo wiith more details. And see what they think of it.

[ywk253100]

OK. I'll implement that on the Kopia side and let estimate that again after the work

[ywk253100]

Opened an issue for Azure SDK: Azure/azure-sdk-for-go#21061

[ywk253100]

Fixed by vmware-tanzu/velero-plugin-for-microsoft-azure#206 and #6686