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

ClusterTrustBundles: document projected volumes #43600

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

ahmedtd
Copy link
Contributor

@ahmedtd ahmedtd commented Oct 20, 2023

ClusterTrustBundle projected volume sources are slated to land in 1.29. Drop a placeholder section in the existing ClusterTrustBundle documentation to document it.

Enhancements Issue: kubernetes/enhancements#3257
Implementation PR: kubernetes/kubernetes#113374

@k8s-ci-robot k8s-ci-robot added this to the 1.29 milestone Oct 20, 2023
@netlify
Copy link

netlify bot commented Oct 20, 2023

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit 6dd3091
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/655ce772557ee800084bb5e3

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 20, 2023
@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. labels Oct 20, 2023
@sftim
Copy link
Contributor

sftim commented Oct 20, 2023

/sig security
/sig auth

/retitle [WIP] ClusterTrustBundles: document projected volumes

@k8s-ci-robot k8s-ci-robot changed the title ClusterTrustBundles: Document projected volumes [WIP] ClusterTrustBundles: document projected volumes Oct 20, 2023
@k8s-ci-robot k8s-ci-robot added sig/security Categorizes an issue or PR as relevant to SIG Security. sig/auth Categorizes an issue or PR as relevant to SIG Auth. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 20, 2023
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.

@katcosgrove
Copy link
Contributor

Hi, @ahmedtd! v1.29 Docs Lead here. Please take a look at Documenting for a release - PR Ready for Review to get your PR ready for review before Tuesday 14th November 2023. Thank you!

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 12, 2023
@ahmedtd ahmedtd changed the title [WIP] ClusterTrustBundles: document projected volumes ClusterTrustBundles: document projected volumes Nov 12, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 12, 2023
@ahmedtd
Copy link
Contributor Author

ahmedtd commented Nov 12, 2023

@sftim @enj This is ready for review.

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.

looks good, just a few nits / suggestions

also add rows in https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/ for ClusterTrustBundle and ClusterTrustBundleProjection gates


{{< note >}}
In Kubernetes {{< skew currentVersion >}}, you must enable the `ClusterTrustBundle` and `ClusterTrustBundleProjection`
[feature gates](/docs/reference/command-line-tools-reference/feature-gates/) to use this feature.
Copy link
Member

Choose a reason for hiding this comment

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

and API, right (with --runtime-config=certificates.k8s.io/v1alpha1/clustertrustbundles=true)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


To select by name, use the `name` field to designate a single ClusterTrustBundle object.

To select by signer name, use the `signerName` and `labelSelector` fields to
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
To select by signer name, use the `signerName` and `labelSelector` fields to
To select by signer name, use the `signerName` field (and optionally the `labelSelector` field) to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

If `labelSelector` is not present, then all ClusterTrustBundles for that signer
are selected.

In both cases, Kubelet will deduplicate all of the certificates from the selected ClusterTrustBundle objects, normalize the PEM representations (discarding comments and headers), and shuffle the order of the certificates before writing them into the file named by `path`. As the set of selected ClusterTrustBundles, or their content, changes, Kubelet will keep the file up-to-date.
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
In both cases, Kubelet will deduplicate all of the certificates from the selected ClusterTrustBundle objects, normalize the PEM representations (discarding comments and headers), and shuffle the order of the certificates before writing them into the file named by `path`. As the set of selected ClusterTrustBundles, or their content, changes, Kubelet will keep the file up-to-date.
The Kubelet deduplicates the certificates in the selected ClusterTrustBundle objects, normalizes the PEM representations (discarding comments and headers), reorders the certificates, and writes them into the file named by `path`. As the set of selected ClusterTrustBundles or their content changes, Kubelet keeps the file up-to-date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

spec:
containers:
- name: container-test
image: busybox:1.28
Copy link
Member

Choose a reason for hiding this comment

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

nit: why the 1.28 tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied this from the example next to it. Removed.

Comment on lines 20 to 21
name: foo
path: foo-roots.pem
Copy link
Member

Choose a reason for hiding this comment

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

nit: use something like example instead of foo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@liggitt
Copy link
Member

liggitt commented Nov 14, 2023

/assign

@ahmedtd
Copy link
Contributor Author

ahmedtd commented Nov 17, 2023

also add rows in https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/ for ClusterTrustBundle and ClusterTrustBundleProjection gates

Done.

Copy link
Member

Choose a reason for hiding this comment

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

Add docs for the projected feature gate in the List of feature gates section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -71,6 +71,8 @@ For a reference to old feature gates that are removed, please refer to
| `AnyVolumeDataSource` | `false` | Alpha | 1.18 | 1.23 |
| `AnyVolumeDataSource` | `true` | Beta | 1.24 | |
| `AppArmor` | `true` | Beta | 1.4 | |
| `ClusterTrustBundle` | `false` | Alpha | 1.28 | |
Copy link
Member

Choose a reason for hiding this comment

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

This is already in this file (see below on line 89).

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 the key outstanding feedback blocking a merge, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@katcosgrove
Copy link
Contributor

Hi, @ahmedtd! Kubernetes v1.29 Docs Lead here. Just a reminder that the deadline to have this PR reviewed and merged is Tuesday, 28 November. Let me know if you need any help!

CCing KEP owners: @liggitt @enj @mikedanese


The Kubelet deduplicates the certificates in the selected ClusterTrustBundle objects, normalizes the PEM representations (discarding comments and headers), reorders the certificates, and writes them into the file named by `path`. As the set of selected ClusterTrustBundles or their content changes, Kubelet keeps the file up-to-date.

By default, the Kubelet will prevent the pod from starting if the named ClusterTrustBundle is not found, or if `signerName` / `labelSelector` do not match any ClusterTrustBundles. If this behavior is not desired, then set the `optional` field to `true`, and the pod will start up with an empty file at `path`.
Copy link
Contributor

Choose a reason for hiding this comment

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

(a nit)

Suggested change
By default, the Kubelet will prevent the pod from starting if the named ClusterTrustBundle is not found, or if `signerName` / `labelSelector` do not match any ClusterTrustBundles. If this behavior is not desired, then set the `optional` field to `true`, and the pod will start up with an empty file at `path`.
By default, the kubelet prevents the pod from starting if the named ClusterTrustBundle is not found, or if `signerName` / `labelSelector` do not match any ClusterTrustBundles. If this behavior is not what you want, then set the `optional` field to `true`, and the pod will start up with an empty file at `path`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -71,6 +71,8 @@ For a reference to old feature gates that are removed, please refer to
| `AnyVolumeDataSource` | `false` | Alpha | 1.18 | 1.23 |
| `AnyVolumeDataSource` | `true` | Beta | 1.24 | |
| `AppArmor` | `true` | Beta | 1.4 | |
| `ClusterTrustBundle` | `false` | Alpha | 1.28 | |
| `ClusterTrustBundleProjection` | `false` | Alpha | 1.29 | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Please preserve the existing collation order (#41793 may help us automate sorting, but that has not yet landed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2023
@enj
Copy link
Member

enj commented Nov 21, 2023

/lgtm

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

LGTM label has been added.

Git tree hash: 627313af259e477714afa4b93392d4a52c4ba6b6

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.

Thanks

/approve

To use this feature in Kubernetes {{< skew currentVersion >}}, you must enable support for ClusterTrustBundle objects with the `ClusterTrustBundle` [feature gate](/docs/reference/command-line-tools-reference/feature-gates/) and `--runtime-config=certificates.k8s.io/v1alpha1/clustertrustbundles=true` kube-apiserver flag, then enable the `ClusterTrustBundleProjection` feature gate.
{{< /note >}}

The `clusterTrustBundle` projected volume source injects the contents of one or more ClusterTrustBundle objects as an automatically-updating file in the container filesystem.
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit / good to fix for beta)
That first mention of ClusterTrustBundle could link to https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/#cluster-trust-bundles

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -441,7 +442,8 @@ Each feature gate is designed for enabling/disabling a specific feature:
- `CloudDualStackNodeIPs`: Enables dual-stack `kubelet --node-ip` with external cloud providers.
See [Configure IPv4/IPv6 dual-stack](/docs/concepts/services-networking/dual-stack/#configure-ipv4-ipv6-dual-stack)
for more details.
- `ClusterTrustBundle`: Enable ClusterTrustBundle objects and kubelet integration.
- `ClusterTrustBundle`: Enable ClusterTrustBundle objects.
- `ClusterTrustBundleProjection`: Enable kubelet ClusterTrustBundle projected volume sources.
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit)

Suggested change
- `ClusterTrustBundleProjection`: Enable kubelet ClusterTrustBundle projected volume sources.
- `ClusterTrustBundleProjection`: Enable
[`clusterTrustBundle` projected volume sources](/docs/concepts/storage/projected-volumes#clustertrustbundle).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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 Nov 21, 2023
@sftim
Copy link
Contributor

sftim commented Nov 21, 2023

Netlify, our static site rendering and serving service, has got into a pickle @ahmedtd

If you edit the commit (can be a no-op other than, eg, a different commit date) and force-push, that might help unbreak things.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2023
@k8s-ci-robot k8s-ci-robot requested review from enj and liggitt November 21, 2023 17:22
@ahmedtd
Copy link
Contributor Author

ahmedtd commented Nov 21, 2023

OK, looks like netlify is succeeding now.

@enj
Copy link
Member

enj commented Nov 21, 2023

/lgtm

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

LGTM label has been added.

Git tree hash: 6868d9dece5b1b2bef78b68d130d1dbf4a8e5c2a

@k8s-ci-robot k8s-ci-robot merged commit 99df3a3 into kubernetes:dev-1.29 Nov 21, 2023
6 checks passed
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/auth Categorizes an issue or PR as relevant to SIG Auth. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/security Categorizes an issue or PR as relevant to SIG Security. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants