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

Update rewriteBasePath documentation #114562

Closed
TinaHeiligers opened this issue Oct 11, 2021 · 7 comments · Fixed by #195465
Closed

Update rewriteBasePath documentation #114562

TinaHeiligers opened this issue Oct 11, 2021 · 7 comments · Fixed by #195465
Assignees
Labels
Feature:Configuration Settings in kibana.yml impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc technical debt Improvement of the software architecture and operational architecture

Comments

@TinaHeiligers
Copy link
Contributor

TinaHeiligers commented Oct 11, 2021

Update 18 July 2024:

After discussing this we think the low maintenance cost means that it's not worth potentially impacting any customers so we will just keep this as is. We should however review our documentation to ensure that it's up to date/accurate.

rewriteBasePath was introduced and a deprecation warning was immediately introduced at the same time.
The original PR describes why this was needed quite well but the relevant parts are:
“ server.basePath expects the proxy sending the request to rewrite the request and strip the basePath” (this was tripping folks up).
We added server.rewriteBasePath to tell Kibana if it should remove the basePath from requests it receives. The setting was going to be enabled by default in 7.0 (which it wasn't) and the deprecation was supposed to be removed in the 7 series (which it also wasn't).

.kibana.yml still has server.rewriteBasePath: false with the same info added in the original PR and
the deprecation message itself hasn’t changed. It's unclear if the setting change was an oversight or if there's a specific reason to not have followed through on the plans.

We should decide on what we want to do here: flip the default to true and remove the deprecation or keep it as it is.

While there isn't much overhead in keeping this deprecation around in the immediate future (in #103915 we changed the deprecation level to warning) , we should come up with a plan on how we could approach it.

Changing the default could also have serious consequences and we need to check:

  • what changes are required in the dev server code (packages/kbn-cli-dev-mode/src/base_path_proxy_server.ts) cc @elastic/kibana-operations
  • if we need to change the default config on cloud somehow (are they using server.basePath with the 'legacy' behavior?)
  • usage data for a change in rewriteBasePath from the default
@botelastic botelastic bot added the needs-team Issues missing a team label label Oct 11, 2021
@TinaHeiligers TinaHeiligers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Oct 11, 2021
@TinaHeiligers TinaHeiligers added technical debt Improvement of the software architecture and operational architecture loe:needs-research This issue requires some research before it can be worked on or estimated impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. Feature:Configuration Settings in kibana.yml labels Oct 11, 2021
@spalger
Copy link
Contributor

spalger commented Oct 11, 2021

I don't love removing settings like this, forcing users to change proxy configs is likely to cause some headaches and issues. It also feels like the only "tech debt" here is a config option that skips code we're going to already planning to maintain (the rewrite code). Just kind of wish we left this around for as long as we can stand, or until usage data indicates it's really unnecessary.

@TinaHeiligers
Copy link
Contributor Author

TinaHeiligers commented Oct 12, 2021

Just kind of wish we left this around for as long as we can stand, or until usage data indicates it's really unnecessary.

I hear you, for 8.0, we're keeping the deprecation but changing the level from critical to warning.

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort and removed loe:needs-research This issue requires some research before it can be worked on or estimated impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. labels Nov 4, 2021
@jonasjancarik
Copy link

Just to be clear, the docs currently say:

In Kibana 8.0 and later, the default is true. Default: deprecated

In fact, though, the default is false? If that's the case (and it seems to be), wouldn't it be worth it updating the docs?

@jonasjancarik
Copy link

Additionally, this is what the docs say about server.publicBaseUrl:

The publicly available URL that end-users access Kibana at. Must include the protocol, hostname, port (if different than the defaults for http and https, 80 and 443 respectively), and the server.basePath (if configured). This setting cannot end in a slash (/).

The if configured part seems wrong - when I don't set server.basePath, it seems to throw an error:

FATAL  Error: [config validation of [server]]: [publicBaseUrl] must contain the [basePath]: /test !== undefined

this is what I have in my kibana.yml (I am using env vars passed through docker-compose.yml) in this case:

# server.basePath: ${KIBANA_BASE_PATH}
# server.rewriteBasePath: true
server.publicBaseUrl: ${KIBANA_PUBLIC_BASE_URL}

Maybe this is an issue only with a Docker setup, though.

@rudolf
Copy link
Contributor

rudolf commented Jul 18, 2024

After discussing this we think the low maintenance cost means that it's not worth potentially impacting any customers so we will just keep this as is. We should however review our documentation to ensure that it's up to date/accurate.

@rudolf rudolf changed the title Handle rewriteBasePath core deprecation Update rewriteBasePath documentation Jul 18, 2024
@TinaHeiligers
Copy link
Contributor Author

TinaHeiligers commented Sep 19, 2024

From the docs:

server.rewriteBasePath
Specifies whether Kibana should rewrite requests that are prefixed with server.basePath or require that they are rewritten by your reverse proxy. In Kibana 6.3 and earlier, the default is false. In Kibana 7.x, the setting is deprecated. In Kibana 8.0 and later, the default is true. Default: deprecated

@rudolf deprecated as the default doesn't make sense to me. Do you recall who made that change and what it's supposed to indicate?

@TinaHeiligers TinaHeiligers self-assigned this Oct 8, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Configuration Settings in kibana.yml impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants