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

Deprecation Notice breaks (atleast zsh) completion #3473

Closed
BronzeDeer opened this issue Jun 21, 2023 · 3 comments · Fixed by #3474
Closed

Deprecation Notice breaks (atleast zsh) completion #3473

BronzeDeer opened this issue Jun 21, 2023 · 3 comments · Fixed by #3474
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@BronzeDeer
Copy link
Contributor

BronzeDeer commented Jun 21, 2023

What broke? What's expected?

When generating the completion script or accessing auto complete functionality while inside a directory with a deprecated project the deprecation notice is generated on standard out and therefore interfers with the normal evaluation. Since the deprecation notice is not valid script syntax the completion script cannot be evaluated and even if loaded correctly, the deprecation notice will intermingle with the correct suggestions in deprecated paths

source <(kubebuilder completion zsh)
# resulting error: /proc/self/fd/15:1: bad pattern: ^[[1

kubebuilder <tab>
# completion                                                                 
# create                                                                         
# edit                                                                           
# help                                                                           
# init                                                                           
# version                                                                        
# ^[[1;36m[Deprecation Notice] This version is deprecated.The `go/v3` cannot scaffold projects using kustomize versions v4x+ and cannot fully support Kubernetes 1.25+.It is recommended to upgrade your project to the latest versions available (go/v4).Please, check the migration guide to learn how to upgrade your project

The error occurs if the completion script is sourced at shell-init and the shell is launched in a deprecated project (for example when opening a terminal from an IDE while working on such a project. A mitigation (cd / &&) can be used to always make the completion initialize cleanly (assuming the user didn't check out a deprecated project into root), this wholly unexpected and not a pattern that users ever need to follow with similar projects, where source <($BIN completion $SHELL) works cleanly. Furthermore there seems to be no mitigation for the completion results, requiring a fix in the cli itself

Reproducing this issue

#Clone a project that will trigger a deprecation warning:
git clone https://github.com/epam/edp-keycloak-operator/tree/1a935cec5558ee7bccaf8b4bc475ca364576e183
# CD into the project so that kubebuilder recognizes a deprecated project
cd edp-keycloak-operator
# Try to generate and source the completion scripts
source <(kubebuilder completion zsh)
# resulting error: /proc/self/fd/15:1: bad pattern: ^[[1

# Proof that the deprecation notice is generated in the subshell
kubebuilder completion zsh | head -n 5 | less -R

# Proof that this does not happen outside of deprecated projects
source <(cd / && kubebuilder completion zsh)

# If loaded correctly, attempt completion inside deprecated project
kubebuilder <tab>
# completion                                                                 
# create                                                                         
# edit                                                                           
# help                                                                           
# init                                                                           
# version                                                                        
# ^[[1;36m[Deprecation Notice] This version is deprecated.The `go/v3` cannot scaffold projects using kustomize versions v4x+ and cannot fully support Kubernetes 1.25+.It is recommended to upgrade your project to the latest versions available (go/v4).Please, check the migration guide to learn how to upgrade your project

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"3.10.0", KubernetesVendor:"1.26.1", GitCommit:"0fa57405d4a892efceec3c5a902f634277e30732", BuildDate:"2023-04-15T08:10:35Z", GoOs:"linux", GoArch:"amd64"}

PROJECT version

3

Plugin versions

layout:
- go.kubebuilder.io/v3

Other versions

zsh --version
zsh 5.9 (x86_64-pc-linux-gnu)

Extra Labels

No response

@BronzeDeer BronzeDeer added the kind/bug Categorizes issue or PR as related to a bug. label Jun 21, 2023
@BronzeDeer
Copy link
Contributor Author

BronzeDeer commented Jun 21, 2023

From my (admittedly very cursory) understanding of both kubebuilder's and cobra's source this stems from the fact that the deprecation notice is always printed, regardless of subcommand

c.printDeprecationWarnings()

But instead we should not print it if the subcommand is either completion (for obvious reasons) .

__complete
https://github.com/spf13/cobra/blob/dcb405a9399ed307eaa0e50f5cb1c290c8979dd4/completions.go#L29

, or __completeNoDesc

https://github.com/spf13/cobra/blob/dcb405a9399ed307eaa0e50f5cb1c290c8979dd4/completions.go#L32

I'll see if I can whip up a PR for this and include the latter two as constants from the dependency rather than magic strings

@camilamacedo86
Copy link
Member

Please, feel free to push a PR to sorted it out.
Your help is welcome.

BronzeDeer added a commit to BronzeDeer/kubebuilder that referenced this issue Jun 21, 2023
Fixes kubernetes-sigs#3473

Currently, when initializing in a project with deprecated plugins
the kubebuilder cli will immediately output a deprecation notice to
stdout before even parsing the command. While this is acceptable and
even desirable for interactive usage of the cli, it will "randomly"
(i.e. dependent on $PWD and not on the command) break automated usage of
the kubebuilder output. This, again is fine if kubebuilder makes no
guarantees of stable machine readable output, but this does not hold for
the completion sub commands which are only ever parsed by machines: The
output of "kubebuilder completion <shell>" must always only output the
exact script generated by cobra and the cobra internal commands
__complete and __completeNoDesc must only only return an exact list of
completion options.

In deprecated projects this will break shell completion if the shell is
started in the directory and if the completion script is correctly
initialized, all completion options will be polluted by the massive
deprecation notice.

This commit instead changes the deprecation notice to output to stderr
(while maintaining the return code of the actual command). This fixes
the completion usecase which only reads in stdout, and should not break
existing integrations, unless there exist scripts which use the length
of stderr to check for errors rather than the return code of the call.
BronzeDeer added a commit to BronzeDeer/kubebuilder that referenced this issue Jun 21, 2023
Fixes kubernetes-sigs#3473

Currently, when initializing in a project with deprecated plugins
the kubebuilder cli will immediately output a deprecation notice to
stdout before even parsing the command. While this is acceptable and
even desirable for interactive usage of the cli, it will "randomly"
(i.e. dependent on $PWD and not on the command) break automated usage of
the kubebuilder output. This, again is fine if kubebuilder makes no
guarantees of stable machine readable output, but this does not hold for
the completion sub commands which are only ever parsed by machines: The
output of "kubebuilder completion <shell>" must always only output the
exact script generated by cobra and the cobra internal commands
__complete and __completeNoDesc must only only return an exact list of
completion options.

In deprecated projects this will break shell completion if the shell is
started in the directory and if the completion script is correctly
initialized, all completion options will be polluted by the massive
deprecation notice.

This commit instead changes the deprecation notice to output to stderr
(while maintaining the return code of the actual command). This fixes
the completion usecase which only reads in stdout, and should not break
existing integrations, unless there exist scripts which use the length
of stderr to check for errors rather than the return code of the call.

Signed-off-by: Joel Pepper <pepper@bronze-deer.de>
@atoato88
Copy link
Contributor

FYI
I confirmed that this issue is also occured in bash.
The error message is different from the one in zsh.

$ source <(kubebuilder completion bash)
-bash: /dev/fd/63: line 1: syntax error near unexpected token `('
-bash: /dev/fd/63: line 1: `[Deprecation Notice] This version is deprecated.The `go/v3` cannot scaffold projects using kustomize versions v4x+ and cannot fully support Kubernetes 1.25+.It is recommended to upgrade your project to the latest versions available (go/v4).Please, check the migration guide to learn how to upgrade your project'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants