Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 #204384

jesuswr · 2024-12-16T12:52:39Z

Summary

resolves #194067

Set HTTP2 as default if ssl is enabled.

resolves #194065

Add deprecation log if ssl is not enabled or if protocol is set to http1

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

See some risk examples
...

elasticmachine · 2024-12-16T12:56:19Z

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

…-fix'

jesuswr · 2024-12-17T10:01:23Z

src/core/server/integration_tests/config/config_deprecation.test.ts

@@ -26,14 +26,15 @@ describe('configuration deprecations', () => {
  });

  if (getFips() === 0) {
-    it('should not log deprecation warnings for default configuration', async () => {
+    it('should log one warning for default configuration, the http/tls deprecation warning', async () => {


Is it ok to have deprecation warnings on default config? Seems ok since the issue is to log when http2 or tls are not enabled but tls can't be enabled by default

Generally we wouldn't want to log a deprecation right "out of the box" after installing Kibana. But given that TLS is such an important security configuration I think this is fine

jesuswr · 2024-12-17T10:02:47Z

packages/core/http/core-http-server-internal/src/http_config.ts

+        addDeprecation({
+          level: 'warning',
+          title: `Consider enabling TLS and using HTTP/2 to improve security and performance.`,
+          configPath: `${fromPath}.protocol,${fromPath}.ssl.enabled`,


Added one deprecation for both config values since they are related

…-fix'

rudolf · 2024-12-17T14:37:36Z

src/core/server/integration_tests/config/config_deprecation.test.ts

@@ -26,14 +26,15 @@ describe('configuration deprecations', () => {
  });

  if (getFips() === 0) {
-    it('should not log deprecation warnings for default configuration', async () => {
+    it('should log one warning for default configuration, the http/tls deprecation warning', async () => {


Generally we wouldn't want to log a deprecation right "out of the box" after installing Kibana. But given that TLS is such an important security configuration I think this is fine

docs/setup/settings.asciidoc

packages/core/http/core-http-server-internal/src/http_config.ts

…-fix'

rudolf · 2024-12-18T10:16:47Z

@lcawl Can you help us feature this in the 9 release notes (or breaking changes?). It's not a breaking change per se, but there is some risk that users with more exotic deployment configurations where Kibana is running behind a proxy could see problems. So out of caution we need to just make them aware of this so that they can test their deployment setup.

Something like:

The Kibana server will now default to using the HTTP2 protocol (when TLS is enabled) for improved browser loading times. If your Kibana server is hosted behind a load balancer or reverse proxy we recommend testing your deployment configuration before upgrading to 9.0.

rudolf

Some suggestions to further nudge users away from setting http1

docs/setup/settings.asciidoc

lcawl · 2024-12-18T16:07:30Z

@lcawl Can you help us feature this in the 9 release notes (or breaking changes?). It's not a breaking change per se, but there is some risk that users with more exotic deployment configurations where Kibana is running behind a proxy could see problems. So out of caution we need to just make them aware of this so that they can test their deployment setup.

If it's not a breaking change, it could perhaps be included in the v9 release notes as a "known issue" like https://www.elastic.co/guide/en/kibana/current/release-notes-8.0.0.html#known-issue-8.0.0 or https://www.elastic.co/guide/en/kibana/7.17/release-notes-7.17.0.html#known-issue-7.17.0

Co-authored-by: Rudolf Meijering <[email protected]>

jesuswr · 2024-12-19T10:05:24Z

@lcawl how can I include this as a "known issue"?

jloleysens · 2024-12-19T10:34:47Z

@lcawl how can I include this as a "known issue"?

@jesuswr I believe you can add an entry to docs/upgrade-notes.asciidoc, there is a template and sections in there. Might be missing something else @lcawl has in mind though.

jesuswr · 2024-12-19T12:07:25Z

Thanks! @jloleysens

docs/upgrade-notes.asciidoc

elasticmachine · 2024-12-24T10:24:05Z

💚 Build Succeeded

Buildkite Build
Commit: 434c223

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/doc-links`	80	81	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`aiAssistantManagementSelection`	93.2KB	93.2KB	+54.0B
`lists`	145.6KB	145.6KB	+54.0B
total			+108.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	447.7KB	447.7KB	+54.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/doc-links`	80	81	+1

History

💛 Build #262280 was flaky 8326897
💔 Build #260842 failed b4179e0
💔 Build #260811 failed 55423f5

cc @jesuswr

lcawl

Added a suggestion and comment about potentially dropping the extra "Known issue" section, but otherwise docs LTGM

docs/upgrade-notes.asciidoc

… is not enabled or protocol is set to HTTP1 (elastic#204384) ## Summary resolves elastic#194067 Set HTTP2 as default if ssl is enabled. resolves elastic#194065 Add deprecation log if ssl is not enabled or if protocol is set to http1 <img width="1665" alt="Screenshot 2024-12-17 at 17 06 50" src="https://github.com/user-attachments/assets/3bc7ff57-1079-4a27-90d2-88f3e09093d6" /> <img width="1727" alt="Screenshot 2024-12-17 at 17 06 22" src="https://github.com/user-attachments/assets/d5489705-6cd6-4e09-8327-fdd0f54292ea" /> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Rudolf Meijering <[email protected]>

jesuswr added 2 commits December 16, 2024 13:29

set http2 as default is ssl is enabled and add deprecation log

55229ee

remove import

55423f5

jesuswr requested a review from a team as a code owner December 16, 2024 12:52

jesuswr self-assigned this Dec 16, 2024

jesuswr added Feature:http Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Dec 16, 2024

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

b4179e0

…-fix'

jesuswr marked this pull request as draft December 16, 2024 16:26

jesuswr and others added 2 commits December 17, 2024 10:56

update tests and docs

b844339

Merge branch 'main' into set-http2-as-default-is-ssl-enabled

dc25ec1

jesuswr marked this pull request as ready for review December 17, 2024 09:58

jesuswr requested a review from a team December 17, 2024 09:58

jesuswr added the docs label Dec 17, 2024

jesuswr commented Dec 17, 2024

View reviewed changes

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

38dfb4d

…-fix'

rudolf approved these changes Dec 17, 2024

View reviewed changes

rudolf added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Dec 17, 2024

lcawl reviewed Dec 17, 2024

View reviewed changes

docs/setup/settings.asciidoc Outdated Show resolved Hide resolved

packages/core/http/core-http-server-internal/src/http_config.ts Outdated Show resolved Hide resolved

use doc links service

8e532bd

jesuswr requested a review from a team as a code owner December 18, 2024 09:15

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

94c9b57

…-fix'

rudolf reviewed Dec 18, 2024

View reviewed changes

docs/setup/settings.asciidoc Outdated Show resolved Hide resolved

jesuswr requested a review from lcawl December 18, 2024 11:25

Update docs/setup/settings.asciidoc

bc4b66d

Co-authored-by: Rudolf Meijering <[email protected]>

Merge branch 'main' into set-http2-as-default-is-ssl-enabled

93bf7d3

jesuswr added 2 commits December 19, 2024 12:42

add known issue for http2 default use

907a2c2

add known issue for http2 default use

35bc46f

change title for known issue

8326897

jesuswr commented Dec 19, 2024

View reviewed changes

docs/upgrade-notes.asciidoc Show resolved Hide resolved

Merge branch 'main' into set-http2-as-default-is-ssl-enabled

434c223

Merge branch 'main' into set-http2-as-default-is-ssl-enabled

8144875

lcawl approved these changes Jan 2, 2025

View reviewed changes

docs/upgrade-notes.asciidoc Outdated Show resolved Hide resolved

docs/upgrade-notes.asciidoc Show resolved Hide resolved

remove known issues section

16ff99e

jesuswr merged commit 1b1d64b into elastic:main Jan 3, 2025
8 checks passed

jesuswr deleted the set-http2-as-default-is-ssl-enabled branch January 3, 2025 09:21

kibanamachine added the v9.0.0 label Jan 3, 2025

rudolf changed the title ~~Set HTTP2 as default is SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1~~ Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 Jan 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 #204384

Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 #204384

jesuswr commented Dec 16, 2024 •

edited

Loading

elasticmachine commented Dec 16, 2024

jesuswr Dec 17, 2024

rudolf Dec 17, 2024

jesuswr Dec 17, 2024

rudolf Dec 17, 2024

rudolf commented Dec 18, 2024

rudolf left a comment

lcawl commented Dec 18, 2024

jesuswr commented Dec 19, 2024

jloleysens commented Dec 19, 2024 •

edited

Loading

jesuswr commented Dec 19, 2024

elasticmachine commented Dec 24, 2024

API count

lcawl left a comment

Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 #204384

Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 #204384

Conversation

jesuswr commented Dec 16, 2024 • edited Loading

Summary

Checklist

Identify risks

elasticmachine commented Dec 16, 2024

jesuswr Dec 17, 2024

Choose a reason for hiding this comment

rudolf Dec 17, 2024

Choose a reason for hiding this comment

jesuswr Dec 17, 2024

Choose a reason for hiding this comment

rudolf Dec 17, 2024

Choose a reason for hiding this comment

rudolf commented Dec 18, 2024

rudolf left a comment

Choose a reason for hiding this comment

lcawl commented Dec 18, 2024

jesuswr commented Dec 19, 2024

jloleysens commented Dec 19, 2024 • edited Loading

jesuswr commented Dec 19, 2024

elasticmachine commented Dec 24, 2024

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

lcawl left a comment

Choose a reason for hiding this comment

jesuswr commented Dec 16, 2024 •

edited

Loading

jloleysens commented Dec 19, 2024 •

edited

Loading