support Deprecated field #275

Jefftree · 2022-01-18T21:38:03Z

go-restful is at version v2.9.5 in k/k already: https://github.com/kubernetes/kubernetes/blob/master/go.mod#L36

This PR supports publishing the deprecated field for an operation in both OpenAPI v2 and v3.

See kubernetes/kubernetes#107622 for how swagger.json changes with this update.

See kubernetes/kubernetes#107498 for more details on the issue.

/cc @apelisse @liggitt @sttts

go.mod

liggitt · 2022-01-18T21:43:02Z

pkg/builder3/openapi.go

 }

 func (o *openAPI) buildOperations(route common.Route, inPathCommonParamsMap map[interface{}]*spec3.Parameter) (*spec3.Operation, error) {
 	ret := &spec3.Operation{


for openapi v3, is it possible to set the deprecated field on the schema object as well as the operation?

Yes, that'd be super sweet, especially if we automatically enable new mechanisms to help with deprecation.

Yeah it should possible but it's not as straightforward and I thought it might be better to include in a follow up after some more discussion.

I'm still doing some investigations, but afaik we can't guarantee that a deprecated operation path would deprecate all the corresponding schemas associated with that path. An example of this is a delete on an alpha/beta resource that returns a meta.v1.Status. Is this a one-off exception or are there more examples of this?

For schema fields, there are cases where we deprecate a single schema field and not the entire schema. This is currently embedded in field descriptions, and it may be beneficial to include these programmatically as well.

afaik we can't guarantee that a deprecated operation path would deprecate all the corresponding schemas associated with that path

IIUC, it shouldn't. Because an operation is deprecated doesn't mean that the type that it received is also deprecated.

Thanks for the perspective, I agree that it'd be nice to have deprecated either for entire types (v1beta1 type is deprecated) or for fields, but we would probably need new markers and what not, probably out of the scope of this project (but the types of things that are enabled by this project).

If you look at how this deprecated operation setting is being driven from k/k, it's based on the entire primary struct for a set of operations being deprecated. Even if it's in another PR, having a way to plumb the fact that that struct is deprecated into openapi would be useful.

OK, thanks for clarifying, I completely agree.

austince · 2022-05-09T16:03:55Z

pkg/common/interfaces.go

+	// Deprecated marks a route as deprecated
+	Deprecated() bool


Will this be released as a breaking change? iirc, we were quite careful when adding this interface to not break backwards compatibility, suggest doing the same here. ref: #252 (comment)

I did a quick scan of the k/k codebase, it looks like nothing is using the particular interface and all calls of the interface are from kube-openapi itself. The GetOperationIDAndTagsFromRoute in the PR linked has not yet been integrated into k/k yet.

So to answer your question, yes this will be a breaking change for current users of the API, but at the moment it seems like we have no users of the particular API (that we know of). I'm okay with taking the safe approach of just creating another interface, just want to gather some thoughts first.

Thanks for your reply. I agree it isn't a huge breaking change and won't affect k/k, though will affect other projects using it (mine 😄), perhaps it's acceptable for this project to only support k/k.
Do you think it would be valuable to switch k/k to the interface, or rather wait until there's a push to get off go-restful?

Thanks for pointing that out, that's enough of a reason to create backwards compatibility and create a new interface. Do you have any preferences for the interface name?

I don't seem any harm in switching k/k to the interface, although that will be a decision up to the approvers.

Thanks very much for the backwards compat effort :) No real preference on name, perhaps DeprecatableRoute?

k8s-triage-robot · 2022-08-07T22:41:08Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

apelisse · 2022-08-09T21:34:33Z

@Jefftree what's the plan here?

k8s-ci-robot · 2022-08-09T23:04:53Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Jefftree
Once this PR has been reviewed and has the lgtm label, please assign apelisse for approval by writing /assign @apelisse in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Jefftree · 2022-08-10T17:40:04Z

@apelisse I think this provides valuable information to the OpenAPI and we should look to get it in once the pending comment is addressed.

k8s-triage-robot · 2022-09-09T18:35:10Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2022-10-09T19:17:13Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2022-10-09T19:17:18Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot requested review from apelisse, liggitt and sttts January 18, 2022 21:38

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 18, 2022

Jefftree force-pushed the openapi-deprecated branch from fb079e1 to 6f34764 Compare January 18, 2022 21:39

liggitt reviewed Jan 18, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

liggitt reviewed Jan 18, 2022

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2022

austince reviewed May 9, 2022

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 7, 2022

Jefftree added 2 commits August 9, 2022 23:04

Upgrade to restful v2.9.5 and support Deprecated field

37a01db

Rebase indent

0c38d6d

Jefftree force-pushed the openapi-deprecated branch from 6f34764 to 0c38d6d Compare August 9, 2022 23:04

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 9, 2022

Jefftree changed the title ~~Upgrade go-restful to v2.9.5 and support Deprecated field~~ support Deprecated field Aug 9, 2022

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 9, 2022

k8s-ci-robot closed this Oct 9, 2022

support Deprecated field #275

support Deprecated field #275

Uh oh!

Conversation

Jefftree commented Jan 18, 2022

Uh oh!

Uh oh!

liggitt Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apelisse Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

austince May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-triage-robot commented Aug 7, 2022

Uh oh!

apelisse commented Aug 9, 2022

Uh oh!

k8s-ci-robot commented Aug 9, 2022

Uh oh!

Jefftree commented Aug 10, 2022

Uh oh!

k8s-triage-robot commented Sep 9, 2022

Uh oh!

k8s-triage-robot commented Oct 9, 2022

Uh oh!

k8s-ci-robot commented Oct 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

liggitt Jan 18, 2022 •

edited

Loading

apelisse Jan 19, 2022 •

edited

Loading

austince May 9, 2022 •

edited

Loading