Proposal: Allow easy secret manipulation from string data

[smarterclayton]

Secrets today have schema Data map[string][]byte json:"data,omit empty". The end result is a base64 string in JSON, which is hard to work with as a normal client on the cli. It should be possible to pass string values for the data that are transformed into []byte. Since v2 is far away, this should be backwards compatible.

Proposal

Add a new StringData map[string]string with json identifier stringData that is read during create and update and merged into data. This is backwards compatible (new clients can send both fields and stringData takes precedence). Clients always read data. stringData is not stored in etcd.

Alternatives:

1. Add a second new map that is not merged - not backwards compatible
2. Add a subresource that takes data as strings - not easily sent via kubectl
3. Add a new top level resource SecretMap that is backed by Secret and has data map[string]string, but operations on SecretMap. Not easily used (requires new client work), also has potential for confusion, and in the long run we need both binary and text data for secrets anyway.
   a. Variant 1 - add a new SecretMap that has both fields, or a deeper struct, and convince everyone to change away from secret. SecretMap would be backed in etcd by Secret resources. Problem: unable to have fields that are binary and fields that are string.

[bgrant0607]

The stringData proposal is different from how anything works today.

What's the problem with the subresource? That seems analogous to server-side template substitution.

[smarterclayton]

Primarily we have no way to express a subresource in the cli or in config files. I think this is something we want in config files. We could simply support a virtual resource that fronts the secret (a new top level resource like StringSecret) which is saved as secret, but I don't know that is a pattern I'd want to start now.

[bgrant0607]

Ok, now we have precedent for the approach in this proposal: rollbackTo #19686. It would sidestep the madness we encountered with PodSecurityContext by storing only one copy.

However, it wouldn't work with kubectl apply, and would likely wreak havok with Template as well -- anything that checks structural equality, so it doesn't solve the "config file" use case.

So, I think we're back to a bidirectional sync of the 2 maps. On update (PUT, PATCH), we'd need to diff with the state in etcd to figure out which map entries were modified and then change the other to match if the entries differed. If both were changed but didn't match, we could throw a validation error, I think.

[smarterclayton]

So another option I thought of last night... we only accept Base64 input to
those fields today. Anything that is not base64 will error out. We could
allow someone to do something similar to the formatting described elsewhere:

{ "data": {
  "key": "base64(some stuff that should be base64)"
  }
}

That is a) unambiguous and b) backwards compatible for new servers and c)
will fail cleanly for new clients talking to old servers

On Fri, Jan 29, 2016 at 12:12 PM, Brian Grant notifications@github.com
wrote:

Ok, now we have precedent for the approach in this proposal: rollbackTo
#19686 #19686. It would
sidestep the madness we encountered with PodSecurityContext by storing only
one copy.

However, it wouldn't work with kubectl apply, and would likely wreak havok
with Template as well -- anything that checks structural equality, so it
doesn't solve the "config file" use case.

So, I think we're back to a bidirectional sync of the 2 maps. On update
(PUT, PATCH), we'd need to diff with the state in etcd to figure out which
map entries were modified and then change the other to match if the entries
differed. If both were changed but didn't match, we could throw a
validation error, I think.

—
Reply to this email directly or view it on GitHub
#19575 (comment)
.

[smarterclayton]

That was supposed to be:

{
  "data": {
    "key1": "base64(some human readable text)",
    "key2": "abcothDEf234nthsna==",
  }
}

key1 is unambiguously not base64, will fail if sent to old servers (or should) and is easy to understand what it is doing (it could be text(or string(). This would fit the human expectations for a templating system very well, while still preserving the existing behavior.

[smarterclayton]

Another option is:

{
  "data": {
    "string(key)": "....",
  }
}

[liggitt]

I wouldn't want to change the map key. An alternate representation in the map value (e.g. string:foobarbaz) that faciliated a one-way conversion to the base64 form would be useful. Quick proof-of-concept in #22059

[smarterclayton]

This was implemented