Skip to content

Conversation

@michalpristas
Copy link
Contributor

What does this PR do?

This PR does not apply same restrictions set on all parts of resulting index and follows 20/100/100 bytes length requirement for type, dataset and namespace

Why is it important?

Makes rules in par with the ones applied at fleet level

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@michalpristas michalpristas added Team:Ingest Management Ingest Management:beta2 Group issues for ingest management beta2 labels Sep 30, 2020
@michalpristas michalpristas self-assigned this Sep 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Team:Ingest Management)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 30, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 30, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #21406 updated]

  • Start Time: 2020-09-30T11:20:39.212+0000

  • Duration: 34 min 40 sec

Test stats 🧪

Test Results
Failed 0
Passed 1384
Skipped 4
Total 1388

Copy link
Collaborator

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Left some nits, but change LGTM.

Two things which can follow later:

  • We need docs to explain these limitations. @jen-huang has opened a PR for the docs for it. For now I would duplicate it but perhaps one day we have a place where we document the indexing strategy and can make it part of it
  • Error message: At the moment, the check only returns true or false. Instead it would be nice to return an error we can also show the user like: namespace is too long, invalid char etc.

I guess this also needs a changelog entry.


// The only two requirement are that it has only characters allowed in an Elasticsearch index name
// Index names must meet the following criteria:
// Not longer than 100 bytes
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Indentation seems to be off


// Cannot include \, /, *, ?, ", <, >, |, ` ` (space character), ,, #
if strings.ContainsAny(namespace, "\\/*?\"<>| ,#") {
if strings.ContainsAny(dsType, "\\/*?\"<>| ,#:") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we reuse the logic from dataset / namespace here to make sure it is always in sync?

return false
}

if strings.ContainsAny(dataset, "\\/*?\"<>| ,#:") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above.

It looks like all 3 methods are identical besides the length limit?

@michalpristas michalpristas merged commit 48c60f8 into elastic:master Oct 2, 2020
michalpristas added a commit to michalpristas/beats that referenced this pull request Oct 2, 2020
…e parts (elastic#21406)

[Ingest Manager] Split index restrictions into type,dataset, namespace parts (elastic#21406)
v1v added a commit to v1v/beats that referenced this pull request Oct 2, 2020
* upstream/master: (27 commits)
  [Ingest Manager] Split index restrictions into type,dataset, namespace parts (elastic#21406)
  Update Filebeat module expected logs files (elastic#21454)
  Edit SQL module docs and fix broken doc structure (elastic#21233)
  [Ingest Manager] Send snapshot flag together with metadata (elastic#21285)
  Revert "[JJBB] Set shallow cloning to 10 (elastic#21409)" (elastic#21447)
  [JJBB] Use reference repo for fast checkouts (elastic#21410)
  Add initial skeleton of filestream input (elastic#21427)
  Initial spec file for apm-server (elastic#21225)
  [Ingest Manager] Upgrade Action: make source URI optional (elastic#21372)
  Add field limit check for AWS Cloudtrail flattened fields (elastic#21388)
  [Winlogbeat] Move winlogbeat javascript processor to libbeat (elastic#21402)
  ci: pipeline to generate the changelog (elastic#21426)
  [JJBB] Set shallow cloning to 10 (elastic#21409)
  docs: add link to release notes for 7.9.2 (elastic#21405) (elastic#21419)
  docs: Prepare Changelog for 7.9.2 (elastic#21229) (elastic#21403)
  fix: mark flaky tests (elastic#21300)
  fix: use a fixed version of setuptools (elastic#21393)
  Move Kubernetes events metricset to its own block in reference config (elastic#21407)
  [libbeat] Enable WriteAheadLimit in the disk queue (elastic#21391)
  docs: fix apt/yum formatting (elastic#21362)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 2, 2020
…ne-2.0-arm

* upstream/master: (54 commits)
  [CI] Change x-pack/auditbeat build events (comments, labels) (elastic#21463)
  [CI] changeset from elastic#20603 was not added to CI2.0 (elastic#21464)
  Add new log file reader for filestream input (elastic#21450)
  [CI] Send slack message with build status (elastic#21428)
  Remove duplicated sources url in dependencies report (elastic#21462)
  Add implementation of FSWatcher and FSScanner for filestream (elastic#21444)
  [Ingest Manager] Split index restrictions into type,dataset, namespace parts (elastic#21406)
  Update Filebeat module expected logs files (elastic#21454)
  Edit SQL module docs and fix broken doc structure (elastic#21233)
  [Ingest Manager] Send snapshot flag together with metadata (elastic#21285)
  Revert "[JJBB] Set shallow cloning to 10 (elastic#21409)" (elastic#21447)
  [JJBB] Use reference repo for fast checkouts (elastic#21410)
  Add initial skeleton of filestream input (elastic#21427)
  Initial spec file for apm-server (elastic#21225)
  [Ingest Manager] Upgrade Action: make source URI optional (elastic#21372)
  Add field limit check for AWS Cloudtrail flattened fields (elastic#21388)
  [Winlogbeat] Move winlogbeat javascript processor to libbeat (elastic#21402)
  ci: pipeline to generate the changelog (elastic#21426)
  [JJBB] Set shallow cloning to 10 (elastic#21409)
  docs: add link to release notes for 7.9.2 (elastic#21405) (elastic#21419)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 2, 2020
…ci-build-label-support

* upstream/master:
  [CI] Change x-pack/auditbeat build events (comments, labels) (elastic#21463)
  [CI] changeset from elastic#20603 was not added to CI2.0 (elastic#21464)
  Add new log file reader for filestream input (elastic#21450)
  [CI] Send slack message with build status (elastic#21428)
  Remove duplicated sources url in dependencies report (elastic#21462)
  Add implementation of FSWatcher and FSScanner for filestream (elastic#21444)
  [Ingest Manager] Split index restrictions into type,dataset, namespace parts (elastic#21406)
  Update Filebeat module expected logs files (elastic#21454)
  Edit SQL module docs and fix broken doc structure (elastic#21233)
  [Ingest Manager] Send snapshot flag together with metadata (elastic#21285)
  Revert "[JJBB] Set shallow cloning to 10 (elastic#21409)" (elastic#21447)
  [JJBB] Use reference repo for fast checkouts (elastic#21410)
  Add initial skeleton of filestream input (elastic#21427)
  Initial spec file for apm-server (elastic#21225)
  [Ingest Manager] Upgrade Action: make source URI optional (elastic#21372)
  Add field limit check for AWS Cloudtrail flattened fields (elastic#21388)
  [Winlogbeat] Move winlogbeat javascript processor to libbeat (elastic#21402)
  ci: pipeline to generate the changelog (elastic#21426)
michalpristas added a commit that referenced this pull request Oct 2, 2020
…e parts (#21406) (#21459)

[Ingest Manager] Split index restrictions into type,dataset, namespace parts (#21406)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingest Management:beta2 Group issues for ingest management beta2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants