[Fleet] Logstash Output - being compliant to RFC-952 #176298
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adjusted form of the hostname, to be accepted if non lowercase.
botelastic
bot
added
the
Team:Fleet
|
Pinging @elastic/fleet (Team:Fleet) |
juliaElastic
added
release_note:fix
backport:prev-minor
juliaElastic
reviewed
Feb 6, 2024
| @@ -34,6 +34,12 @@ describe('Output form validation', () => { | |||
| expect(res).toBeUndefined(); | |||
| }); | |||
|
|
|||
| it('should work with hostnames using uppercase letters', () => { | |||
| const res = validateKafkaHosts(['tEsT.fr:9200', 'TEST2.fr:9200', 'teSt.fR:9999']); | |||
There was a problem hiding this comment.
did you mean to test validateLogstashHosts, since the change is there?
There was a problem hiding this comment.
Yes, yes I did thank you :D
juliaElastic
approved these changes
Feb 6, 2024
💚 Build Succeeded
Metrics [docs]Async chunks
To update your PR or re-run it, just comment with: |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 6, 2024
## Summary The logstash output in fleet was not following the [RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only accepting lowercase url's for the hostname, even though according to the standard, this should be irrelevant. Additionally added two new unit tests, to check that this behaviour is compliant in the future as well. ## Fix explanation The issue was in the past that the URL Type in nodejs is automatically lowercase, and then the value of the form is or the string provided is checked against this url type. The types behaviour is explained here: https://nodejs.org/api/url.html#the-whatwg-url-api As we would like to be case insensitive, in the check, the value is also lowerecased with: val.toLowerCase() to pass the check ### Checklist Delete any items that are not applicable to this PR. - [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 (cherry picked from commit 5542917)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Feb 6, 2024
…#176323) # Backport This will backport the following commits from `main` to `8.12`: - [[Fleet] Logstash Output - being compliant to RFC-952 (#176298)](#176298) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Alex S","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-06T16:33:56Z","message":"[Fleet] Logstash Output - being compliant to RFC-952 (#176298)\n\n## Summary\r\n\r\nThe logstash output in fleet was not following the\r\n[RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only\r\naccepting lowercase url's for the hostname, even though according to the\r\nstandard, this should be irrelevant.\r\n\r\nAdditionally added two new unit tests, to check that this behaviour is\r\ncompliant in the future as well.\r\n\r\n\r\n## Fix explanation\r\n\r\nThe issue was in the past that the URL Type in nodejs is automatically\r\nlowercase, and then the value of the form is or the string provided is\r\nchecked against this url type. The types behaviour is explained here:\r\nhttps://nodejs.org/api/url.html#the-whatwg-url-api\r\n\r\nAs we would like to be case insensitive, in the check, the value is also\r\nlowerecased with: val.toLowerCase() to pass the check\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [X] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"55429179ba30ca3a922f6daf44ce17f8d5f19507","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","backport:prev-minor","v8.13.0"],"title":"[Fleet] Logstash Output - being compliant to RFC-952","number":176298,"url":"https://github.com/elastic/kibana/pull/176298","mergeCommit":{"message":"[Fleet] Logstash Output - being compliant to RFC-952 (#176298)\n\n## Summary\r\n\r\nThe logstash output in fleet was not following the\r\n[RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only\r\naccepting lowercase url's for the hostname, even though according to the\r\nstandard, this should be irrelevant.\r\n\r\nAdditionally added two new unit tests, to check that this behaviour is\r\ncompliant in the future as well.\r\n\r\n\r\n## Fix explanation\r\n\r\nThe issue was in the past that the URL Type in nodejs is automatically\r\nlowercase, and then the value of the form is or the string provided is\r\nchecked against this url type. The types behaviour is explained here:\r\nhttps://nodejs.org/api/url.html#the-whatwg-url-api\r\n\r\nAs we would like to be case insensitive, in the check, the value is also\r\nlowerecased with: val.toLowerCase() to pass the check\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [X] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"55429179ba30ca3a922f6daf44ce17f8d5f19507"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/176298","number":176298,"mergeCommit":{"message":"[Fleet] Logstash Output - being compliant to RFC-952 (#176298)\n\n## Summary\r\n\r\nThe logstash output in fleet was not following the\r\n[RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only\r\naccepting lowercase url's for the hostname, even though according to the\r\nstandard, this should be irrelevant.\r\n\r\nAdditionally added two new unit tests, to check that this behaviour is\r\ncompliant in the future as well.\r\n\r\n\r\n## Fix explanation\r\n\r\nThe issue was in the past that the URL Type in nodejs is automatically\r\nlowercase, and then the value of the form is or the string provided is\r\nchecked against this url type. The types behaviour is explained here:\r\nhttps://nodejs.org/api/url.html#the-whatwg-url-api\r\n\r\nAs we would like to be case insensitive, in the check, the value is also\r\nlowerecased with: val.toLowerCase() to pass the check\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [X] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"55429179ba30ca3a922f6daf44ce17f8d5f19507"}}]}] BACKPORT--> Co-authored-by: Alex S <[email protected]>
|
This PR didn't make it on time to the latest build candidate of v8.12.1. Updating the labels. |
jloleysens
added a commit
that referenced
this pull request
Feb 7, 2024
* main: (224 commits) [Http] Replace `buildNr` with `buildSha` in static asset paths (#175898) [Ops] Fix GCS bucket access for future buildkite agents (#174756) [api-docs] 2024-02-07 Daily api_docs build (#176362) skip flaky suite (#176002) skip failing es promotion suite (#176359) [Cloud Security] [Grouping] Add URL Params support to the grouping components (#175749) chore(NA): update versions after v8.12.2 bump (#176309) chore(NA): update versions after v7.17.19 bump (#176313) skip failing test suite (#176352) [SLO] Enable burn rate alert by default during creation via UI (#176317) [Fleet] Add the uptime capability to observability projects (#176285) [Security Solution][Endpoint] Fix Manifest Manger so that it works with large (>10k) (#174411) [ResponseOps] Alert creation delay based on user definition (#175851) [data views] Default field formatters based on field meta values (#174973) [Cloud Security]Detection Rules counter on Rules Flyout (#176041) [Security Solution] Data Quality Dashboard persistence (#175673) [Ent Search] Connector client copy cleanup (#176290) [ML] Anomaly Detection: Adds actions menu to anomaly markers in Single Metric Viewer chart. (#175556) [ML] Anomaly Detection: Fix `values-dots` colors (#176303) [Fleet] Logstash Output - being compliant to RFC-952 (#176298) ...
fkanout
pushed a commit
to fkanout/kibana
that referenced
this pull request
Feb 7, 2024
## Summary The logstash output in fleet was not following the [RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only accepting lowercase url's for the hostname, even though according to the standard, this should be irrelevant. Additionally added two new unit tests, to check that this behaviour is compliant in the future as well. ## Fix explanation The issue was in the past that the URL Type in nodejs is automatically lowercase, and then the value of the form is or the string provided is checked against this url type. The types behaviour is explained here: https://nodejs.org/api/url.html#the-whatwg-url-api As we would like to be case insensitive, in the check, the value is also lowerecased with: val.toLowerCase() to pass the check ### Checklist Delete any items that are not applicable to this PR. - [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
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
## Summary The logstash output in fleet was not following the [RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only accepting lowercase url's for the hostname, even though according to the standard, this should be irrelevant. Additionally added two new unit tests, to check that this behaviour is compliant in the future as well. ## Fix explanation The issue was in the past that the URL Type in nodejs is automatically lowercase, and then the value of the form is or the string provided is checked against this url type. The types behaviour is explained here: https://nodejs.org/api/url.html#the-whatwg-url-api As we would like to be case insensitive, in the check, the value is also lowerecased with: val.toLowerCase() to pass the check ### Checklist Delete any items that are not applicable to this PR. - [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
fkanout
pushed a commit
to fkanout/kibana
that referenced
this pull request
Mar 4, 2024
## Summary The logstash output in fleet was not following the [RFC-952](https://www.rfc-editor.org/rfc/rfc952) correctly, only accepting lowercase url's for the hostname, even though according to the standard, this should be irrelevant. Additionally added two new unit tests, to check that this behaviour is compliant in the future as well. ## Fix explanation The issue was in the past that the URL Type in nodejs is automatically lowercase, and then the value of the form is or the string provided is checked against this url type. The types behaviour is explained here: https://nodejs.org/api/url.html#the-whatwg-url-api As we would like to be case insensitive, in the check, the value is also lowerecased with: val.toLowerCase() to pass the check ### Checklist Delete any items that are not applicable to this PR. - [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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The logstash output in fleet was not following the RFC-952 correctly, only accepting lowercase url's for the hostname, even though according to the standard, this should be irrelevant.
Additionally added two new unit tests, to check that this behaviour is compliant in the future as well.
Fix explanation
The issue was in the past that the URL Type in nodejs is automatically lowercase, and then the value of the form is or the string provided is checked against this url type. The types behaviour is explained here: https://nodejs.org/api/url.html#the-whatwg-url-api
As we would like to be case insensitive, in the check, the value is also lowerecased with: val.toLowerCase() to pass the check
Checklist
Delete any items that are not applicable to this PR.