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.

Sign up for GitHub

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

Jump to bottom

Re-organize certificate and key helper libraries #71004

Closed
awly opened this issue Nov 13, 2018 · 9 comments
Closed

Re-organize certificate and key helper libraries #71004

awly opened this issue Nov 13, 2018 · 9 comments
Assignees
@awly
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/auth Categorizes an issue or PR as relevant to SIG Auth.

Comments

@awly
Copy link
Contributor

awly commented Nov 13, 2018

The libraries with helper funcs in client-go and kubeadm are very dis-organized and have a lot of overlap. There's also many very specialized funcs that are not reusable.

List of current libraries:

Propose the following organization:

  • k8s.io/client-go/util/x509util - certificate helpers used by k8s clients
  • k8s.io/client-go/util/keyutil - key(pair) helpers used by k8s clients
  • k8s.io/kubernetes/pkg/util/x509util - internal-only certificate and CSR helpers used by control plane and native tools
  • k8s.io/kubernetes/pkg/util/keyutil - internal-only key(pair) helpers used by control plane and native tools
  • kubeadm and kubelet (like csr.RequestNodeCertificate) specialized code will live under their respective source trees

I'll work on incrementally moving towards that organization

/kind bug
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 13, 2018
@awly
Copy link
Contributor Author

awly commented Nov 13, 2018

/sig auth
/assign awly
CC @mikedanese @liggitt

@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 13, 2018
This was referenced Nov 13, 2018
Merged
Merged
Merged
Merged
@mikedanese mikedanese added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Nov 13, 2018
@mikedanese
Copy link
Member

Ideal, client-go has only what we need to support the client. We should be very careful with what we expose outside of that package.

@neolit123
Copy link
Member

cc @luxas

@liggitt liggitt removed the kind/bug Categorizes issue or PR as related to a bug. label Nov 14, 2018
@awly awly mentioned this issue Dec 10, 2018
Merged
@mourya007
Copy link

@awly @mikedanese What we need to in this library https://godoc.org/k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil

@awly
Copy link
Contributor Author

awly commented Jan 17, 2019

@mourya007 https://godoc.org/k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil has some overlap with the other libraries. It is also kubeadm-specific but has several dependencies in client-go that nobody else uses.

I want to pull anything in client-go that only kubeadm uses into pkiutil.
If stuff in client-go isn't obviously useful to a k8s client (managing a PKI is not, for example) it should land somewhere in k8s.io/kubernetes/pkg or /cmd.

@mourya007
Copy link

@awly Can you give me one example for what exactly we need to do in this library https://godoc.org/k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil ?

@awly
Copy link
Contributor Author

awly commented Jan 22, 2019

@mourya007 for example move these kubeadm-only functions into pkiutil:

@mourya007 mourya007 mentioned this issue Jan 23, 2019
Merged
awly pushed a commit to awly/kubernetes that referenced this issue Feb 19, 2019
Extract new keyutil package from client-go/util/cert
1845839 
This package contains public/private key utilities copied directly from
client-go/util/cert. All imports were updated.

Future PRs will actually refactor the libraries.

Updates kubernetes#71004
@liggitt liggitt added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Mar 14, 2019
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 Jun 12, 2019
@mikedanese
Copy link
Member

Is this fixed?

@rajathagasthya rajathagasthya mentioned this issue Nov 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@awly awly

Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@neolit123 @liggitt @awly @mikedanese @k8s-ci-robot @fejta-bot @mourya007