Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Support OIDC in Azure Pipelines #4343

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ellismg
Copy link
Member

@ellismg ellismg commented Sep 18, 2024

This change teaches azd how to login using a service connection for
an OIDC like experience when running in Azure Pipelines using service
connections and then updates our pipelines to use this authentication
strategy.

Contributes To #4341

local.StringVar(
&lf.systemAccessTokenEnvVar,
"system-access-token-environment-variable-name",
"SYSTEM_ACCESSTOKEN",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the sample name used in the example on how to get this (liked to via the identity SDK): https://learn.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml#systemaccesstoken, so I figured I'd use that as a default.

ClientID *string `json:"clientId,omitempty"`
TenantID *string `json:"tenantId,omitempty"`
ServiceConnectionID *string `json:"serviceConnectionId,omitempty"`
SystemAccessTokenName *string `json:"systemAccessTokenName,omitempty"`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of storing the value (which is both secret and per build) I figured we'd store the name of the env-var and then look up the value when we needed it. That means we don't need to add anything to the persisted secret struct.

This is used as a bearer token when doing the OIDC dance, internally there's an injected env-var that has a endpoint that the credential make requests to and this token secures these requests.

@@ -1,6 +1,7 @@
parameters:
SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources)
AzdDirectory: ""
ServiceConnectionId: "3d79cc98-46f2-428c-bdd5-861414f85602"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danieljurek I assume the right thing to do here is to pull this off of SubscriptionConfiguration but I don't know enough about what that structure looks like to know if this value there or not. These needs to be the ID of the service connection that we want to use.

The AzureCLI@2 task seems to do name to id translation (I am guessing) but I haven't looked into how that works yet and if we could support it. I found this GUID by looking around at some of our internal configuration files. I won't check this in until you tell me the right way to do it, but trying to get stuff off the ground and taking some shortcuts.

Copy link
Contributor

@weikanglim weikanglim Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may or may not help:

  • AzureCli task that handles the input parameters
  • vsts-task-lib that probably has the implementation for retrieving service connections

I think environment variables are being seeded -- I'm not sure if that means you may need an azdo task defined at the pipeline level to get everything working e2e.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think environment variables are being seeded

Yes, it looks like this happens when you have a task with an input of type connectedService:AzureRM. So I guess we could make this "just work" in the context of an AzureCLI@2 task (or create our own).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellismg ellismg force-pushed the ellismg/support-azure-pipelines-auth branch 4 times, most recently from 6d6aec1 to f22a1ca Compare September 18, 2024 23:29
@@ -139,6 +141,16 @@ func (lf *loginFlags) Bind(local *pflag.FlagSet, global *internal.GlobalCommandO
cClientCertificateFlagName,
"",
"The path to the client certificate for the service principal to authenticate with.")
local.StringVar(
&lf.serviceConnectionID,
"service-connection-id",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, I would like to hide all the implementation details from the connection because it is 100% unique to Azure Devops.
Azd currently has the flag --federated-credential-provider to hide all the complexity and implementation of how to do OICD in github actions. That means, when you want to log in azd in in GitHub actions, you do:

run: |
          azd auth login `
            --client-id "$Env:AZURE_CLIENT_ID" `
            --federated-credential-provider "github" `
            --tenant-id "$Env:AZURE_TENANT_ID"

The implementation about how to get the system token and any other tokens is inside azd's implementation.

So, I would like to just have a new provider azdo, which I can use in a Azdo task like:

run: |
          azd auth login `
            --client-id "$Env:AZURE_CLIENT_ID" `
            --federated-credential-provider "azdo" `
            --tenant-id "$Env:AZURE_TENANT_ID"

Azd would internally know what ENV VARS to looks for to get the system access token and the connection-id.
Azd can adapte itself to multiple scenarios. For example, running azd login from the AzureCLI task.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can look into this but I think the problem is that without the connection id or service connection name we are not going to be able to make this work. We could consider something like --federated-credential-provider azdo:<service-connection-name> so it would be something like --federated-credential-provider azdo:azure-sdk-tests for us. Maybe we can make that work?

Like you, I'd love to be able to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we're done with investigation, I'd love to be able to make sense of whether azd should just be an AzDo task to support this scenario better (my hunch was always that it is indeed necessary, without moving mountains in AzDo).

If we are able to deliver an up-to-par experience, I'd love for that to be captured in #4341

@ellismg ellismg force-pushed the ellismg/support-azure-pipelines-auth branch from f22a1ca to 0a257b5 Compare September 20, 2024 20:29
This change teaches `azd` how to login using a service connection for
an OIDC like experience when running in Azure Pipelines using service
connections and then updates our pipelines to use this authentication
strategy.

Contributes To Azure#4341
@ellismg ellismg force-pushed the ellismg/support-azure-pipelines-auth branch from 0a257b5 to 2ee5a78 Compare September 20, 2024 21:55
@jongio jongio changed the title WIP: Support ODIC in Azure Pipelines WIP: Support OIDC in Azure Pipelines Sep 23, 2024
@ellismg ellismg force-pushed the ellismg/support-azure-pipelines-auth branch from 933aecd to cd2961b Compare October 7, 2024 19:37
@ellismg ellismg force-pushed the ellismg/support-azure-pipelines-auth branch from cd2961b to 5a7e72d Compare October 7, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants