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

Adjust the directory layout of the bootstraptoken API to standard form #2903

Closed
SataQiu opened this issue Jul 13, 2023 · 4 comments
Closed
Labels
kind/design Categorizes issue or PR as related to design. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Milestone

Comments

@SataQiu
Copy link
Member

SataQiu commented Jul 13, 2023

Is this a BUG REPORT or FEATURE REQUEST?

FEATURE REQUEST

Versions

kubeadm version (use kubeadm version):

Environment:

  • Kubernetes version (use kubectl version):
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Container runtime (CRI) (e.g. containerd, cri-o):
  • Container networking plugin (CNI) (e.g. Calico, Cilium):
  • Others:

What happened?

I found that the directory layout of the bootstraptoken API is very simple:

.
├── doc.go
└── v1
    ├── defaults.go
    ├── types.go
    ├── utils.go
    ├── utils_test.go
    └── zz_generated.deepcopy.go

No internal types were defined.
It does not even contain something about runtime scheme registration, and it's not like a completed API package.
But we've already defined the group name bootstraptoken.kubeadm.k8s.io for it.

A standard API package layout looks something like this:

.
├── doc.go
├── register.go
├── types.go
├── fuzzer
│   ├── fuzzer.go
│   └── fuzzer_test.go
├── scheme
│   └── scheme.go
├── v1betaxx
│   ├── conversion.go
│   ├── defaults.go
│   ├── doc.go
│   ├── register.go
│   ├── types.go
│   ├── zz_generated.conversion.go
│   ├── zz_generated.deepcopy.go
│   └── zz_generated.defaults.go
├── validation
│   ├── xxx.go
└── zz_generated.deepcopy.go

So, shall we consider adjusting the bootstraptoken directory layout to a standard form?
Or did we design it that way on purpose?

Waiting for more feedback.

What you expected to happen?

Adjust the directory layout of the bootstraptoken API to standard form.

How to reproduce it (as minimally and precisely as possible)?

Anything else we need to know?

@neolit123
Copy link
Member

neolit123 commented Jul 13, 2023

i think it was mainly me who worked on the token package.
frankly, i tried to keep it very simple and not introduce the more common API layout.

my vote is to continue to keep it simple / the way it is and only introduce the private type and conversion if we add v2 in the future.

fuzzing and validation seems nice, but given kubeadm is the only consumer for now this is already done in the kubeadm fuzzer / validation, if i am not mistaken. we could add the dedicated fuzzing / validation in the token package, but IIUC it will cause (some) duplication. that's not necessarily bad , so if you think we can improve the package without making it complex for no reason - we can do that.

@neolit123 neolit123 added priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. kind/design Categorizes issue or PR as related to design. labels Jul 13, 2023
@neolit123 neolit123 added this to the v1.28 milestone Jul 13, 2023
@SataQiu
Copy link
Member Author

SataQiu commented Jul 14, 2023

I see. Thank you @neolit123
+1 for introduce the private type and conversion if we add v2 in the future

@neolit123 neolit123 modified the milestones: v1.28, v1.29 Jul 21, 2023
@SataQiu
Copy link
Member Author

SataQiu commented Aug 23, 2023

/close

@k8s-ci-robot
Copy link
Contributor

@SataQiu: Closing this issue.

In response to this:

/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
kind/design Categorizes issue or PR as related to design. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
None yet
Development

No branches or pull requests

3 participants