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

GetReference shouldn't hardcode api prefix #1490

Closed
dchen1107 opened this issue Sep 29, 2014 · 7 comments
Closed

GetReference shouldn't hardcode api prefix #1490

dchen1107 opened this issue Sep 29, 2014 · 7 comments
Labels
area/api Indicates an issue on api area. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@dchen1107
Copy link
Member

#1475 assumes there is a fixed path prefixes in the apiserver and client, so that it can retrieve version like this:

var versionFromSelfLink = regexp.MustCompile("/api/([^/]*)/")
...
version := versionFromSelfLink.FindStringSubmatch(jsonBase.SelfLink())

But above assumption is not valid today, we should do something to enforce such assumption, or find another way to retrieve version.

@smarterclayton

@smarterclayton
Copy link
Contributor

Being able to prefix the apiserver with a path is useful to some folks. The tradeoff being that general url generation gets much more difficult. In addition, in the real world URLs tend to drift (hosts, paths) so persisting "real urls" requires a fair amount of planning for the future to allow migrations.

The primary use case for prefixing the base path is to run two installations on the same server, or to fit into a root domain (mycompany.com/kubernetes/api) that has an existing proxy or load balancing infrastructure, or because the chosen prefix "api" is not preferred.

On Sep 29, 2014, at 1:45 PM, Dawn Chen notifications@github.com wrote:

PR 1475 assumes there is a fixed path prefixes in the apiserver and client, so that it can retrieve version like this:

var versionFromSelfLink = regexp.MustCompile("/api/([^/]*)/")
...
version := versionFromSelfLink.FindStringSubmatch(jsonBase.SelfLink())

But above assumption is not valid today, we should do something to enforce such assumption, or find another way to retrieve version.

@smarterclayton


Reply to this email directly or view it on GitHub.

smarterclayton referenced this issue in jhadvig/origin Oct 1, 2014
@bgrant0607 bgrant0607 added the area/api Indicates an issue on api area. label Oct 3, 2014
@bgrant0607 bgrant0607 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Dec 4, 2014
@bgrant0607 bgrant0607 self-assigned this Dec 4, 2014
@lavalamp
Copy link
Member

I think that regexp is compatible with prefixes, no? Does it need to change?

@smarterclayton
Copy link
Contributor

Our prefix is /osapi/, which is think triggered this

@lavalamp
Copy link
Member

Oh, well, that's unfortunate. Hrm.

@bgrant0607 bgrant0607 changed the title Validation SelfLink in JsonBase object GetReference shouldn't hardcode api prefix Dec 15, 2014
@bgrant0607
Copy link
Member

We definitely shouldn't hardcode the api prefix here.

Isn't there a better way than parsing selfLink?

Or maybe we should just represent object references using relative URLs?

@bgrant0607 bgrant0607 added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed 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
@davidopp davidopp added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. team/master labels Feb 17, 2015
@bgrant0607
Copy link
Member

I think this is prerequisite for #2098.

@bgrant0607 bgrant0607 removed their assignment Feb 28, 2015
@bgrant0607 bgrant0607 added sig/node Categorizes an issue or PR as relevant to SIG Node. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. status/help-wanted and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Feb 28, 2015
@nikhiljindal
Copy link
Contributor

I dont see https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/api/ref.go using that regex anymore. It is extracting the version from ObjectReference.
API prefix is not harcoded anywhere in that function or file.

stlaz pushed a commit to stlaz/kubernetes that referenced this issue Mar 1, 2023
OCPBUGS-7267: add SeccompProfile to Pod and Container accessors/mutators
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. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests

6 participants