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

Fix aws.profile rendering in cred-provider template #2904

Merged

Conversation

stmcginnis
Copy link
Contributor

Issue number:

Closes #2885

Description of changes:

The ecr-credential-provider can use a profile for determining which credential settings to use out of ~/.aws. This wasn't getting rendered in the configuration template because it was inside an {{#each}} block that would change the handlebars context so it would look for settings values relative to the setting being iterated.

This fixes that by explicitly referencing the aws.profile setting from the root, ignoring iteration context.

Another issue was that setting aws.profile after setting the credential provider settings would not trigger re-rendering of the credential provider configuration file. This adds an "affected-services" value for the setting to tell it to restart kubelet, which triggers the rendering of the template.

A migration was needed because of this affected-services metadata change.

Testing done:

Set credential provider data:

# apiclient apply << EOF
> [settings.kubernetes.credential-providers.ecr-credential-provider]
> enabled = true
> # (optional - defaults to "12h")
> cache-duration = "30m"
> image-patterns = [
>   # One or more URL paths to match an image prefix. Supports globbing of subdomains.
>   "*.dkr.ecr.us-east-2.amazonaws.com",
>   "*.dkr.ecr.us-west-2.amazonaws.com"
> ]
> EOF

Verified the config file rendered with the default aws.profile value:

# cat /etc/kubernetes/kubelet/credential-provider-config.yaml
apiVersion: kubelet.config.k8s.io/v1beta1
kind: CredentialProviderConfig
providers:
  - name: ecr-credential-provider
    matchImages:
      - "*.dkr.ecr.us-east-2.amazonaws.com"
      - "*.dkr.ecr.us-west-2.amazonaws.com"
    defaultCacheDuration: "30m"
    apiVersion: credentialprovider.kubelet.k8s.io/v1alpha1
    env:
      - name: HOME
        value: /root
      - name: AWS_PROFILE
        value: default

Updated the profile setting:

# apiclient set -j '{"aws": { "profile": "foo"}}'

Verified the config file was re-rendered with the correct information:

# cat /etc/kubernetes/kubelet/credential-provider-config.yaml
apiVersion: kubelet.config.k8s.io/v1beta1
kind: CredentialProviderConfig
providers:
  - name: ecr-credential-provider
    matchImages:
      - "*.dkr.ecr.us-east-2.amazonaws.com"
      - "*.dkr.ecr.us-west-2.amazonaws.com"
    defaultCacheDuration: "30m"
    apiVersion: credentialprovider.kubelet.k8s.io/v1alpha1
    env:
      - name: HOME
        value: /root
      - name: AWS_PROFILE
        value: foo

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.

Release.toml Outdated
@@ -189,3 +189,6 @@ version = "1.13.0"
"migrate_v1.13.0_aws-control-container-v0-7-1.lz4",
"migrate_v1.13.0_public-control-container-v0-7-1.lz4",
]
"(1.13.0, 1.14.0)" = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should probably be 1.13.0, 1.13.1 in case we want to backport and do a stable release for this. I'll update it to make cherry-picking to the 1.13.x branch easier. Then if we do a point release it is there. If not, easy enough to change back to 1.14.0 on the develop branch.

@stmcginnis stmcginnis force-pushed the cred-provider-profile branch 2 times, most recently from 4d2ea48 to bc4132a Compare March 18, 2023 22:14
The ecr-credential-provider can use a profile for determining which
credential settings to use out of `~/.aws`. This wasn't getting rendered
in the configuration template because it was inside an `{{#each}}` block
that would change the handlebars context so it would look for settings
values relative to the setting being iterated.

This fixes that by explicitly referencing the `aws.profile` setting from
the root, ignoring iteration context.

Another issue was that setting `aws.profile` after setting the
credential provider settings would not trigger re-rendering of the
credential provider configuration file. This adds an "affected-services"
value for the setting to tell it to restart kubelet, which triggers the
rendering of the template.

A migration was needed because of this affected-services metadata
change.

Signed-off-by: Sean McGinnis <stmcg@amazon.com>
@@ -15,9 +15,9 @@ providers:
env:
- name: HOME
value: /root
{{#if settings.aws.profile}}
{{#if @root.settings.aws.profile}}
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why was the @root. addition needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that was the tricky, key part of this.

Because this is inside an {{#each}} block, that means handlebars iterates through "each" of the settings.kubernetes.credential-providers values in that collection.

What's not immediately obvious is that it also means you are within the context of those iterated values. So that's why on the next line you can do:

{{#if this.enabled}}

instead of some sort of indexing operation into the collection. But that's also where the subtle gotcha is.

It meant that later on with the line:

{{#if settings.aws.profile}}

it was actually looking at settings.kubernetes.credential-providers[x].settings.aws.profile. Since we're inside the each block, we actually need to break out of that local context to get to the real settings.aws.profile.

Handlebars allows you to do this in two different ways - either relationally (../../aws.profile) or by explicitly going from the anchor like what has been done here. This looks a lot cleaner, and probably safer, so I went with the @root route.

@stmcginnis stmcginnis merged commit 7c8cb90 into bottlerocket-os:develop Mar 21, 2023
@stmcginnis stmcginnis deleted the cred-provider-profile branch March 21, 2023 19:50
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.

aws.profile not respected in ecr-credential-provider template.
4 participants