kubernetes / kubectl

Issue tracker and mirror of kubectl code
Apache License 2.0
2.89k stars 924 forks source link

Add kubectl create secret --from-base64-literal and --random-value-for-key #1654

Open ash2k opened 2 months ago

ash2k commented 2 months ago

What would you like to be added?

kubectl create secret --from-literal=key=secret-value is very handy. I have a use case where I'm creating a secret that needs to be binary (an encryption key), be of a certain size, and use all of those bits for entropy. I'm taking randomness from /dev/urandom but cannot use it as a literal value since it's binary.

I could put the secret material into a file and use kubectl create secret --from-file=./file but:

Ideally I'd use something like kubectl create secret --from-base64-literal="key=$(head -c 64 /dev/urandom | base64)" to generate a value of 64 bytes, base64 encode it, pass to kubectl. It would decode it, persist into the cluster.

Yet another idea (can open a separate issue) is to have --random-value-for-key=key=64. It would generate a random value of length 64 and persist it into the key key. Even nicer and more secure (secret is not exposed on disk and via command line flags, where it can be seen by other processes) for bootstrapping a secret (from a script or console).

/sig cli

Why is this needed?

Better scripting experience, more secure secret data handling.

k8s-ci-robot commented 2 months ago

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
xuzhenglun commented 2 months ago

/assign

maybe it is festiable by using stdin device, here is my workaround.

head -c 64 /dev/urandom | kubectl create secret genetic my-secret --from-file=/dev/stdin

EDIT: Although there is a workaround, I still vote to adding this flag.

  1. This method seems not portiable in Windows(Sorry I'm not familiar with Windows, correct me if I'm wrong).
  2. Flag way is simpler and have better experience.
xuzhenglun commented 2 months ago

WDYT about thie feature? @eddiezane

Do you want to implement this @ash2k ? If I may, once this feature is accepted, I'm happy to raise a PR to make it happen.

ash2k commented 2 months ago

Do you want to implement this @ash2k ?

Not anytime soon. I need the feature but don't have capacity to build it right now.

ak20102763 commented 2 months ago

Hi @ash2k / @xuzhenglun , if this feature is accepted, kubectl create secret generic --random-value-for-key=key=64 this could be one way to add it. Developed and tested this, let me know if I can contribute.

Bhargav-manepalli commented 2 months ago

Hi @xuzhenglun / @ash2k,

I have worked on this feature: kubectl create secret generic --random-value-for-key=key=64.
I am ready to contribute my implementation via a pull request.

xuzhenglun commented 2 months ago

@Bhargav-manepalli / @ak20102763

Don't worry, I'm fine if you want to take over it.

But I'm not sure that such change will be accepted, You would be better to ensure this with approver of sig-cli. IMHO, flag likes --random-value-for-key could lack of versatility:

  1. Is the randomness cryptographically random or just ordinary pseudo-random. If people want to decide how to generate the data, maybe --from-base64-literal is better.
  2. This semantics is not explicitly shown in the flag name, so 64 seems not very clear about the length of random bytes.
ardaguclu commented 2 months ago

/transfer kubectl

ak20102763 commented 2 months ago

@xuzhenglun I understand your concern, I am using crypto/rand package that generates high entropy cryptographic randomness, which can be suitable to generate keys, tokens. And if user needs just high entropy random bytes irrespective of how it is created, generating it through secret creation and storing it would give better experience. Though as you said, user may want to decide how to generate data, then --from-base64-literal would be a better option.

And here is more information about this flag: --random-value-for-key=key=size Specify a key and size to generate random binary value from crypto/rand package (e.g., --random-value-for-key=key=64). The size should be a positive integer representing the number of bytes.

Bhargav-manepalli commented 2 months ago

@xuzhenglun , I've implemented the --from-base64-literal=<base64 encoded key> flag in kubectl. With this flag, when creating a secret, the secret will be created with the encoded value. The logic is handled in kubectl, where the base64-encoded string from the CLI arguments is decoded and passed as the decoded key to the API server. The API server will then create an encoded secret for us. This ensures flexibility for users while maintaining proper handling of encoded secrets.

xuzhenglun commented 2 months ago

That’s really awesome, thanks for your contribution. Feel free to raise a PR and link with this issue when you are ready. @Bhargav-manepalli

/assign @Bhargav-manepalli /unassign

Bhargav-manepalli commented 2 months ago

Thanks @ak20102763 , @xuzhenglun I've submitted a pull request that implements the --from-base64-literal flag for kubectl create secret generic. The implementation includes the new flag, associated logic, unit tests. I'd appreciate your review and feedback.

ardaguclu commented 2 months ago

I think this should be triaged by sig-cli first to move forward. Because adding new flag requires strong justification.

ardaguclu commented 2 months ago

There is a workaround as stated in https://github.com/kubernetes/kubectl/issues/1654#issuecomment-2346265086

head -c 64 /dev/urandom | kubectl create secret genetic my-secret --from-file=/dev/stdin

and this workaround is also a reasonable and looks not hacky.

We usually would consider adding more flags, if there is no other way around and in that case there is.

Bhargav-manepalli commented 2 months ago

Hi @xuzhenglun , I raised a PR. Any comments on this ?

xuzhenglun commented 2 months ago

Added --from-base64-literal feature to secret kubernetes/kubernetes#127609

@Bhargav-manepalli
I'm afraid you have to give a good reason to convince sig-cli of the necessity of this parameter. AFAIK, for signle key with random data, reading from stdin is a fine solution. maybe mulitiple key with random value? But I wander is this a real world scenario? 🤔 💭

ash2k commented 2 months ago

It's a good workaround, but it only works if you need a secret with a single key/value.

Bhargav-manepalli commented 2 months ago

@ash2k , It supports for multiple key:value 's as well. i mean multiple keys with encoded values. kubectl create secret generic my-secrey --from-base64-literal=user=<encoded string> --from-base64-literal=pass=<encoded string>

ash2k commented 2 months ago

I was talking about the workaround :)

ak20102763 commented 2 months ago

@ash2k / @xuzhenglun , how about this workaround , this way we can have multiple key/value pairs in single secret

kubectl create secret generic my-secret --from-file=key1=<(head -c 64 /dev/urandom) --from-file=key2=<(head -c 128 /dev/urandom) --from-file=key3=<(head -c 64 /dev/urandom)

ash2k commented 2 months ago

@ak20102763 Sure. But why make the user do this vs have flags that do what's necessary? I think working with secrets is quite important and default kubectl experience should be fool-proof as much as possible (mostly to avoid leaking secret data). So in my mind it makes sense to provide a clean UX for this.

I think it's not about "possible to do" vs "impossible to do without the flag(s)". It's about helping the user to avoid leaking the secret data. This is way more important and should be prioritized. Most people will not know all these shell tricks (I didn't!), and they probably don't work on Windows either. Users will just put their secret data into a file(s), which is strictly less secure as it exposes it on disk.

Again, we have those specialized kubectl create commands for better UX, not because one cannot do the same thing without them. It's about UX, not possible/impossible.

Hence, I think both --from-base64-literal and --random-value-for-key should be added for the best cli/scripting UX/security.

Does this make sense? WDYT?

ak20102763 commented 2 months ago

@ash2k I agree, we have kubectl create commands just to give better UX, and the reason I thought --random-value-for-key would be better option is its ease of use, user just have to give number of bytes he/she wants in the secret and it will be created. This way user doesn’t even need to know any specialised commands to create binary data, and it will be applicable across platforms. It’s is also more secure in the way it creates secret as it doesn’t need any data from the disk.

Bhargav-manepalli commented 2 months ago

@ash2k , The test pull-kubernetes-verify is failing with error command "kubectl create secret generic": flag name "from-base64-literal" is invalid, long form of flag names can only contain lowercase characters or dash (must match ^[a-z]+[a-z\-]*$) . Because, currently it's only accepting lowercases and dashes. https://github.com/Bhargav-manepalli/kubernetes/blob/10d393a4f8f946753b7b2ed7e017e0a907e51e0f/staging/src/k8s.io/kubectl/pkg/cmd/util/sanity/cmd_sanity.go#L147

Either to change the flag-name or add numeric support to the flag ?

ash2k commented 2 months ago

@Bhargav-manepalli I'm not on sig-cli, cannot make any decisions. But in my personal opinion numbers should be allowed in flag names, I don't see why that would be bad in general.

Alternatively, what are the good names for the flag if not --from-base64-literal? 🤔

Bhargav-manepalli commented 2 months ago

@Bhargav-manepalli I'm not on sig-cli, cannot make any decisions. But in my personal opinion numbers should be allowed in flag names, I don't see why that would be bad in general.

Alternatively, what are the good names for the flag if not --from-base64-literal? 🤔

Any suggestions on this ? @xuzhenglun

xuzhenglun commented 2 months ago

Any suggestions on this ? @xuzhenglun

In my personal opinion, I think this new flag could bring user better experience indeed. But I'm new and not on charge, cannot make decision too.

Would you please help us to triage this? @ardaguclu

ardaguclu commented 2 months ago

As I tried to state in here https://github.com/kubernetes/kubectl/issues/1654#issuecomment-2360657615, we are hesitant to add new flags and it is considered only for the cases where there is no workaround. Because usability of a flag may seem reasonable from one point of view but it may also lead to a bad user experience from another point of view (for example, numerous flags in one command, just like run has not good UX).

In that case there are workarounds for every possible case and I'd not prefer adding a new flag for an imperative command (i.e. create secret). But let's leave this issue as open for now to get more feedback from the community.

valid-var commented 4 weeks ago

I vote for --from-base64-literal or --from-encoded-literal flag, as it can be real handy for passing any abnormal symbols, such as tabs, newlines, etc and basically any byte data. Although passing data through stdin has proven to be a valid workaround, this flag might make kubectl a bit more user-friendly