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

[Egress providers - S3 store] Add support for service accounts when running dotnet monitor in kubernetes #6626

Merged

Conversation

jwreford99
Copy link
Contributor

Summary

Reason for change
As it stands, the S3 egress provider does not currently support the use of service accounts when running dotnet monitor in kubernetes.

This is one of the two recommended ways to give pods permission to access AWS resources via roles. This tends to be recommended over long lived access keys as it removes the need for regular rotation of credentials etc.

Details of change made

The full documentation for this lives here on AWS's documentation site. It uses OpenID Connect to do authentication and is natively supported in the existing C# SDK which is already used. However, there is a note when talking about SDK support for .NET that says:

You must also include AWSSDK.SecurityToken.

This is correlated to my experience trying service accounts on my own kubernetes cluster, as I tried to use service accounts and received the error:

{Succeeded:false,FailureMessage:Assembly AWSSDK.SecurityToken could not be found or loaded. This assembly must be available at runtime to use Amazon.Runtime.AssumeRoleAWSCredentials.}

This happens because the S3 Storage extension already uses FallbackCredentialsFactory.GetCredentials, which does all the authentication work automatically

awsCredentials ??= FallbackCredentialsFactory.GetCredentials();

(note that the different ways it can be authenticated are best shown in the AWS SDK code itself, which shows it supports Web Identity Credentials (the thing needed for service accounts) as well as credentials from environment variables)

However, that function isn't possible unless AWSSDK.SecurityToken is available at runtime. Therefore, this PR adds a dependency to the S3 project to bring in AWSSDK.SecurityToken

Places I was unsure
I wasn't sure how the dependabot setup works. AFAICT it relies on variables defined in the Versions file, but I couldn't quite figure out how to get that linked up to actually work so any guidance on that would be much appreciated

I also very much welcome any feedback on the documentation, as I wasn't quite sure on tone of voice etc that made sense for this change

I haven't created an issue for this as it was such a small change, however, if you would rather I do so I would be happy to!

Release Notes Entry

Support added for kubernetes service accounts when using the S3 storage egress provider

…entication via Open ID connect is possible

It is enough for the project to be referenced for this to work, and there is no reference to it in the code base, because it's existence allows the AWS SDK to work
@jwreford99 jwreford99 requested a review from a team as a code owner May 15, 2024 14:52
@jwreford99
Copy link
Contributor Author

@dotnet-policy-service agree company="Gearset"

@jwreford99 jwreford99 changed the title Egress provider - S3 store - add support for service accounts when running dotnet monitor in kubernetes [Egress providers - S3 store] Add support for service accounts when running dotnet monitor in kubernetes May 15, 2024
Copy link
Member

@schmittjoseph schmittjoseph left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this and making this PR with the detailed explanation!

Can you also update this file in this PR? We need to explicitly configuring signing information for 3rd party files:

<ItemGroup>
<FileSignInfo Include="AWSSDK.Core.dll" CertificateName="3PartySHA2" />
<FileSignInfo Include="AWSSDK.S3.dll" CertificateName="3PartySHA2" />
<FileSignInfo Include="Newtonsoft.Json.dll" CertificateName="3PartySHA2" />

e.g. add a new line with:

    <FileSignInfo Include="AWSSDK.SecurityToken.dll" CertificateName="3PartySHA2" />

@schmittjoseph schmittjoseph added the update-release-notes Pull requests that should be mentioned in the release notes label May 15, 2024
Spell checker flagged that Kubernetes should have an uppercase K and to use the American spelling of utilize

Co-authored-by: Justin Anderson <jander-msft@users.noreply.github.com>
@jwreford99
Copy link
Contributor Author

@schmittjoseph done in c3c0a5f, thanks!

Anything further I need to do with the dependabot so that this new package gets updated correctly?

@schmittjoseph
Copy link
Member

schmittjoseph commented May 16, 2024

@schmittjoseph done in c3c0a5f, thanks!

Anything further I need to do with the dependabot so that this new package gets updated correctly?

Nope, everything looks good in terms of dependabot! We pull our public dependencies from the .NET org's nuget.org mirror(https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet-public) which has a slightly older version of AWSSDK.SecurityToken (the one you've selected already). Since dependabot also looks at this mirror, it won't try to update this dependency yet if you tried to.

@schmittjoseph schmittjoseph merged commit d9fda60 into dotnet:main May 16, 2024
29 checks passed
@schmittjoseph
Copy link
Member

/backport to release/8.x

Copy link
Contributor

Started backporting to release/8.x: https://github.com/dotnet/dotnet-monitor/actions/runs/9116375732

github-actions bot pushed a commit that referenced this pull request May 16, 2024
…unning dotnet monitor in kubernetes (#6626)

* Reference AWSSDK.SecurityToken in the S3 Storage project so that authentication via Open ID connect is possible
It is enough for the project to be referenced for this to work, and there is no reference to it in the code base, because it's existence allows the AWS SDK to work

* Documentation: add docs to cover the use of service accounts for S3

* Update documentation in line with spell checker

Spell checker flagged that Kubernetes should have an uppercase K and to use the American spelling of utilize

Co-authored-by: Justin Anderson <jander-msft@users.noreply.github.com>

* PR Feedback: Add AWSSDK.SecurityToken.dll to the Signing.props file

---------

Co-authored-by: Justin Anderson <jander-msft@users.noreply.github.com>
@schmittjoseph
Copy link
Member

/backport to release/8.0

Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/dotnet-monitor/actions/runs/9116382467

github-actions bot pushed a commit that referenced this pull request May 16, 2024
…unning dotnet monitor in kubernetes (#6626)

* Reference AWSSDK.SecurityToken in the S3 Storage project so that authentication via Open ID connect is possible
It is enough for the project to be referenced for this to work, and there is no reference to it in the code base, because it's existence allows the AWS SDK to work

* Documentation: add docs to cover the use of service accounts for S3

* Update documentation in line with spell checker

Spell checker flagged that Kubernetes should have an uppercase K and to use the American spelling of utilize

Co-authored-by: Justin Anderson <jander-msft@users.noreply.github.com>

* PR Feedback: Add AWSSDK.SecurityToken.dll to the Signing.props file

---------

Co-authored-by: Justin Anderson <jander-msft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update-release-notes Pull requests that should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants