Skip to content

Add "root-path" to swagger-ui#44981

Open
alexey-plotnikoff wants to merge 28 commits intoquarkusio:mainfrom
alexey-plotnikoff:swagger
Open

Add "root-path" to swagger-ui#44981
alexey-plotnikoff wants to merge 28 commits intoquarkusio:mainfrom
alexey-plotnikoff:swagger

Conversation

@alexey-plotnikoff
Copy link

If the quarkus application behind proxy with custom location path swagger UI doesn't work correctly. This PR fix it.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request!

I think we need to clarify the semantic here, also because I suppose this part is probably not the only part that would need tweaking.

Question: would it be possible to use relative paths instead and thus avoiding entirely this issue?

@alexey-plotnikoff
Copy link
Author

Question: would it be possible to use relative paths instead and thus avoiding entirely this issue?

Good point. I'll check it.

@alexey-plotnikoff
Copy link
Author

@gsmet current plugin generates all links with absolute path:

Screenshot 2024-12-09 at 19 45 41

Do you mean I should change all these links to relative paths?

@michalvavrik
Copy link
Member

\cc @MikeEdgar

@MikeEdgar
Copy link
Contributor

I agree that this seems like something that would be handled at a high level than just the Swagger UI. For example, the one link being configured is the backHref that resolves to /q/dev. @alexey-plotnikoff have you tried using the quarkus.http.non-application-root-path configuration for this?

@alexey-plotnikoff
Copy link
Author

Thank you @MikeEdgar!
Yes, it works! But I should reconfigure nginx from:

server {
    listen  80;
    server_name quarkus.home.arpa;

    location /nginx/path/ {
        proxy_pass http://host.docker.internal:8080/;
    }
}

to:

server {
    listen  80;
    server_name quarkus.home.arpa;

    location /nginx/path/ {
        proxy_pass http://host.docker.internal:8080/nginx/path/;
    }
}

@alexey-plotnikoff
Copy link
Author

alexey-plotnikoff commented Dec 16, 2024

@MikeEdgar you solution with quarkus.http.non-application-root-path will change liveness and readiness probes in Kubernetes environment.
So all microservices in Kubernetes will have their own probes instead same for all. For me it is not good solution.
I think will be better add a new parameter for swagger UI only as in this PR.
What do you think?

@MikeEdgar
Copy link
Contributor

I'm not sure if there are other components that should participate with a configuration like the one proposed. I'll let the others chime in on that.

@gsmet
Copy link
Member

gsmet commented Jan 16, 2025

@cescoffier I think this one should be discussed with you. I'm not entirely sure how we can solve this issue globally.

@alexey-plotnikoff
Copy link
Author

@cescoffier What do you think about this PR?

* If application behind the proxy with custom location path.
*/
@ConfigItem
Optional<String> locationPath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the root path for openapi?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2025-03-05 at 22 38 32

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I should rename locationPath to rootPath?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


String openApiPath = nonApplicationRootPathBuildItem.resolvePath(openapi.path);
if (swaggerUiConfig.locationPath.isPresent()) {
openApiPath = swaggerUiConfig.locationPath.get() + openApiPath;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have to normalized the resulting path, or do we expect it to be consistent?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean I should check a forward slash at the start of swaggerUiConfig.locationPath.get() if it doesn't exists?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, also that the given URL is normalized: https://en.wikipedia.org/wiki/URI_normalization

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@cescoffier
Copy link
Member

I don't believe we can use a global setting here. The global setting will affect the probes, metrics... Now in such kind of setup, the openapi page is considered public, while the probes (and metrics) are considered private.

The easiest is clearly to configure the extension.

We could imagine another option to declare the openapi routes to be part of the public API and mounted differently, but it will complexify the routing code.

Alexey Plotnikov and others added 3 commits March 5, 2025 22:10
# Conflicts:
#	extensions/swagger-ui/deployment/src/main/java/io/quarkus/swaggerui/deployment/SwaggerUiConfig.java
#	extensions/swagger-ui/deployment/src/main/java/io/quarkus/swaggerui/deployment/SwaggerUiProcessor.java
Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some comments.

@alexey-plotnikoff
Copy link
Author

alexey-plotnikoff commented Mar 10, 2025

@cescoffier I made some fixes. Take a look pls.

@alexey-plotnikoff alexey-plotnikoff changed the title Add "location-path" to swagger-ui Add "root-path" to swagger-ui Mar 10, 2025
@cescoffier
Copy link
Member

@alexey-plotnikoff can you rebase?
@gsmet, can you re-check this PR once you're back? Probably @phillip-kruger too.

@cescoffier
Copy link
Member

@alexey-plotnikoff Any chance for you to rebase the PR?

@alexey-plotnikoff
Copy link
Author

@alexey-plotnikoff Any chance for you to rebase the PR?

So much changes!
I'll try.

Alexey Plotnikov added 3 commits February 16, 2026 22:48
# Conflicts:
#	extensions/swagger-ui/deployment/src/main/java/io/quarkus/swaggerui/deployment/SwaggerUiProcessor.java
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@alexey-plotnikoff
Copy link
Author

@alexey-plotnikoff Any chance for you to rebase the PR?

@cescoffier could someone approve my changes to start GitHub Actions?

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@alexey-plotnikoff
Copy link
Author

@cescoffier Merging is blocked by @gsmet from Mar 11, 2025. Looks like he doesn't like my PR 😢

@cescoffier
Copy link
Member

It looks like there are merge commits.

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 15, 2026

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 5ef855b.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 15, 2026

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 5ef855b.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants