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

tls command not found despite the extension present in the application #42751

Closed
cescoffier opened this issue Aug 26, 2024 · 14 comments · Fixed by #43025
Closed

tls command not found despite the extension present in the application #42751

cescoffier opened this issue Aug 26, 2024 · 14 comments · Fixed by #43025
Assignees
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) kind/bug Something isn't working
Milestone

Comments

@cescoffier
Copy link
Member

Describe the bug

The tls commands are not found despite the tls registry extension being present in the application (transitive from quarkus-rest -> quarkus-vertx-http -> quarkus-tls-registry).

My very ugly workaround consists of adding the following JSON object to the catalog:

 "tls": {
        "name" : "tls",
        "type" : "maven",
        "location" : "io.quarkus:quarkus-tls-registry-cli:999-SNAPSHOT",
        "description" : "Generate a TLS certificate and key for use with Quarkus applications",
        "inProjectCatalog" : false
      }

Even with this hack, when launching quarkus tls, it complains it cannot find the command (but executes them correctly...)

Expected behavior

quarkus tls commands should be available as soon as the extension is in the classpath

Actual behavior

The commands are not found.

How to Reproduce?

  1. Generate a project using quarkus-rest from https://code.quarkus.io
  2. Go to the project root and issue quarkus tls --help

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@cescoffier cescoffier added the kind/bug Something isn't working label Aug 26, 2024
@iocanel
Copy link
Contributor

iocanel commented Aug 26, 2024

At a glance:

The metadata of the extension are correctly defined.

On a fresh project I added tls-registry, helm and authzed, all three are registering cli plugins. The output of quarkus plug list is

❯ quarkus plug list
    Name        	Type  	Scope   	Location                                         	Description 	
 *  authzed     	maven 	project 	io.quarkiverse.authzed:quarkus-authzed-cli:0.5.0 	            	
 *  helm        	maven 	project 	io.quarkiverse.helm:quarkus-helm-cli:1.2.3       	            	
 *   io.quarkus 	maven 	project 	tls: io.quarkus:quarkus-tls-registry-cli:3.13.3  	        

So, it must be an issue with the plugin alias. I'll fix it asap.

@iocanel iocanel self-assigned this Aug 26, 2024
@maxandersen
Copy link
Member

@iocanel the ask/issue here is that the tls extension is transitively included. At least thats what @cescoffier reports thus it is expected to be available.

I don't think we look in the full tree of dependencies (and should be careful about that too).

@maxandersen
Copy link
Member

i.e. quarkus create app creates app that has tls in transitive dependency:

mvn dependency:tree | grep tls
Aug 26, 2024 8:50:57 AM org.jline.utils.Log logr
WARNING: Unable to create a system terminal, creating a dumb terminal (enable debug logging for more information)
[main] WARN org.jline - Unable to create a system terminal, creating a dumb terminal (enable debug logging for more information)
[INFO] |  |  +- io.quarkus:quarkus-tls-registry:jar:3.13.3:compile

but quarkus plug list does not list it nor does quarkus tls find it as an option to enable.

@maxandersen
Copy link
Member

quarkus ext add tls-registry makes it work.

so yeah, transitive dependency "blindness".

@iocanel
Copy link
Contributor

iocanel commented Aug 28, 2024

The ExtensionManager does not seem to provide a way to get the extensions that are installed transitively, as it only uses the dependencies added directly to the pom. I think that this is due to how the MavenProjectBuildFile is dealing with dependencies.

I imagine that we either never needed transitives before, it was tricky to provide a uniform behavior across build tools, or we wanted to avoid the overhead of resolving transitives etc. @aloubyansky do you see another way of getting the transitive extensions?

@aloubyansky
Copy link
Member

There are a couple of ways to get the info about the transitive ones.
A list of them is included in the extension metadata. It's recorded at extension build time, which likely will not change in an app but there is a probability it will. It also will not capture enabled conditional dependencies.
The other way would be to resolve the ApplicationModel that'll capture all of them but will take a bit more time.

@cescoffier cescoffier added the area/cli Related to quarkus cli (not maven/gradle/etc.) label Aug 30, 2024
@amusarra
Copy link
Contributor

The ExtensionManager does not seem to provide a way to get the extensions that are installed transitively, as it only uses the dependencies added directly to the pom. I think that this is due to how the MavenProjectBuildFile is dealing with dependencies.

I imagine that we either never needed transitives before, it was tricky to provide a uniform behavior across build tools, or we wanted to avoid the overhead of resolving transitives etc. @aloubyansky do you see another way of getting the transitive extensions?

Hi @iocanel , unfortunately the documentation 10. Quarkus CLI commands and development CA (Certificate Authority) says to use quarkus tls directly without expressing the requirement to install the extension first via command quarkus ext add tls-registry

@maxandersen
Copy link
Member

@amusarra the docs is though wrong. It was written where someone had expliciltiy added the tls command thus it behaves differently. So we need to figure out how we like this to work but also what is feasible.

@iocanel @aloubyansky for other plugins (i.e. in jbang catalog or in the environment) we generally require explicit registration/install. we do that for security reasons (don't just run an random quarkus-xxx command in path) but also to avoid performance overhead. and always resolving the full transitive set of a quarkus app to be able to list quarkus help with transitive plugins I'm pretty sure will have a negative impact.

Would it be sufficient that quarkus tls when run will first then search the transitive set info and then we explicit install it?

@cescoffier
Copy link
Member Author

While the documentation is wrong, it's the experience we want. Having to explicitly add an extension to use commands is counterintuitive. In this case, why do we have transitive extensions at all?

So, +1 to search the transitive set and install it automatically.

@maxandersen
Copy link
Member

maxandersen commented Sep 3, 2024

@cescoffier i get your ask but if that means quarkus cli startup multiple seconds slower I would be -1.

remember quarkus tls will work; its just the guide/help that wont show it before you run it the first time.

@cescoffier
Copy link
Member Author

Yes, that's what I mean - it must work. How it's implemented under the scene does not matter (as soon as it does not extend the startup time which I hope we will reduce drastically at some point)

@iocanel
Copy link
Contributor

iocanel commented Sep 4, 2024

Created a pull request. I followed @aloubyansky advice and got the transitives extension from the metadata. This makes things pretty simple and without any significant overhead.

@cescoffier
Copy link
Member Author

Awesome @iocanel - can you link the PR?

@aloubyansky
Copy link
Member

#43025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) kind/bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

9 participants
@maxandersen @aloubyansky @iocanel @cescoffier @amusarra @gsmet @geoand and others