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 docs for configuring kubelet credential provider plugins #25226

Merged
merged 2 commits into from Dec 3, 2020
Merged

Add docs for configuring kubelet credential provider plugins #25226

merged 2 commits into from Dec 3, 2020

Conversation

andrewsykim
Copy link
Member

Signed-off-by: Andrew Sy Kim kim.andrewsy@gmail.com

Add docs for configuring kubelet credential provider plugin. Replaces #24929 since that was targeting the wrong branch.

@k8sio-netlify-preview-bot
Copy link
Collaborator

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 924acf8

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5fc8372a3c7a790008a61bcf

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 24, 2020
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 24, 2020
@annajung
Copy link
Contributor

/milestone 1.20
/assign
/sig cloud-provider node

@k8s-ci-robot k8s-ci-robot added the sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. label Nov 24, 2020
@k8s-ci-robot k8s-ci-robot added this to the 1.20 milestone Nov 24, 2020
@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Nov 24, 2020
@andrewsykim
Copy link
Member Author

/assign @liggitt @cheftako


## {{% heading "prerequisites" %}}

* kubelet on v1.20 or later. If on v1.20, ensure the alpha feature gate `KubeletCredentialProviders` is on.
Copy link
Contributor

Choose a reason for hiding this comment

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

KubeletCredentialProviders needs to be added to the feature gate list content/en/docs/reference/command-line-tools-reference/feature-gates/ with a short description describing the feature

Copy link
Member Author

Choose a reason for hiding this comment

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

ack, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

* The kubelet image credential provider is introduced in 1.20 as an alpha feature.
  As with other alpha features, a feature gate `KubeletCredentialProviders` must be
  explicitly enabled for the feature to work

Not a blocker though. Just trying to ensure the information will not become outdated soon. For example, we don't know if the feature will stay in alpha on v1.21.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great call @tengqm, will update with your suggestion

Comment on lines 19 to 21
1. API calls to a cloud provider service are required to retreive authentication information for a registry.
2. Credentials have short expiration times and requesting new credentials frequently is required.
3. Storing registry credentials on disk or in image pull Secrets is not acceptable.
Copy link
Contributor

Choose a reason for hiding this comment

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

nits:

  • add an empty line before line 19
  • avoid explicit numbers in ordered list, always use 1.

This guide demonstrates how to configure the kubelet's image credential provider plugin mechanism.

{{< note >}}
This capabilitiy will eventually replace built-in logic in the kubelet to serve registry credentials for Azure, AWS and GCP.
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit forward looking, we'd better avoid statements about future in the doc.

Comment on lines +46 to +45
* `--image-credential-provider-config` - the path to the credential provider plugin config file.
* `--image-credential-provider-bin-dir` - the path to the directory where credential provider plugin binaries are located.
Copy link
Contributor

Choose a reason for hiding this comment

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

Better document the equivalent kubelet config field names here as well. We are seemingly moving away from command line flags anyway.

Copy link
Member

Choose a reason for hiding this comment

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

@andrewsykim are there corresponding config field names, or are these only flags?

Copy link
Member Author

Choose a reason for hiding this comment

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

We did not add config fields for these, which I think has been expected for container runtime options. For example, none of the flags in ContainerRuntimeOptions have config field equivalents:

https://github.com/kubernetes/kubernetes/blob/51441fd052756d2c76daee83d39b352bd8a2d265/pkg/kubelet/config/flags.go#L26-L82

* `--image-credential-provider-config` - the path to the credential provider plugin config file.
* `--image-credential-provider-bin-dir` - the path to the directory where credential provider plugin binaries are located.

If you are on v1.20, ensure the feature gate `KubeletCredentialProviders` is enabled via the `--feature-gates` flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

This prerequisite is already mentioned on line 31


## {{% heading "prerequisites" %}}

* kubelet on v1.20 or later. If on v1.20, ensure the alpha feature gate `KubeletCredentialProviders` is on.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

* The kubelet image credential provider is introduced in 1.20 as an alpha feature.
  As with other alpha features, a feature gate `KubeletCredentialProviders` must be
  explicitly enabled for the feature to work

Not a blocker though. Just trying to ensure the information will not become outdated soon. For example, we don't know if the feature will stay in alpha on v1.21.

- "*.dkr.ecr-fips.*.amazonaws.com"
- "*.dkr.ecr.us-iso-east-1.c2s.ic.gov"
- "*.dkr.ecr.us-isob-east-1.sc2s.sgov.gov"
defaultCacheDuration: 12h
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultCacheDuration: 12h
defaultCacheDuration: "12h"

The `matchImages` field for each credenetial provider is used by the kubelet to determine whether a plugin should be invoked for a given image that a Pod is using.
Each entry in `matchImages` is a pattern which can optionally contain a port and a path. Globs can be used in the domain, but not in the port or the path. Globs are
supported as subdomains like '\*.k8s.io' or 'k8s.\*.io', and top-level domains such as 'k8s.\*'. Matching partial subdomains like 'app\*.k8s.io' is also supported. Each
glob can only match a single subdomain segment, so '\*.io' does NOT match '\*.k8s.io'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider wrap the long lines properly so that future changes can be easily tracked.


The `matchImages` field for each credenetial provider is used by the kubelet to determine whether a plugin should be invoked for a given image that a Pod is using.
Each entry in `matchImages` is a pattern which can optionally contain a port and a path. Globs can be used in the domain, but not in the port or the path. Globs are
supported as subdomains like '\*.k8s.io' or 'k8s.\*.io', and top-level domains such as 'k8s.\*'. Matching partial subdomains like 'app\*.k8s.io' is also supported. Each
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
supported as subdomains like '\*.k8s.io' or 'k8s.\*.io', and top-level domains such as 'k8s.\*'. Matching partial subdomains like 'app\*.k8s.io' is also supported. Each
supported as subdomains like `*.k8s.io` or `k8s.*.io`, and top-level domains such as
`k8s.*`. Matching partial subdomains like `app*.k8s.io` is also supported. Each

The point here is that by wrapping these literals with backtiqs, readers can easily identify what are the patterns with NO confusion. Having backslashes in the text can thus considered harmful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, will update


A match exists between an image and a matchImage entry when all of the below are true:
* Both contain the same number of domain parts and each part matches.
* The URL path of an imageMatch must be a prefix of the target image URL path.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is an "imageMatch"?

Comment on lines 110 to 114
* 123456789.dkr.ecr.us-east-1.amazonaws.com
* \*.azurecr.io
* gcr.io
* \*.\*.registry.io
* foo.registry.io:8080/path
Copy link
Contributor

Choose a reason for hiding this comment

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

plse consider using backtiqs for these string literals.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@andrewsykim, my take on this documentation is that it is closer to a conceptual overview than a task that the reader could step through.

I am not sure where the best home for this might be. I have a suggestion, though:
https://kubernetes.io/docs/concepts/containers/image-credential-plugins/

Would you be willing to move this documentation there and switch the content_type to concept?

## {{% heading "prerequisites" %}}

* The kubelet image credential provider is introduced in v1.20 as an alpha feature. As with other alpha features,
a feature gate `KubeletCredentialProviders` must be explicitly enabled for the feature to work
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: link to the list of feature gates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where does the reader enable that feature gate? Just for the kubelet?


## Installing Plugins on Nodes

A credential provider plugin is an exectuable binary that will be run by the kubelet. Ensure that the plugin binary exists on
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a spelling issue:

Suggested change
A credential provider plugin is an exectuable binary that will be run by the kubelet. Ensure that the plugin binary exists on
A credential provider plugin is an executable binary that will be run by the kubelet. Ensure that the plugin binary exists on

* `--image-credential-provider-config` - the path to the credential provider plugin config file.
* `--image-credential-provider-bin-dir` - the path to the directory where credential provider plugin binaries are located.

### Credential Provider Configuration File
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a task page, a better heading might be

Suggested change
### Credential Provider Configuration File
### Configure a kubelet credential provider

### Credential Provider Configuration File

The configuration file passed into `--image-credential-provider-config` is read by the kubelet to determine which exec plugins
should be invoked for which container images. Here's an example configuration file you may end up using if you are using the ECR-based plugin:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider expanding ECR or hyperlinking to a page that explains what it is.

defaultCacheDuration: "12h"
apiVersion: credentialprovider.kubelet.k8s.io/v1alpha1
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this task list an AWS EC2 instance as a prerequisite? Alternatively, is there a different credential mechanism that we can explain, perhaps something more generic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Trying to use a real example in the docs instead of dummy values. I think AWS ECR is a good reference because you can enable it on non-ec2 instance as long as there are credentials on disk.


Consult the plugin implementors to determine what set of arguments and environment variables are required for a given plugin.

#### Match Images
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### Match Images
#### Configure image matching

and top-level domains such as `k8s.*`. Matching partial subdomains like `app*.k8s.io` is also supported. Each glob can only match
a single subdomain segment, so `*.io` does NOT match `*.k8s.io`.

A match exists between an image and a matchImage entry when all of the below are true:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A match exists between an image and a matchImage entry when all of the below are true:
A match exists between an image name and a `matchImage` entry when all of the below are true:

* The URL path of match image must be a prefix of the target image URL path.
* If the imageMatch contains a port, then the port must match in the image as well.

Some example values of `matchImages` are:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Some example values of `matchImages` are:
Some example values of `matchImages` patterns are:

A credential provider plugin is an exectuable binary that will be run by the kubelet. Ensure that the plugin binary exists on
every node in your cluster and stored in a known directory. The directory will be required later when configuring kubelet flags.

## Configuring Kubelet
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Configuring Kubelet
## Configure the kubelet


## Configuring Kubelet

### Flags
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove this line.

@annajung
Copy link
Contributor

Hi @liggitt @cheftako can you provide tech review?

@annajung
Copy link
Contributor

annajung commented Dec 1, 2020

Just a friendly reminder that Docs deadline to get this merged in is in two days, Dec. 2nd
Please respond to the feedback and make sure both docs/tech reviews are given/received asap.

@andrewsykim
Copy link
Member Author

andrewsykim commented Dec 2, 2020

@andrewsykim, my take on this documentation is that it is closer to a conceptual overview than a task that the reader could step through.

I am not sure where the best home for this might be. I have a suggestion, though:
https://kubernetes.io/docs/concepts/containers/image-credential-plugins/

Would you be willing to move this documentation there and switch the content_type to concept?

@sftim I had initially added this to "Concepts" but you suggested moving it to "Tasks" in #24929 (review) 😛 -- I think it makes a lot of sense though. I can move it back but I'll need a day or two to work it out. Sorry for the delay.

Copy link
Member

@liggitt liggitt left a comment

Choose a reason for hiding this comment

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

Some typos/nits, but this is a good start for configuring a plugin. Were you planning on adding docs for developing a plugin that detailed the request/response bits?

Starting from Kubernetes v1.20, the kubelet can dynamically retrieve credentials for a container image registry
using exec plugins. The kubelet and the exec plugin communicate through stdio (stdin, stdout, and stderr) using
Kubernetes versioned APIs. These plugins allow the kubelet to request credentials for a container registry dynamically
as oppposed to storing static credentials on disk. For example, the plugin may talk to a local metadata server to retrieve
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
as oppposed to storing static credentials on disk. For example, the plugin may talk to a local metadata server to retrieve
as opposed to storing static credentials on disk. For example, the plugin may talk to a local metadata server to retrieve


* API calls to a cloud provider service are required to retreive authentication information for a registry.
* Credentials have short expiration times and requesting new credentials frequently is required.
* Storing registry credentials on disk or in image pull Secrets is not acceptable.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Storing registry credentials on disk or in image pull Secrets is not acceptable.
* Storing registry credentials on disk or in imagePullSecrets is not acceptable.


You may be interested in using this capabilitiy if any of the below are true:

* API calls to a cloud provider service are required to retreive authentication information for a registry.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* API calls to a cloud provider service are required to retreive authentication information for a registry.
* API calls to a cloud provider service are required to retrieve authentication information for a registry.

as oppposed to storing static credentials on disk. For example, the plugin may talk to a local metadata server to retrieve
short-lived credentials for an image that is being pulled by the kubelet.

You may be interested in using this capabilitiy if any of the below are true:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
You may be interested in using this capabilitiy if any of the below are true:
You may be interested in using this capability if any of the below are true:

Comment on lines +46 to +45
* `--image-credential-provider-config` - the path to the credential provider plugin config file.
* `--image-credential-provider-bin-dir` - the path to the directory where credential provider plugin binaries are located.
Copy link
Member

Choose a reason for hiding this comment

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

@andrewsykim are there corresponding config field names, or are these only flags?

```yaml
kind: CredentialProviderConfig
apiVersion: kubelet.config.k8s.io/v1alpha1
providers:
Copy link
Member

Choose a reason for hiding this comment

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

I've found it helpful to annotate config and request/response documentation with the API docs, e.g. https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#request

This is a great place to explain what each field means, what it does, etc

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, will updat


#### Match Images

The `matchImages` field for each credenetial provider is used by the kubelet to determine whether a plugin should be invoked
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `matchImages` field for each credenetial provider is used by the kubelet to determine whether a plugin should be invoked
The `matchImages` field for each credential provider is used by the kubelet to determine whether a plugin should be invoked

@andrewsykim
Copy link
Member Author

Were you planning on adding docs for developing a plugin that detailed the request/response bits?

Yes, can someone from SIG docs recommend where a document like this should live on the doc site?

@andrewsykim
Copy link
Member Author

@sftim I'm thinking that this feature actually needs both a "concepts" and "tasks" page, as well as a developer-focused page for plugin authors. I'd like to keep this PR scoped to just the tasks page but I will follow-up to add the others before this feature goes beta.

@annajung
Copy link
Contributor

annajung commented Dec 2, 2020

@sftim I'm thinking that this feature actually needs both a "concepts" and "tasks" page, as well as a developer-focused page for plugin authors. I'd like to keep this PR scoped to just the tasks page but I will follow-up to add the others before this feature goes beta.

I think this works, but I will wait for Tim/others to chime in. Maybe opening an issue where this can be tracked could help make sure this doesn't get lost could help as well.

@sftim
Copy link
Contributor

sftim commented Dec 2, 2020

wait for Tim/others to chime in

Hi, I'm a bit too busy at the moment - please feel free to get reviews elsewhere, and to dismiss / ignore any earlier reviews of mine if superseded.

@celestehorgan
Copy link
Contributor

@andrewsykim In the interests of time, and after reading through this doc, I think it's well-placed in /tasks, though I do recommend picking up @sftim's suggestion of retitling it to Configure a kubelet credential provider.

@jimangel
Copy link
Member

jimangel commented Dec 3, 2020

Seconding @celestehorgan's comment. I think the content is sufficient to enable early adopters of 1.20 to leverage the new enhancement as-is, which is the most important part of getting docs in before the actual release.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2020
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 40dde3e30651b8e6e256013b52cee9889f975311

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2020
@andrewsykim
Copy link
Member Author

@andrewsykim In the interests of time, and after reading through this doc, I think it's well-placed in /tasks, though I do recommend picking up @sftim's suggestion of retitling it to Configure a kubelet credential provider.

Title updated!

@zacharysarah
Copy link
Contributor

Additional 👍 to retitling the page per @sftim and @celestehorgan, (thanks for the retitle!) merging this into /tasks/ for release, then following up with separate concept and task pages.

@andrewsykim

Were you planning on adding docs for developing a plugin that detailed the request/response bits?

Yes, can someone from SIG docs recommend where a document like this should live on the doc site?

One good possibility might be content/en/tasks/extend-kubernetes/. Another possibility would be to add an additional task page in /content/en/tasks/kubelet-credential-provider/ after this PR merges and creates the directory.

@zacharysarah
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2020
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: efa85f01d02043d7a215ca98a8c30b1f49d97b67

@annajung
Copy link
Contributor

annajung commented Dec 3, 2020

Hi @liggitt @cheftako pinging for tech review again. Please note that your concern with documentation on the plugin will be addressed when the feature graduates to beta as noted here #25226 (review) and #25226 (comment)

Once we have tech review / lgtm, this should be good to merge.

@annajung
Copy link
Contributor

annajung commented Dec 3, 2020

concerns with needing more docs will be addressed when features goes to beta
with this being alpha, this will merge as it is. thanks everyone for all your reviews!

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: annajung

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2020
@k8s-ci-robot k8s-ci-robot merged commit 864d7ec into kubernetes:dev-1.20 Dec 3, 2020
Copy link
Contributor

@DangerOnTheRanger DangerOnTheRanger left a comment

Choose a reason for hiding this comment

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

LGTM, very helpful for future plugin implementations!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet