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

Add k8s credential provider support #2377

Merged
merged 1 commit into from Nov 4, 2022

Conversation

stmcginnis
Copy link
Contributor

@stmcginnis stmcginnis commented Aug 24, 2022

Issue number:

Closes #1702

Description of changes:

This adds the capability to use Kubernetes image credential providers to
retrieve credentials to use when pulling images for container creation.

Initially we will only support the ecr-credential-provider, but things
are set up so we may add more providers in future updates.


Yet to do:

  • Include ecr-credential-provider binary
  • Add new config settings
  • Perform full testing and verification
  • Documentation on using this feature (basic docs, more detailed docs TBD)

Testing done:

  • For ecr-credential-provider binary: Added RPM files, built and deployed instance, verified the binary was in the expected location on the filesystem.
  • Addition of new settings: Build, deployed, set values by using apiclient set. Verified settings saved and available using apiclient get and viewing config settings written to filesystem.
  • any-enabled template helper: Addition of unit tests to validate various input. Created small test app to execute templating code and verify expected output.
  • Application of settings:
    • default cred provider config file written.
    • Setting values recreates file and restarts kubelet with correct parameters.
    • Sonobuoy quick tests pass
    • Validate use of ECR plugin for registry validation
    • Tested with Kubernetes 1.20, 1.21, 1.22, and 1.23
    • Tested with separate aws-config and aws-credentials as well as combined using only aws-config

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@stmcginnis
Copy link
Contributor Author

Note: right now this contains some of John's work from #2378 since that will need to merge before the addition of ecr-credential-provider. Hopefully looking at the individual commits can help isolate the relevant things if someone wants to review parts of this while it's being worked on.

sources/models/src/modeled_types/credentialproviders.rs Outdated Show resolved Hide resolved
packages/kubernetes-1.20/kubelet-config Outdated Show resolved Hide resolved
packages/kubernetes-1.21/credential-provider-config.yaml Outdated Show resolved Hide resolved
sources/models/shared-defaults/kubernetes-services.toml Outdated Show resolved Hide resolved
packages/ecr-credential-provider/Cargo.lock Outdated Show resolved Hide resolved
sources/models/src/modeled_types/credentialproviders.rs Outdated Show resolved Hide resolved
variants/aws-dev/Cargo.toml Outdated Show resolved Hide resolved
packages/kubernetes-1.20/kubernetes-1.20.spec Outdated Show resolved Hide resolved
@stmcginnis stmcginnis force-pushed the cred-providers branch 11 times, most recently from 3671e96 to 159bbb7 Compare September 23, 2022 19:20
@stmcginnis stmcginnis marked this pull request as ready for review September 23, 2022 19:50
@stmcginnis
Copy link
Contributor Author

Marking as ready to review, but still performing testing/validation. Everything should be in place though, so pending any testing issues found this should be good to go.

@stmcginnis stmcginnis force-pushed the cred-providers branch 2 times, most recently from cd7eda1 to 007cf75 Compare September 27, 2022 21:46
@stmcginnis stmcginnis mentioned this pull request Sep 28, 2022
5 tasks
@stmcginnis
Copy link
Contributor Author

I believe I have addressed all comments so far. Please let me know if I have missed anything.

I've resolved all of the ones that I think were fully addressed. There are a few that either had questions or were open to further discussion which I have not resolved yet pending some sort of resolution.

@stmcginnis stmcginnis force-pushed the cred-providers branch 3 times, most recently from a57cd47 to 7293603 Compare October 21, 2022 21:19
@matthewdaltonfanduel
Copy link

@arnaldo2792 pinging for re-review

@stmcginnis
Copy link
Contributor Author

Updated to resolve merge conflict in sources/Cargo.lock.

Copy link
Member

@webern webern left a comment

Choose a reason for hiding this comment

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

I guess I'm a little confused about what the "service" named aws is, but assuming that is done correctly... looks good.

Comment on lines 352 to 345
struct AwsSettings {
region: SingleLineString,
Copy link
Member

Choose a reason for hiding this comment

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

Upon further reflection, I now understand why we arrived at aws.settings.* for this. It's because there's essentially one ~/.aws/ profile on the machine and it wouldn't make sense to bury that under, e.g. settings.kubernetes. Makes sense now: 👍

README.md Outdated Show resolved Hide resolved
sources/api/schnauzer/src/lib.rs Outdated Show resolved Hide resolved
sources/models/src/modeled_types/credential_providers.rs Outdated Show resolved Hide resolved
sources/models/src/modeled_types/credential_providers.rs Outdated Show resolved Hide resolved
sources/models/src/modeled_types/credential_providers.rs Outdated Show resolved Hide resolved
sources/models/shared-defaults/aws-creds.toml Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
packages/ecr-credential-provider/Cargo.toml Outdated Show resolved Hide resolved
packages/filesystem/filesystem.spec Outdated Show resolved Hide resolved
packages/ecr-credential-provider/Cargo.toml Outdated Show resolved Hide resolved
variants/Cargo.lock Outdated Show resolved Hide resolved
sources/Cargo.lock Outdated Show resolved Hide resolved
This adds the capability to use Kubernetes image credential providers to
retrieve credentials to use when pulling images for container creation.

Initially we will only support the ecr-credential-provider, but things
are set up so we may add more providers in future updates.

Signed-off-by: Sean McGinnis <stmcg@amazon.com>
@stmcginnis
Copy link
Contributor Author

Rebased and resolved merge conflicts.

@stmcginnis stmcginnis merged commit 66be5e6 into bottlerocket-os:develop Nov 4, 2022
@stmcginnis
Copy link
Contributor Author

Thanks everyone!

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.

kubelet: add credential provider feature-gate
6 participants