-
Notifications
You must be signed in to change notification settings - Fork 101
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
Azure DevOps OIDC #550
Azure DevOps OIDC #550
Conversation
Need to do the |
It seems like a lot of PRs on this repo does not get a lot of attention with the oldest hanging back from 2020. I have done a review of your code and docs, and I think you have done a stellar job @davidcorrigan714 |
Thanks for you contribution @davidcorrigan714 , and thank you all for your patience with this issue. I'm currently working with @ROunofF to land PR #553, which will provide OIDC support for all of the Toolkit's tasks. |
Can you articulate your reasoning for preferring that approach? I’ll do a
longer write up when I’m back at my computer on the pros/cons of both
approaches but using an Azure connection for accessing AWS has some really
bad ergonomics. Having AWS connections labeled as Azure connections in the
UI sounds like a really bad idea. There’s an expectation that filtering by
a type of connection lists all available connections for that service.
Theres some minor security concerns if the Azure connection uses OIDC for
both Azure and AWS since the audience field of the claims can’t be
customized. That approach also involves touching every pipeline that uses
the connections, by changing just the service connection definition and the
tasks auth code the credentials can be updated with no pipeline code
changes.
…On Thu, Jul 18, 2024 at 1:21 PM Rishi Barad ***@***.***> wrote:
Thanks for you contribution @davidcorrigan714
<https://github.com/davidcorrigan714> , and thank you all for your
patience with this issue. I'm currently working with @ROunofF
<https://github.com/ROunofF> to land PR #553
<#553>, which will
provide OIDC support for all of the Toolkit's tasks.
—
Reply to this email directly, view it on GitHub
<#550 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALOFOJVUBUM3RHM5PV3IBO3ZNAPSZAVCNFSM6AAAAABG3IRM4WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZXGQ4TCOBTGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I never saw you could reference another service connection in the way you did... You're referencing an azure Service Connection in the aws Service Connection so the potential security concerns is the same. We could probably merge both :
|
To chime in here, from an ergonomics point of view, the implementation that uses an AWS service connection with an OIDC option is heavily prefered from our organization. We run some 800 AWS accounts and have more than 900 ADO projects. To me it also feels more like the spirit of how users would expect this to work from a GUI and setup perspective.
Just editing the Service Connections as an Admin on the users behalf is a lot more feasable as it can be done with admin changes only and no modifications to the pipelines themselves.. |
Thanks @HenrikStanley for the insights on this.
Questions:
In both scenarios, you'll need: 1 - Setup the OIDC provider in all AWS accounts Then David's way: The other scenario #553 : There is pros and cons for both... |
We don’t have nearly as many service connections, but we do it all through Terraform. Once this is in I’ll put up a PR for the ADO Terraform provider. We already use OIDC with HCP tokens a lot so we have modules already for setting up all the identity stuff in AWS. Really nice thing with this is it won’t leave any keys in the state files. |
So most of that is wrong. In my method AWS is configured with the OIDC IdP, existing AWS service connections are switched to use OIDC, then roles are created in AWS with a trust policy allowing the service connections to assume them. Azure doesn’t come into play at all, other than the OIDC token having an Azure audience. The wonky audience is far from ideal and I’m a tad ticked at Microsoft for not finishing the feature as originally designed for better support for third party services but IMO the subject of a service connection is already tied to a specific audience just because service connections are naturally created one per service. |
So yes, we need to do work regardless, that is a given, but the important part is that the work can be done without needing preform any changes to the developers pipelines. The amount of pipelines that connect to AWS accounts ranges in the thousands, so even if it is a small snippet, changing that for each and every pipeline and coordinating these changes with the developers could take the time to implement from weeks to years. Also I do have question about the #553 implementation. With Davids solution an AWS service connection clearly is of the type AWS and you configure it with the desired type (in this case OIDC instead of static IAM) but both options reside in the plugin. Lastly, the whole point of service connections in the first place is that we do not need to directly expose the credentials in the pipelines as environment variables. the #553 implementation seems to require that all tasks now do this. So all tasks that are configured to use the service connection now needs to be changed to now reference it directly but instead rely on the environment variables. These do not persist between server changes (which happens every time you do a new stage btw), making it quite messy as every single task that does things in AWS in every pipeline in the entire org would need to be updated. Have we also tested that ALL tasks that currently rely on a service connection can just fall back to the exported credentials? It has been a long time since I did some of my pipelines, but I do remember years ago that some tasks would only work with the service connection. All of these things makes my organization heavily lean towards the @davidcorrigan714 solution. Or at the very least, any solution that does not require auth to be separate from the selected service connection as this would require re-writes of all pipeliens to support, on top of the Role / Service Connection changes. |
There is a problem with this here which is that you break the inherent design philosophy of using service connections in Azure DevOps. While I will be the first to admit I think there are many flaws with Service Connections in general, a big selling point is that each tasks explicitly references which credentials you are using. And persistence between jobs / stages / tasks is no issue as tasks individually references their authentication. In the #553 implementation, you flip this on its head so now all AWS related tasks would implicitly authenticate using whatever the environment variables are at that moment. Not only does this require that you remove the reference on all AWS tasks in all pipelines so they no longer point to the Service Connection, but you also create challenges when you want pipelines to run in multiple AWS accounts, or into the same account but with different dedicated roles. In this case I would need to run the sts assume setup every time I want to use a different credential set. And if I mess up and forget to explicit change my implicit authentication, I could easily end up executing commands in the wrong context. In my opinion, relying on implicit anything in regards to declarative pipelines is a very dangerous proposition. One implementation relies on the existing design language in terms of authentication and is (IAM Roles and Provider Trusts not withstanding) a drop in replacement. The other requires that you fundamentally re-write all your pipelines to take this new design philosophy into account. At least to us, these are very important factors that should be considered when choosing an implementation. |
@davidcorrigan714 @HenrikStanley Following your comments on this thread, we add a simple options to enable OIDC in the service connections and made the tasks use this to get the right credentials see #558 on the implementation. This is simpler and seems to address the comments you left. The powershell tasks seems to use a different mechanism, which I'm checking right now but let us know what you think. This will make #553 not useful (and will superseded #550 as well) |
Thank you all for the contributions and helpful feedback in this PR. We've merged #558 to support OIDC and released this feature in v1.15.0 of the Toolkit. Closing this PR. |
Description
This change allows AWS connections from Azure DevOps to use OIDC authentication to AWS instead of stored access tokens. Microsoft also calls this "Workload Identity Federation".
Motivation
Using long lived credentials for authenticating into AWS is highly discouraged and incurs the manual overhead of managing those credentials. This process uses short lived OIDC tokens generated by Azure DevOps which are generated for each run and authenticated by AWS and a configured OIDC IdP to provide temporary credentials for a role.
Related Issue(s), If Filed
#521
Testing
I've been testing this during in an Azure DevOps Services account, the change is not applicable to the Azure DevOps Server product but I have confirmed that it does not break plugin installation for it. I tested primarily against the AWSPowerShellModuleScript and the AWSCLI task, some more testing is probably warranted though the rest of the tasks seem to leverage the authentication code that I updated.
Checklist
npm run newChange
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.