Support authentication via Shared Access Signatures (SAS) for Azure artifacts

[alexdittmann]

Summary

Support authentication via Shared Access Signatures (SAS) for Azure artifacts

Use Cases

With argo v3.4 support for Azure artifacts has been implemented. As far as I understand, the current implementation only supports authentication via Storage Account access keys, but not via Shared Access Signatures (SAS).
I would like to be able to use SAS for authentication, too.

@brianloss Since you worked on this feature, could you confirm that I am right why my assumption? I could not find a way to make it work with SAS tokens. (and thanks btw for implementing it!)

Me/my team would potentially be willing do this contribution.

---

Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

[brianloss]

@alexdittmann the underlying authentication uses the Azure SDK for Go, and the DefaultAzureCredential in particular. This supports authentication via environment variables, Workload Identity, Managed Identity, or Azure CLI. (See this learn page for more details as well.) This is all enabled by setting UseSDKCreds to true. Otherwise, the authentication method is shared access key. So, although more than just access key access is supported, SAS is not one of the supported methods. This is primarily because SAS tokens are effectively just additional query parameters added to the URL.

You could try to add the SAS token to the endpoint field and set UseSDKCreds to true to use the DefaultAzureCredential, but I'm guessing that might fail since you wouldn't actually provide any credentials. You'd likely also need to call azblob.NewContainerClientWithNoCredential instead of azblob.NewContainerClient or azblob.NewContainerClientWithSharedKey in the newAzureContainerClient method when you want to use a SAS.

[missingcharacter]

I want to clarify that Workload Identity is not yet supported as of azure-sdk-for-go/sdk/azidentity version 1.2.1, which is the version argo-workflows version 3.4.5 uses at the moment.

argo-workflows/go.mod

Line 7 in 1322f26

github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.2.1

[zsfarkas]

It's a pity. We would have used this feature with Azure ManagedIdentity very gladly. This is actually the suggested and easiest way to access storage from kubernetes.

[brianloss]

Ah, it looks like that feature in the Go SDK is still in beta. Not sure if a PR with a beta version of identity would be accepted or not, but it would be a fairly simple change to make if anyone has time to test it out.

[alexdittmann]

Hi,
I would like to bring this issue up again. We (my team) have created it in the past and would now like to look into this issue.
However, regarding contributions to the artifacts area i have one question: In the past I asked a similar question about contributing to the argo workflows artifacts codebase (see this slack thread) and from that I understood that argo wants to minimize new artifact functionality.
Now, this contribution here would only extend an already existing artifact type (azure) by an additional authentication method.
So my question is: Would such a contribution have a general chance of being accepted?
@agilgur5 maybe you can comment on this again?

Thank you and best regards
alex

[agilgur5]

Yes, as I wrote in that Slack thread too, extending existing functionality is likely to be accepted. Similarly as I wrote there, the spec etc is still important; I haven't read this issue in much detail to comment.

If we externalize artifact plugins (#5862), some may still be "core" plugins that are built-in or more closely maintained. The main "hyperscaler" clouds are the most popular to support, so Azure features certainly welcome.

[kavishdahekar-sap]

Hi @brianloss and @agilgur5 , I am a part of @alexdittmann 's team currently working on a proposal for the support of Azure SAS tokens.

Could you please quickly verify one of these approaches as possibly suitable for moving forward with this contribution?

1. For supporting azure SAS tokens, we do not introduce any spec changes, instead when reading the AccountKeySecret, the Azure ArtifactDriver will parse the secret string to check for query param structure (&<param-name>=<param-value>) to determine whether the provided secret is a SAS token. If yes, the driver will internally initialize a NewContainerClientWithNoCredential.

2. We introduce a new boolean spec param useSASToken which the Azure artifact driver will use for determining whether to consider the contents of AccountKeySecret as a SAS token.

3. We introduce a new spec field SASTokenSecret similar to current accountKeySecret.

spec:
  entrypoint: input-artifact-azure-example
  templates:
  - name: input-artifact-azure-example
    inputs:
      artifacts:
      - name: my-art
        path: /my-artifact
        azure:
          SASTokenSecret:
            name: my-azure-sas-token
            key: token

For the 2nd and 3rd options, we would need a mechanism to resolve cases where the user provides more than one option (example : setting both useSDKCreds and useSASToken to true, or providing both accountKeySecret and SASTokenSecret).

[alexdittmann]

Hi again @agilgur5,
do you have any opinion on the query of @kavishdahekar-sap or could tell us where we could get some additional feedback?
Otherwise, as we would like to implement this soon, we just start implementing option 1.

Thank you and best
Alex

[kavishdahekar-sap]

Could I get a review here please? @agilgur5 @brianloss
#13360

[brianloss]

@kavishdahekar-sap I'm not a committer, but I can take a look.