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

Strip out Status and other readonly fields from user's POST/PUT #3005

Closed
dchen1107 opened this issue Dec 17, 2014 · 16 comments
Closed

Strip out Status and other readonly fields from user's POST/PUT #3005

dchen1107 opened this issue Dec 17, 2014 · 16 comments
Labels
area/api Indicates an issue on api area. area/usability priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@dchen1107
Copy link
Member

Forked from #1370

We need to strip out Status info that users's POST/PUT. We've seen examples of users doing this, and config solutions will make it even more common to POST/PUT data returned by GET, including Status.

cc/ @bgrant0607

@bgrant0607 bgrant0607 added area/api Indicates an issue on api area. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Dec 17, 2014
@bgrant0607
Copy link
Member

Ideally, this would be done in a generic way in the apiserver's resthandler. One idea I have is to use readonly field tags (#2970) to identify fields that should be dropped on POST/PUT.

@bgrant0607
Copy link
Member

#1821 is a similar issue. uid is readonly.

@brendandburns
Copy link
Contributor

@nikhiljindal are you working on this?

@bgrant0607
Copy link
Member

Not yet, AFAIK.

@nikhiljindal
Copy link
Contributor

Yes not working on this right now.
I have a few other P1s that I am looking at right now.
Will pick up this one if no one has picked it by the time I get others done.

@ddysher
Copy link
Contributor

ddysher commented Jan 27, 2015

I was planning to work on similar things once #3733 goes in. Also see #2726.

If we drop out Status info, then we'll need to figure out how to convey status information from k8s component, e.g. controller -> apiserver (for node status), kublet -> apiserver (for pod status), etc. We probably need another endpoint, or filter based on authorization policy.

The issus is marked P1, any concern for 20%er to work on it? I saw P1 should be delivered in 2 weeks....

@smarterclayton
Copy link
Contributor

I've got part of this (creates) that I'm working on. Update is harder

@ddysher
Copy link
Contributor

ddysher commented Jan 27, 2015

@smarterclayton Any pointer to your work? so I can stand on the shoulders of giants : )

@smarterclayton
Copy link
Contributor

Going to rebase shortly, Tim had some feedback I'm trying to address.

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

@smarterclayton Any pointer to your work? so I can stand on the shoulders of
giants : )


Reply to this email directly or view it on GitHub:
#3005 (comment)

@smarterclayton
Copy link
Contributor

I'm in here anyway, I think I'm going to do the bare minimum and clean it up now. Will appreciate review.

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

Going to rebase shortly, Tim had some feedback I'm trying to address.

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

@smarterclayton Any pointer to your work? so I can stand on the shoulders
of
giants : )


Reply to this email directly or view it on GitHub:
#3005 (comment)

@smarterclayton
Copy link
Contributor

Finding some horrible things as I test this... as in, pod updates can change names under the covers if it hasn't been scheduled yet, you can clear PortalIP on a Service update (it looks intentional, but seems bad), and we apparently depend on some subset of the pod status info being stored in etcd.

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

I'm in here anyway, I think I'm going to do the bare minimum and clean it up
now. Will appreciate review.

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

Going to rebase shortly, Tim had some feedback I'm trying to address.

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

@smarterclayton Any pointer to your work? so I can stand on the
shoulders
of
giants : )


Reply to this email directly or view it on GitHub:
#3005 (comment)

@derekwaynecarr
Copy link
Member

Can we chat on this tomorrow? I am currently using a Status on an object to update the status of another in #3796. I can easily make it that a spec of object A is what is used to update the status object B if we truly intend to block Status on all PUT. If we go that route, we basically always require a mirror object pattern where spec of one object is status of other.

Sent from my iPhone

On Jan 27, 2015, at 7:47 PM, Clayton Coleman notifications@github.com wrote:

Finding some horrible things as I test this... as in, pod updates can change names under the covers if it hasn't been scheduled yet, you can clear PortalIP on a Service update (it looks intentional, but seems bad), and we apparently depend on some subset of the pod status info being stored in etcd.

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

I'm in here anyway, I think I'm going to do the bare minimum and clean it up
now. Will appreciate review.

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

Going to rebase shortly, Tim had some feedback I'm trying to address.

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

@smarterclayton Any pointer to your work? so I can stand on the
shoulders
of
giants : )


Reply to this email directly or view it on GitHub:
#3005 (comment)


Reply to this email directly or view it on GitHub.

@smarterclayton
Copy link
Contributor

I don't think my initial changes will be as strict as described here. I think we need to reach closure among authz of updates, proper api behavior, and patterns before we fix this or introduce more status writing. Let's circle these wagons.

On Jan 27, 2015, at 9:25 PM, Derek Carr notifications@github.com wrote:

Can we chat on this tomorrow? I am currently using a Status on an object to update the status of another in #3796. I can easily make it that a spec of object A is what is used to update the status object B if we truly intend to block Status on all PUT. If we go that route, we basically always require a mirror object pattern where spec of one object is status of other.

Sent from my iPhone

On Jan 27, 2015, at 7:47 PM, Clayton Coleman notifications@github.com wrote:

Finding some horrible things as I test this... as in, pod updates can change names under the covers if it hasn't been scheduled yet, you can clear PortalIP on a Service update (it looks intentional, but seems bad), and we apparently depend on some subset of the pod status info being stored in etcd.

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

I'm in here anyway, I think I'm going to do the bare minimum and clean it up
now. Will appreciate review.

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

Going to rebase shortly, Tim had some feedback I'm trying to address.

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

@smarterclayton Any pointer to your work? so I can stand on the
shoulders
of
giants : )


Reply to this email directly or view it on GitHub:
#3005 (comment)


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

@smarterclayton
Copy link
Contributor

#3789 adds a framework for create that allows each type to specify its reset rules on create. We can now add a similar behavior on update.

@bgrant0607 bgrant0607 added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Feb 5, 2015
@bgrant0607 bgrant0607 changed the title Strip out Status info from user's POST/PUT Strip out Status and other readonly fields from user's POST/PUT Feb 5, 2015
@bgrant0607 bgrant0607 modified the milestone: v1.0 Feb 6, 2015
@smarterclayton
Copy link
Contributor

#4248 is the first bit of allowing PUT /status

@lavalamp lavalamp removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Mar 3, 2015
@smarterclayton
Copy link
Contributor

This is fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Indicates an issue on api area. area/usability priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

8 participants