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

retrospective add of RotateKubeletServerCertificate #3806

Conversation

SergeyKanzhelev
Copy link
Member

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 31, 2023
@k8s-ci-robot k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Jan 31, 2023
@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 31, 2023
Comment on lines +104 to +109
https://kubernetes.io/docs/reference/access-authn-authz/kubelet-tls-bootstrapping/#certificate-rotation

### Notes/Constraints/Caveats (Optional)

The note about default implementation that will not approve certificate requests
is already a part of documentation. See the Note on this page: https://kubernetes.io/docs/reference/access-authn-authz/kubelet-tls-bootstrapping/#certificate-rotation
Copy link
Contributor

Choose a reason for hiding this comment

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

Watch out that:

  • these URLs may go stale
  • we don't yet have a way to make a durable “permalink” to docs at a point in time

@enj
Copy link
Member

enj commented Feb 6, 2023

@SergeyKanzhelev are we targeting v1.27 for this retroactive KEP?

@SergeyKanzhelev
Copy link
Member Author

@SergeyKanzhelev are we targeting v1.27 for this retroactive KEP?

This PR only fills out what is done. I cannot commit on promoting metrics, writing a tests and docs. So ideally we want this PR merged and then separate PR that initiates GA-ing of this feature. Likely in next release

@enj
Copy link
Member

enj commented Feb 9, 2023

@SergeyKanzhelev are we targeting v1.27 for this retroactive KEP?

This PR only fills out what is done. I cannot commit on promoting metrics, writing a tests and docs. So ideally we want this PR merged and then separate PR that initiates GA-ing of this feature. Likely in next release

SGTM!

/milestone v1.28

@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone Feb 9, 2023
@aramase
Copy link
Member

aramase commented Apr 17, 2023

@SergeyKanzhelev Is this still planned for v1.28?

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 16, 2023
@enj enj removed this from the v1.28 milestone Sep 7, 2023
Copy link
Contributor

@kannon92 kannon92 left a comment

Choose a reason for hiding this comment

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

@SergeyKanzhelev What is needed to get this KEP merged?

I am tasked with leading the GA for RotateKubeletServerCertificate so I think this is the first step for this.

If you don't have the bandwidth, please feel free to give me access to your fork so I can push changes during the review process.

@sftim
Copy link
Contributor

sftim commented Dec 7, 2023

@kannon92 you can also copy this commit onto your own fork and iterate from there (ie, a separate PR)

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SergeyKanzhelev
Once this PR has been reviewed and has the lgtm label, please assign enj for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@kannon92
Copy link
Contributor

kannon92 commented Dec 7, 2023

/lgtm

I'll be taking over this work so our hope is that we can at least merge this as is and iterate.

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

It seems that there will be a lot more work needed to get this PR to merge as a even a restrospective.

I met with sig-auth and I will hopefully try and get some knowledge transfer around this work.
From what I understand, I will need to find authors of the PRs and poke them around design decisions made for this feature.

Metrics should be beta also.

Integration tests should be sufficient (cc @deads2k ).

I opened up #4411 so I can edit this with my research.

@SergeyKanzhelev I am going to close this PR.

/close

@k8s-ci-robot
Copy link
Contributor

@kannon92: Closed this PR.

In response to this:

It seems that there will be a lot more work needed to get this PR to merge as a even a restrospective.

I met with sig-auth and I will hopefully try and get some knowledge transfer around this work.
From what I understand, I will need to find authors of the PRs and poke them around design decisions made for this feature.

Metrics should be beta also.

Integration tests should be sufficient (cc @deads2k ).

I opened up #4411 so I can edit this with my research.

@SergeyKanzhelev I am going to close this PR.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants