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

Do not automatically decode runtime.RawExtension #7490

Merged
merged 1 commit into from May 1, 2015

Conversation

smarterclayton
Copy link
Contributor

Make clients opt in to decoding objects that are stored in the generic api.List object by invoking runtime.DecodeList() with a set of schemes. Makes it easier to handle unknown schema objects because decoding is in the control of the code.

Change

A caller who is aware of the api.List type (or any object that has a runtime.RawExtension -> runtime.Object conversion) must also be aware that the runtime.Objects they receive may be unprocessed.

The type they get is *runtime.Unknown (as before for a type not in the schema), but even objects in the schema are left unprocessed.

To make this easier, I added runtime.DecodeList which walks a []runtime.Object and looks for unrecognized objects. Hypothetical call pattern is:

obj, _ := codec.Decode(data)
list, _ := runtime.ExtractList(obj)
runtime.Decode(list, <scheme1>, <otherschemes>)

The new "unstructured type" is itself a scheme, so you could do:

runtime.Decode(list, api.Scheme, config.Scheme, UnstructuredJSONScheme)

which would result in all *runtime.Unknown being converted to objects in api.Scheme, config.Scheme, or turned into *runtime.Unstructured which is map[string]interface{}

Also added a number of interfaces to runtime/interfaces which starts to tease apart conversion and decoding (and cleaned up a few places where that code was missing).

Fixes #7070

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@smarterclayton
Copy link
Contributor Author

@lavalamp I would appreciate your review here given familiarity with scheme.

@smarterclayton
Copy link
Contributor Author

Or @nikhiljindal or @deads2k

@lavalamp lavalamp self-assigned this Apr 29, 2015
if j.Items == nil {
j.Items = []runtime.Object{}
}
// TODO: uncomment when round trip starts from a versioned object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: prefer if false { to prevent bitrot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

----- Original Message -----

@@ -115,22 +115,24 @@ func FuzzerFor(t *testing.T, version string, src
rand.Source) *fuzz.Fuzzer {
},
func(j *api.List, c fuzz.Continue) {
c.FuzzNoCustom(j) // fuzz self without calling this function again

  •       if j.Items == nil {
    
  •           j.Items = []runtime.Object{}
    
  •       }
    
  •       // TODO: uncomment when round trip starts from a versioned object
    

nit: prefer if false { to prevent bitrot.


Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/7490/files#r29353705

Make clients opt in to decoding objects that are stored
in the generic api.List object by invoking runtime.DecodeList()
with a set of schemes. Makes it easier to handle unknown
schema objects because decoding is in the control of the code.

Add runtime.Unstructured, which is a simple in memory
representation of an external object.
@smarterclayton
Copy link
Contributor Author

Anything else? Travis was a flake, rerunning it now

@googlebot
Copy link

CLAs look good, thanks!

@smarterclayton
Copy link
Contributor Author

Ping

@smarterclayton
Copy link
Contributor Author

Will trade favors for review... :)

// binary representation of an object.
type ObjectCodec interface {
ObjectEncoder
Decoder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjectDecoder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does both sides (decode and encode) which we call a codec elsewhere - this is the more powerful interface (let's you pick output version).

On May 1, 2015, at 12:06 PM, Daniel Smith notifications@github.com wrote:

In pkg/runtime/interfaces.go:

+// ObjectScheme represents common conversions between formal external API versions
+// and the internal Go structs. ObjectScheme is typically used with ObjectCodec to
+// transform internal Go structs into serialized versions. There may be many valid
+// ObjectCodecs for each ObjectScheme.
+type ObjectScheme interface {

  • ObjectConvertor
  • ObjectTyper
  • ObjectCreater
  • ObjectCopier
    +}

+// ObjectCodec represents the common mechanisms for converting to and from a particular
+// binary representation of an object.
+type ObjectCodec interface {

  • ObjectEncoder
  • Decoder
    ObjectDecoder?


Reply to this email directly or view it on GitHub.

@lavalamp
Copy link
Member

lavalamp commented May 1, 2015

LGTM-- sorry, have had limited availabliity.

@smarterclayton
Copy link
Contributor Author

Sorry to nag, trying to lay the groundwork for clients that don't transform the object.

On May 1, 2015, at 12:07 PM, Daniel Smith notifications@github.com wrote:

LGTM-- sorry, have had limited availabliity.


Reply to this email directly or view it on GitHub.

@smarterclayton
Copy link
Contributor Author

Rerruning Travis

smarterclayton added a commit that referenced this pull request May 1, 2015
Do not automatically decode runtime.RawExtension
@smarterclayton smarterclayton merged commit 1a8845a into kubernetes:master May 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: change how api.List is handled internally
3 participants