Allow specifying secret data using strings #28263

liggitt · 2016-06-30T03:08:01Z

This PR allows specifying non-binary data values in Secret objects as "stringData":{"key":"string value"}, in addition to the existing base64 []byte serializations in the data field.

On write, the keys and values in the stringData field are merged to the data map, overwriting any values already present in the data map. The move is one-way, the stringData field is never output when reading from the API.

A Secret can be created like this:

{
  "kind":"Secret",
  "apiVersion":"v1",
  "metadata":{"name":"mysecret"},
  "data":{
    "image":"<base64-encoded-jpg>"
  },
  "stringData":{
    "username": "myuser",
    "password": "mypassword"
  }
}

and when read from the API would look like this:

{
  "kind":"Secret",
  "apiVersion":"v1",
  "metadata":{"name":"mysecret",...},
  "data":{
    "image":"<base64-encoded-jpg>"
    "username": "bXl1c2Vy",
    "password": "bXlwYXNzd29yZA=="
  }
}

smarterclayton · 2016-06-30T03:22:37Z

pkg/api/v1/types.go

@@ -3089,6 +3089,13 @@ type Secret struct {
 	// Described in https://tools.ietf.org/html/rfc4648#section-4
 	Data map[string][]byte `json:"data,omitempty" protobuf:"bytes,2,rep,name=data"`

+	// StringData allows specifying non-binary secret data in string form.


API convention doc "stringData" as swagger reader would see it

smarterclayton · 2016-06-30T03:23:34Z

pkg/api/v1/types.go

+	// All keys and values are moved to the data field on write.
+	// It is never output when reading from the API.
+	// +genconversion=false
+	StringData map[string]string `json:"stringData,omitempty" protobuf:"bytes,4,rep,name=stringData"`


Tag 4 is correct (note to other reviewers) since this is a net new field in proto.

smarterclayton · 2016-06-30T03:25:34Z

I assume you'll apply this in the rest strategy?

liggitt · 2016-06-30T03:26:07Z

I applied it in conversion, so that the internal type never has to know stringData exists

thockin · 2016-06-30T03:26:34Z

pkg/api/v1/types.go

@@ -3089,6 +3089,13 @@ type Secret struct {
 	// Described in https://tools.ietf.org/html/rfc4648#section-4
 	Data map[string][]byte `json:"data,omitempty" protobuf:"bytes,2,rep,name=data"`

+	// StringData allows specifying non-binary secret data in string form.
+	// It is provided as a write-only convenience method.
+	// All keys and values are moved to the data field on write.


s/moved/merged/ ? I assume it merges if you have both fields. Who wins in case of colliding keys?

yeah, merged. stringData wins... it never exists when reading from the server, so any time it's set in a request to the server, the caller must have added it. that also lets patch work as intended (you can patch a field in either map and it takes effect)

I meant "update the comment" with that info :)

On Wed, Jun 29, 2016 at 8:28 PM, Jordan Liggitt notifications@github.com
wrote:

In pkg/api/v1/types.go
#28263 (comment)
:

@@ -3089,6 +3089,13 @@ type Secret struct {
// Described in https://tools.ietf.org/html/rfc4648#section-4
Data map[string][]byte json:"data,omitempty" protobuf:"bytes,2,rep,name=data"

// StringData allows specifying non-binary secret data in string form.

// It is provided as a write-only convenience method.

// All keys and values are moved to the data field on write.

yeah, merged. stringData wins... it's write-only, so any time you specify
it, you're saying you want it. that lets patch work as intended (you can
patch a field in either map and it takes effect)

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubernetes/pull/28263/files/ca0c1ab6efc5ab4ed34d3fcdeb5c126007fb4320#r69064660,
or mute the thread
https://github.com/notifications/unsubscribe/AFVgVPALGQmILLBCiC46qquBldHsmJZLks5qQzfhgaJpZM4JBwt9
.

liggitt · 2016-06-30T03:26:49Z

I could add the field to the internal type as well, and add validation, and apply it in BeforeCreate/BeforeUpdate, but I wasn't sure I wanted that field around internally... any strong reason not to do it in conversion?

thockin · 2016-06-30T03:27:28Z

I like this design better. It's an interesting precedent - write-only fields - but not too bizarre.,

smarterclayton · 2016-06-30T05:41:29Z

Agreed, write only isn't as bizarre as I was afraid of.

smarterclayton · 2016-06-30T05:43:46Z

That's fine, I guess it only means validation is a bit wierd until we do validation on external types.

ncdc · 2016-06-30T17:56:14Z

@liggitt test case?

liggitt · 2016-06-30T18:06:40Z

oops, had one and forgot to push it

ncdc · 2016-06-30T18:30:05Z

@bgrant0607 @thockin @smarterclayton are we in agreement that we can move forward with this?

k8s-bot · 2016-06-30T20:36:05Z

GCE e2e build/test passed for commit 993ab1d.

bgrant0607 · 2016-06-30T21:16:43Z

Thanks @liggitt @ncdc @smarterclayton. Yes, I'm happy with this approach.

ncdc · 2016-06-30T21:20:46Z

@sjenning can you prep a backport POC of this to your origin (it replaces the other string: secrets PR)?

smarterclayton · 2016-06-30T21:30:55Z

Lgtm

thockin · 2016-06-30T22:19:26Z

SGTM

On Thu, Jun 30, 2016 at 2:31 PM, Clayton Coleman notifications@github.com
wrote:

Lgtm

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28263 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AFVgVC72ngAGWqt-ciFZCalSehRkIZ0Pks5qRDW9gaJpZM4JBwt9
.

k8s-github-robot · 2016-07-01T06:54:45Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-07-01T07:27:42Z

GCE e2e build/test passed for commit 993ab1d.

k8s-github-robot · 2016-07-01T07:28:56Z

Automatic merge from submit-queue

transitive-bullshit · 2017-03-08T10:20:03Z

Is this available in the current version of k8? if so, is it documented anywhere?

liggitt · 2017-03-08T15:09:18Z

Is this available in the current version of k8? if so, is it documented anywhere?

Yes, it has been available for several releases. It is documented in the API documentation:
https://kubernetes.io/docs/api-reference/v1/definitions/#_v1_secret

transitive-bullshit · 2017-03-08T18:32:49Z

awesome -- missed that!

wilmayeung · 2019-04-10T17:01:46Z

The drawback of this approach is any secrets using stringData would be recognized as changed and a patch is needed when calling kubectl apply. Hence always resulted in "configured" even when nothing was changed.

googlebot added the cla: yes label Jun 30, 2016

liggitt mentioned this pull request Jun 30, 2016

Allow specifying secret values as strings #22059

Closed

k8s-github-robot assigned erictune Jun 30, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Jun 30, 2016

smarterclayton reviewed Jun 30, 2016
View reviewed changes

liggitt force-pushed the stringdata branch from ca0c1ab to b013670 Compare June 30, 2016 03:23

smarterclayton reviewed Jun 30, 2016
View reviewed changes

liggitt force-pushed the stringdata branch from b013670 to ef2a84d Compare June 30, 2016 03:24

thockin reviewed Jun 30, 2016
View reviewed changes

liggitt added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jun 30, 2016

liggitt force-pushed the stringdata branch from ef2a84d to a567919 Compare June 30, 2016 03:33

liggitt force-pushed the stringdata branch from a567919 to 2e1e435 Compare June 30, 2016 18:06

liggitt force-pushed the stringdata branch from 2e1e435 to c2b3935 Compare June 30, 2016 18:14

liggitt force-pushed the stringdata branch 2 times, most recently from ef338a0 to 993ab1d Compare June 30, 2016 20:03

Allow specifying secret data using strings

993ab1d

bgrant0607 assigned smarterclayton and unassigned erictune Jun 30, 2016

smarterclayton added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 30, 2016

sjenning mentioned this pull request Jul 1, 2016

UPSTREAM: 28263: Allow specifying secret data using strings openshift/origin#9663

Merged

k8s-github-robot merged commit 34244ef into kubernetes:master Jul 1, 2016

liggitt deleted the stringdata branch July 11, 2016 02:23

smarterclayton mentioned this pull request Aug 5, 2016

1.3 upstream picks openshift/origin#9790

Closed

mcristina422 mentioned this pull request Jun 1, 2020

Secrets applied with stringData show configured even when no changes made #87736

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying secret data using strings #28263

Allow specifying secret data using strings #28263

liggitt commented Jun 30, 2016 •

edited

Loading

smarterclayton Jun 30, 2016

smarterclayton Jun 30, 2016

smarterclayton commented Jun 30, 2016

liggitt commented Jun 30, 2016 •

edited

Loading

thockin Jun 30, 2016

liggitt Jun 30, 2016 •

edited

Loading

thockin Jun 30, 2016

liggitt commented Jun 30, 2016 •

edited

Loading

thockin commented Jun 30, 2016

smarterclayton commented Jun 30, 2016 via email

smarterclayton commented Jun 30, 2016 via email

ncdc commented Jun 30, 2016

liggitt commented Jun 30, 2016

ncdc commented Jun 30, 2016

k8s-bot commented Jun 30, 2016

bgrant0607 commented Jun 30, 2016

ncdc commented Jun 30, 2016

smarterclayton commented Jun 30, 2016

thockin commented Jun 30, 2016

k8s-github-robot commented Jul 1, 2016

k8s-bot commented Jul 1, 2016

k8s-github-robot commented Jul 1, 2016

transitive-bullshit commented Mar 8, 2017

liggitt commented Mar 8, 2017

transitive-bullshit commented Mar 8, 2017

wilmayeung commented Apr 10, 2019

Allow specifying secret data using strings #28263

Allow specifying secret data using strings #28263

Conversation

liggitt commented Jun 30, 2016 • edited Loading

smarterclayton Jun 30, 2016

Choose a reason for hiding this comment

smarterclayton Jun 30, 2016

Choose a reason for hiding this comment

smarterclayton commented Jun 30, 2016

liggitt commented Jun 30, 2016 • edited Loading

thockin Jun 30, 2016

Choose a reason for hiding this comment

liggitt Jun 30, 2016 • edited Loading

Choose a reason for hiding this comment

thockin Jun 30, 2016

Choose a reason for hiding this comment

liggitt commented Jun 30, 2016 • edited Loading

thockin commented Jun 30, 2016

smarterclayton commented Jun 30, 2016 via email

smarterclayton commented Jun 30, 2016 via email

ncdc commented Jun 30, 2016

liggitt commented Jun 30, 2016

ncdc commented Jun 30, 2016

k8s-bot commented Jun 30, 2016

bgrant0607 commented Jun 30, 2016

ncdc commented Jun 30, 2016

smarterclayton commented Jun 30, 2016

thockin commented Jun 30, 2016

k8s-github-robot commented Jul 1, 2016

k8s-bot commented Jul 1, 2016

k8s-github-robot commented Jul 1, 2016

transitive-bullshit commented Mar 8, 2017

liggitt commented Mar 8, 2017

transitive-bullshit commented Mar 8, 2017

wilmayeung commented Apr 10, 2019

liggitt commented Jun 30, 2016 •

edited

Loading

liggitt commented Jun 30, 2016 •

edited

Loading

liggitt Jun 30, 2016 •

edited

Loading

liggitt commented Jun 30, 2016 •

edited

Loading