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.

Sign up for GitHub

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

Jump to bottom

Warn users about CRD alias naming conflicts #108573

Closed
ciberkleid opened this issue Mar 7, 2022 · 24 comments · Fixed by #117668
Closed

Warn users about CRD alias naming conflicts #108573

ciberkleid opened this issue Mar 7, 2022 · 24 comments · Fixed by #117668
Assignees
@NikhilSharmaWe
Labels
area/usability kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/cli Categorizes an issue or PR as relevant to SIG CLI.

Comments

@ciberkleid
Copy link

Issue #94860 was closed as stale so opening a new one.

Another case in which this happens is with kubectl get img if both Knative Serving and kpack are installed.

Calling kubectl api-resources shows that a kpack Image resource should have four aliases:

$ k api-resources | grep kpack | grep -i image
images                            cnbimage,cnbimages,img,imgs   kpack.io/v1alpha2                           true         Image

Three of these aliases work:

$ k get cnbimage
NAME     LATESTIMAGE                                                                                                      READY
manual   gcr.io/fe-ciberkleid/demo/app@sha256:c3a9f2e8f8eced4f6779aa99419642c164665768872960c9c7737bd1146cd044   True

$ k get cnbimages
NAME     LATESTIMAGE                                                                                                      READY
manual   gcr.io/fe-ciberkleid/demo/app@sha256:c3a9f2e8f8eced4f6779aa99419642c164665768872960c9c7737bd1146cd044   True

$ k get imgs
NAME     LATESTIMAGE                                                                                                      READY
manual   gcr.io/fe-ciberkleid/demo/app@sha256:c3a9f2e8f8eced4f6779aa99419642c164665768872960c9c7737bd1146cd044   True

However, one of the aliases (img) does not work:

$ k get img
No resources found in default namespace.

I think it is important to inform the user that the img resource may refer to more than one type. The above results are misleading and can cause users to draw incorrect conclusions about what is or isn't running.

Aside: I had originally logged this under kpack but was advised that it is a kubectl issues, hence bringing it up here. For reference, the kpack issue is buildpacks-community/kpack#929.

Originally posted by @ciberkleid in #94860 (comment)
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 7, 2022
@k8s-ci-robot
Copy link
Contributor

@ciberkleid: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@ciberkleid
Copy link
Author

/sig cli

@k8s-ci-robot k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 7, 2022
@pacoxu
Copy link
Member

pacoxu commented Mar 8, 2022

/kind bug
adding a warning for that is valid to me

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 8, 2022
@kkkkun
Copy link
Member

kkkkun commented Mar 8, 2022 •
edited

Could you please paste k api-resources | grep -i img ?

@ciberkleid
Copy link
Author

Results of k api-resources | grep -i img:

images                            img                           caching.internal.knative.dev/v1alpha1       true         Image
images                            cnbimage,cnbimages,img,imgs   kpack.io/v1alpha2                           true         Image

@liggitt liggitt added kind/feature Categorizes issue or PR as related to a new feature. area/usability and removed kind/bug Categorizes issue or PR as related to a bug. labels Mar 9, 2022
@NikhilSharmaWe
Copy link
Member

NikhilSharmaWe commented Mar 11, 2022 •
edited

@ciberkleid Could you please explain me this statement of yours -

  • I think it is important to inform the user that the img resource may refer to more than one type.

Especially more than one type part. Do you mean there can be another resource but of different type with name img, and there are no restrictions on aliases to make them unique from the name of other resources ?

@ciberkleid
Copy link
Author

In this example, the types would be images.kpack.io and images.caching.internal.knative.dev.

I am not sure how restricting aliases would work (would it be a bug in kpack or knative? would it be an operator decision on a per-cluster basis? both have their drawbacks...). However it is implemented, at the very minimum, if nothing else changes, the current result of k get images or k get img on a cluster that has both kpack and knative should inform the user that they need to use a different alias or use the fully qualified resource name (one of the "types" above).

@NikhilSharmaWe
Copy link
Member

@ciberkleid @pacoxu So we have to add a warning in the logs when we face this situation like - use different alias or use the fully qualified resource name.

But where the code is written for this log.

When I did git grep -n 'No resources found in default namespace.' in the repo, nothing shows up.

@NikhilSharmaWe
Copy link
Member

NikhilSharmaWe commented Mar 12, 2022 •
edited

@pacoxu @ciberkleid I think we have to update the statement which is printed here
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go#L598-L600. Could you please confirm. If yes, we can add something like the statement mentioned in my previous comment.

@NikhilSharmaWe
Copy link
Member

/assign

@NikhilSharmaWe NikhilSharmaWe mentioned this issue Mar 13, 2022
Closed
@ciberkleid
Copy link
Author

Worth adding that this is relevant to multiple verbs (get, delete, describe, create, patch, edit, set, diff... others?), and that a warning message is probably sufficient as any other change would break backwards compatibility.

cc @stealthybox

@NikhilSharmaWe Thanks for giving it a shot! I think more conditional logic is required, as well as a separate warning message, but I don't personally have the expertise to know how to solve this, so I will defer to the maintainers.

@soltysh
Copy link
Contributor

soltysh commented Mar 31, 2022

This is not a bug, this is working as designed. I do understand that this is causing a lot of confusion, but in your case the problem is not with kubectl which is reading information from API discovery and picking one resource over the other, but rather it's coming from the fact that you're using an alias to a resource which any resource can define freely when registering CRD. In those cases where you have multiple resources with identical aliases, or even identical names I ALWAYS advise users to use fully qualified name to ensure they are retrieving requested resource.

@BenTheElder BenTheElder mentioned this issue May 26, 2022
Closed
@k8s-triage-robot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 29, 2022
@k8s-triage-robot
Copy link

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-ci-robot 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 Jul 29, 2022
@k8s-triage-robot
Copy link

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:

  • Reopen this issue or PR with /reopen
  • Mark this issue or 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
Copy link
Contributor

@k8s-triage-robot: Closing this issue.

In response to this:

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:

  • Reopen this issue or PR with /reopen
  • Mark this issue or 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.

@ardaguclu ardaguclu mentioned this issue Oct 25, 2022
Closed
@ardaguclu
Copy link
Member

/reopen

@k8s-ci-robot
Copy link
Contributor

@ardaguclu: Reopened this issue.

In response to this:

/reopen

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 k8s-ci-robot reopened this Apr 28, 2023
@ardaguclu ardaguclu mentioned this issue Apr 28, 2023
Merged
@k8s-triage-robot
Copy link

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

This bot triages issues 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:

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

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

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

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

This bot triages issues 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:

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

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

/close not-planned

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 k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2023
@ardaguclu
Copy link
Member

/reopen

@k8s-ci-robot
Copy link
Contributor

@ardaguclu: Reopened this issue.

In response to this:

/reopen

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 k8s-ci-robot reopened this May 28, 2023
@ardaguclu
Copy link
Member

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 7, 2023
@jhostetler
Copy link

Chiming in to second the described behavior being a problem. An especially troublesome case is with the delete verb. For example:

  1. I define a custom resource things.mydomain
  2. I get used to typing kubectl delete --all things
  3. I install some operator, not realizing it defines its own things.otherdomain
  4. The next time I type kubectl delete --all things, the name things now refers to things.otherdomain and I break my installation in an obscure way.

That may be the intended behavior, but I was also surprised that the client was willing to delete something when the name was ambiguous.

This was referenced Oct 19, 2023
Closed
Open
@siggy siggy mentioned this issue Dec 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@NikhilSharmaWe NikhilSharmaWe

Labels
area/usability kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/cli Categorizes an issue or PR as relevant to SIG CLI.
Projects
SIG CLI
Archived in project
Milestone
No milestone
10 participants
@soltysh @jhostetler @liggitt @pacoxu @ciberkleid @ardaguclu @kkkkun @k8s-ci-robot @k8s-triage-robot @NikhilSharmaWe