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

csr: add expirationSeconds field to control cert lifetime #99494

Merged
merged 3 commits into from
Jul 2, 2021

Conversation

enj
Copy link
Member

@enj enj commented Feb 26, 2021

This change updates the CSR API to add a new, optional field called
expirationSeconds. This field is a request to the signer for the
maximum duration the client wishes the cert to have. The signer is
free to ignore this request based on its own internal policy. The
signers built-in to KCM will honor this field if it is not set to a
value greater than --cluster-signing-duration. The minimum allowed
value for this field is 600 seconds (ten minutes).

This change will help enforce safer durations for certificates in
the Kube ecosystem and will help related projects such as
cert-manager with their migration to the Kube CSR API.

Future enhancements may update the Kubelet to take advantage of this
field when it is configured in a way that can tolerate shorter
certificate lifespans with regular rotation.

Signed-off-by: Monis Khan mok@vmware.com

/kind feature
/kind api-change
/sig auth
/priority important-longterm
/triage accepted
@kubernetes/sig-auth-pr-reviews @kubernetes/sig-security-pr-reviews
/assign @mikedanese @liggitt @munnerz

Fixes #92678

xref: cert-manager/cert-manager#3646

The CertificateSigningRequest.certificates.k8s.io API supports an optional expirationSeconds field to allow the client to request a particular duration for the issued certificate.  The default signer implementations provided by the Kubernetes controller manager will honor this field as long as it does not exceed the --cluster-signing-duration flag.
- [KEP]: https://github.com/kubernetes/enhancements/issues/2784
- [Usage]: https://github.com/kubernetes/website/pull/28070

Special notes for your reviewer:

  • There are several TODOs in regards to minor details

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 26, 2021
@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. kind/feature Categorizes issue or PR as related to a new feature. sig/security Categorizes an issue or PR as relevant to SIG Security. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 26, 2021
@k8s-ci-robot k8s-ci-robot added area/apiserver area/kubectl area/kubelet area/test sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 26, 2021
@fejta-bot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@adisky adisky added this to Needs Reviewer in SIG Node PR Triage Feb 26, 2021
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 26, 2021
@k8s-ci-robot
Copy link
Contributor

@logicalhan: GitHub didn't allow me to request PR reviews from the following users: to, verify.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

csr_requested_duration_total would be more conventional
/cc @brancz to verify

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

enj added 3 commits July 1, 2021 23:38
This change updates the CSR API to add a new, optional field called
expirationSeconds.  This field is a request to the signer for the
maximum duration the client wishes the cert to have.  The signer is
free to ignore this request based on its own internal policy.  The
signers built-in to KCM will honor this field if it is not set to a
value greater than --cluster-signing-duration.  The minimum allowed
value for this field is 600 seconds (ten minutes).

This change will help enforce safer durations for certificates in
the Kube ecosystem and will help related projects such as
cert-manager with their migration to the Kube CSR API.

Future enhancements may update the Kubelet to take advantage of this
field when it is configured in a way that can tolerate shorter
certificate lifespans with regular rotation.

Signed-off-by: Monis Khan <mok@vmware.com>
Signed-off-by: Monis Khan <mok@vmware.com>
Signed-off-by: Monis Khan <mok@vmware.com>
@enj enj force-pushed the enj/i/not_after_ttl_hint branch from b5400d9 to 8d49502 Compare July 2, 2021 03:40
@liggitt
Copy link
Member

liggitt commented Jul 2, 2021

/retest

@liggitt
Copy link
Member

liggitt commented Jul 2, 2021

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, liggitt

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 Jul 2, 2021
@enj
Copy link
Member Author

enj commented Jul 2, 2021

/retest

@enj enj moved this from Needs Reviewer to Done in SIG Node PR Triage Jul 2, 2021
@enj enj moved this from In Review (v1.22) to Closed / Done in SIG Auth Old Jul 2, 2021
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jul 2, 2021

@enj: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubernetes-e2e-gce-alpha-features 8d49502 link /test pull-kubernetes-e2e-gce-alpha-features

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@enj
Copy link
Member Author

enj commented Jul 2, 2021

/retest

@k8s-ci-robot k8s-ci-robot merged commit 659c7e7 into kubernetes:master Jul 2, 2021
timebertt added a commit to timebertt/gardener that referenced this pull request Oct 14, 2021
timebertt added a commit to gardener/gardener that referenced this pull request Oct 14, 2021
* Upgrade to k8s.io/*@v0.22.2 in go.mod

* [automated] make revendor

* [automated] make generate

* [automated] make revendor

github.com/go-openapi/spec seems to be orphaned after previous make generate

* Upgrade to c-r@v0.10.2 in go.mod

Also, upgrade setup-envtest (doesn't have a tagged release yet, so use
release commit instead)

* [automated] make revendor

* Upgrade to controller-tools@v0.7.0 in go.mod

* [automated] make revendor

* Add missing WarningsOn{Create,Update} to rest strategies

* Replace dot imports for github.com/onsi/gomega/types

Fix linting errors: `Assertion` redeclared in this block (typecheck)

* Switch to typed values for WebhookInstallOptions.*Webhooks

ref kubernetes-sigs/controller-runtime#1626

* RequestCertificate now takes an optional requestedDuration

ref kubernetes/kubernetes#99494

* Switch to matchers.DeepEqual to test semantic equality

Maps (e.g. labels, selectors, resource requirements) might be sorted differently
than expected. Hence, use semantic equality instead of strict equality, as this
is what matters to us.
Also, DeepEqual outputs yaml and adds a nice diff indicator instead of printing
some large confusing go struct representation.

* Add new memorySwap field to expected kubelet config

ref kubernetes/kubernetes#102823

* Round condition.lastUpdateTime to seconds in test

There were several changes in the fake clients that might cause the failure
to happen just now.

* Correct unit tests falsely succeeding

These tests were not preparing the test objects correctly: they only updated
them in memory but not on the fake client. This wasn't caught until now
because the fake client mimicked the real json decoder, which didn't unset
fields not present on the server. Now that the fake client zeroes fields,
the tests started failing (which is correct). So fix the tests.

ref kubernetes-sigs/controller-runtime#1651

* Remove workarounds for missing zeroing in json decoder

Now that the c-r client zeroes fields before decoding into the object,
we can drop our workarounds for this, so basically drop
kutil.CreateResetObjectFunc and its usages.

ref kubernetes-sigs/controller-runtime#1640

* Drop setting webhook gvk explicitly in envtest

webhookConfig.SetGroupVersionKind is not needed anymore with
kubernetes-sigs/controller-runtime#1665

* Add some follow-up TODO comments

* [automated] make generate

but with go 1.16.9

* Address review comments
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
* Upgrade to k8s.io/*@v0.22.2 in go.mod

* [automated] make revendor

* [automated] make generate

* [automated] make revendor

github.com/go-openapi/spec seems to be orphaned after previous make generate

* Upgrade to c-r@v0.10.2 in go.mod

Also, upgrade setup-envtest (doesn't have a tagged release yet, so use
release commit instead)

* [automated] make revendor

* Upgrade to controller-tools@v0.7.0 in go.mod

* [automated] make revendor

* Add missing WarningsOn{Create,Update} to rest strategies

* Replace dot imports for github.com/onsi/gomega/types

Fix linting errors: `Assertion` redeclared in this block (typecheck)

* Switch to typed values for WebhookInstallOptions.*Webhooks

ref kubernetes-sigs/controller-runtime#1626

* RequestCertificate now takes an optional requestedDuration

ref kubernetes/kubernetes#99494

* Switch to matchers.DeepEqual to test semantic equality

Maps (e.g. labels, selectors, resource requirements) might be sorted differently
than expected. Hence, use semantic equality instead of strict equality, as this
is what matters to us.
Also, DeepEqual outputs yaml and adds a nice diff indicator instead of printing
some large confusing go struct representation.

* Add new memorySwap field to expected kubelet config

ref kubernetes/kubernetes#102823

* Round condition.lastUpdateTime to seconds in test

There were several changes in the fake clients that might cause the failure
to happen just now.

* Correct unit tests falsely succeeding

These tests were not preparing the test objects correctly: they only updated
them in memory but not on the fake client. This wasn't caught until now
because the fake client mimicked the real json decoder, which didn't unset
fields not present on the server. Now that the fake client zeroes fields,
the tests started failing (which is correct). So fix the tests.

ref kubernetes-sigs/controller-runtime#1651

* Remove workarounds for missing zeroing in json decoder

Now that the c-r client zeroes fields before decoding into the object,
we can drop our workarounds for this, so basically drop
kutil.CreateResetObjectFunc and its usages.

ref kubernetes-sigs/controller-runtime#1640

* Drop setting webhook gvk explicitly in envtest

webhookConfig.SetGroupVersionKind is not needed anymore with
kubernetes-sigs/controller-runtime#1665

* Add some follow-up TODO comments

* [automated] make generate

but with go 1.16.9

* Address review comments
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
* Upgrade to k8s.io/*@v0.22.2 in go.mod

* [automated] make revendor

* [automated] make generate

* [automated] make revendor

github.com/go-openapi/spec seems to be orphaned after previous make generate

* Upgrade to c-r@v0.10.2 in go.mod

Also, upgrade setup-envtest (doesn't have a tagged release yet, so use
release commit instead)

* [automated] make revendor

* Upgrade to controller-tools@v0.7.0 in go.mod

* [automated] make revendor

* Add missing WarningsOn{Create,Update} to rest strategies

* Replace dot imports for github.com/onsi/gomega/types

Fix linting errors: `Assertion` redeclared in this block (typecheck)

* Switch to typed values for WebhookInstallOptions.*Webhooks

ref kubernetes-sigs/controller-runtime#1626

* RequestCertificate now takes an optional requestedDuration

ref kubernetes/kubernetes#99494

* Switch to matchers.DeepEqual to test semantic equality

Maps (e.g. labels, selectors, resource requirements) might be sorted differently
than expected. Hence, use semantic equality instead of strict equality, as this
is what matters to us.
Also, DeepEqual outputs yaml and adds a nice diff indicator instead of printing
some large confusing go struct representation.

* Add new memorySwap field to expected kubelet config

ref kubernetes/kubernetes#102823

* Round condition.lastUpdateTime to seconds in test

There were several changes in the fake clients that might cause the failure
to happen just now.

* Correct unit tests falsely succeeding

These tests were not preparing the test objects correctly: they only updated
them in memory but not on the fake client. This wasn't caught until now
because the fake client mimicked the real json decoder, which didn't unset
fields not present on the server. Now that the fake client zeroes fields,
the tests started failing (which is correct). So fix the tests.

ref kubernetes-sigs/controller-runtime#1651

* Remove workarounds for missing zeroing in json decoder

Now that the c-r client zeroes fields before decoding into the object,
we can drop our workarounds for this, so basically drop
kutil.CreateResetObjectFunc and its usages.

ref kubernetes-sigs/controller-runtime#1640

* Drop setting webhook gvk explicitly in envtest

webhookConfig.SetGroupVersionKind is not needed anymore with
kubernetes-sigs/controller-runtime#1665

* Add some follow-up TODO comments

* [automated] make generate

but with go 1.16.9

* Address review comments
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. area/apiserver area/kubectl area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/security Categorizes an issue or PR as relevant to SIG Security. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: API review completed, 1.22
Archived in project
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

CSR API: cluster signing duration is not flexible and has unsafe defaulting
9 participants