-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
UPSTREAM: 42275: discovery restmapping should always prefer /v1 #13727
UPSTREAM: 42275: discovery restmapping should always prefer /v1 #13727
Conversation
@@ -45,6 +45,10 @@ func NewRESTMapper(groupResources []*APIGroupResources, versionInterfaces meta.V | |||
var resourcePriority []unversioned.GroupVersionResource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: these declaration can be dropped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wanted to follow the original commit as close as possible ;-)
@@ -45,6 +45,10 @@ func NewRESTMapper(groupResources []*APIGroupResources, versionInterfaces meta.V | |||
var resourcePriority []unversioned.GroupVersionResource | |||
var kindPriority []unversioned.GroupVersionKind | |||
|
|||
// /v1 is special. It should always come first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// new servers will report non-core apigroups for origin resources. Because this client does not know about
// them yet, we enforce that all origin resources resolve to /v1 kinds.
Looks good. Two cosmetic comments. |
23d91d4
to
256a705
Compare
@sttts comments addressed |
[test] |
Evaluated for origin test up to 256a705 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/697/) (Base Commit: efa3fe6) |
[merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/697/) (Base Commit: efa3fe6) (Image: devenv-rhel7_6137) |
Evaluated for origin merge up to 256a705 |
@liggitt @deads2k @sttts PTAL
I tested this manually and it returns the v1.Route object (so describer works as expected).
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1441089