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

.get_endpoints.kind == "Endpoint" vs .get_endpoint(...).kind == "Endpoints" #307

Closed
cben opened this issue Feb 18, 2018 · 2 comments · Fixed by #366 or #370
Closed

.get_endpoints.kind == "Endpoint" vs .get_endpoint(...).kind == "Endpoints" #307

cben opened this issue Feb 18, 2018 · 2 comments · Fixed by #366 or #370
Assignees
Labels

Comments

@cben
Copy link
Collaborator

cben commented Feb 18, 2018

When getting a single entity, we return .kind as-is:

pry(main)> kclient.get_endpoint('docker-registry', 'default', as: :raw)[0..39]
=> "{\"kind\":\"Endpoints\",\"apiVersion\":\"v1\",\"m"
pry(main)> kclient.get_endpoint('docker-registry', 'default').kind
=> "Endpoints"
pry(main)> kclient.get_security_context_constraint('nonroot').kind
=> "SecurityContextConstraints"

For collections, k8s does not return individual .kind in each item, only a global list kind.
We do some magic to convert that to a singular .kind, without the List.
Somewhat confusingly, we do this on the EntityList pseudo-array, no .kind on individual item:

pry(main)> kclient.get_endpoints(as: :raw)[0..39]
=> "{\"kind\":\"EndpointsList\",\"apiVersion\":\"v1"
pry(main)> kclient.get_endpoints.kind
=> "Endpoint"
pry(main)> kclient.get_endpoints.first.kind
=> nil
pry(main)> kclient.get_security_context_constraints.kind
=> "SecurityContextConstraint"

The immediate bug here is "Endpoints" vs "Endpoint" and "SecurityContextConstraints" vs "SecurityContextConstraint" mismatch, because the single is already plural in k8s (see kubernetes/kubernetes#8115 (comment), kubernetes/kubernetes#18622, #81, #261 for background).
I'm not 100% sure which way to fix it, but I think the reason we "fixed" the double plural is merely we needed distinct get_foo / get_foos method names — which is a kubeclient concern that should not leak into .kind, it should always be a valid k8s API kind.


The wider question I want to discuss (not necessarily here) is what's the purpose of this magic?

  • The best use I envision is making it easy to feed the kind back to the API, which is not yet applicable much in kubeclient but is discussed on Allow different resources to use different apiVersion #208 etc.
    Towards that, I'd like us to provide the non-List .kind (and apiVersion?) inside every entity of the array, such that you don't have to care whether you obtained one from via List or Get operation. (And then potentially restore get_foos.kind to be "FooList".)
@cben
Copy link
Collaborator Author

cben commented Nov 21, 2018

Besides kind, neither the collection nor the individual items expose .api_version. It's simply lost.

cben added a commit to cben/kubeclient that referenced this issue Nov 22, 2018
cben added a commit to cben/kubeclient that referenced this issue Nov 22, 2018
Also fixes ManageIQ#307 - get_endpoints.kind now plural as in kubernetes.
cben added a commit to cben/kubeclient that referenced this issue Nov 23, 2018
cben added a commit to cben/kubeclient that referenced this issue Nov 23, 2018
Also fixes ManageIQ#307 - get_endpoints.kind now plural as in kubernetes.
@cben cben self-assigned this Nov 23, 2018
@cben
Copy link
Collaborator Author

cben commented Nov 23, 2018

Split off the wider interface questions to #368.
This issue is only about the bugs get_endpoints.kind == "Endpoint" and get_security_context_constraints.kind == "SecurityContextConstraint".
Fixing in #366.

cben added a commit to cben/kubeclient that referenced this issue Nov 23, 2018
ManageIQ#367

Also fixes ManageIQ#307 - get_security_context_constraints.kind,
  get_endpoints.kind are now plural as in kubernetes.
Also fixes ManageIQ#367 - create_security_context_constraint now works.
cben added a commit to cben/kubeclient that referenced this issue Nov 23, 2018
cben added a commit to cben/kubeclient that referenced this issue Nov 26, 2018
cben added a commit to cben/kubeclient that referenced this issue Nov 26, 2018
ManageIQ#367

Also fixes ManageIQ#307 - get_security_context_constraints.kind,
  get_endpoints.kind are now plural as in kubernetes.
Also fixes ManageIQ#367 - create_security_context_constraint now works.
cben added a commit to cben/kubeclient that referenced this issue Nov 26, 2018
@cben cben closed this as completed in #366 Nov 26, 2018
@cben cben mentioned this issue Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant