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

added support for mustache scripting of rollup.target_index field #435

Merged
merged 14 commits into from
Aug 5, 2022

Conversation

petardz
Copy link
Contributor

@petardz petardz commented Jul 28, 2022

Issue #, if available: 61

Description of changes:

Added capability to use mustache scripting as target_index field value. Only supported param right now is source_index: target_index: rollup_{{ctx.source_index}}

CheckList:

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@petardz petardz requested a review from a team July 28, 2022 20:51
@downsrob
Copy link
Contributor

Looks like the test and build workflows are failing due to code style errors.
You can run ./gradlew detekt to run this code style workflow locally. I think the ones you added are:

VarCouldBeVal - [targetIndexResolvedName] at /home/runner/work/index-management/index-management/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/mapping/TransportUpdateRollupMappingAction.kt:63:9
VarCouldBeVal - [targetIndexResovledName] at /home/runner/work/index-management/index-management/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/mapping/TransportUpdateRollupMappingAction.kt:118:9
VarCouldBeVal - [targetIndexResolvedName] at /home/runner/work/index-management/index-management/src/main/kotlin/org/opensearch/indexmanagement/rollup/RollupMapperService.kt:73:9
VarCouldBeVal - [targetIndexResolvedName] at /home/runner/work/index-management/index-management/src/main/kotlin/org/opensearch/indexmanagement/rollup/RollupIndexer.kt:127:13

Which shouldn't be hard to fix

petardz added 2 commits July 29, 2022 00:02
Signed-off-by: Petar Dzepina <[email protected]>
Signed-off-by: Petar Dzepina <[email protected]>
@downsrob
Copy link
Contributor

We allow source index names to have wildcards, so we can have scenarios like this:
source_index: logs_etc_*
target_index: {{ctx.source_index}}rollup
At index time the target index would then be resolved to logs_etc
*_rollup and then the indexing would fail because you would be trying to write to an index with a wildcard in the name. It is also a bit of an ambiguous customer experience, as a user might expect the target index in this case to be something like logs_etc_1_rollup, logs_etc_2_rollup, etc. based on the actual source index.

To prevent these failures at runtime and clarify the behavior let's validate that the user isn't using both wildcard source indices and a template in their target index.

I think an easy way would just be to temporarily resolve the target index name in the init block of Rollup.kt and validate that it doesn't contain any wildcards. I don't mean updating the target index name with the resolved template, just resolving here to perform input validation. Feel free to implement it however you find best though!

petardz added 3 commits August 1, 2022 15:57
Signed-off-by: Petar Dzepina <[email protected]>
…etIndex on Rollup init; added test for wildcards

Signed-off-by: Petar Dzepina <[email protected]>
Signed-off-by: Petar Dzepina <[email protected]>
@downsrob
Copy link
Contributor

downsrob commented Aug 3, 2022

Thanks Petar, I am happy to approve after these.

petardz added 6 commits August 4, 2022 18:30
Signed-off-by: Petar Dzepina <[email protected]>
Signed-off-by: Petar Dzepina <[email protected]>
Signed-off-by: Petar Dzepina <[email protected]>
Signed-off-by: Petar Dzepina <[email protected]>
Signed-off-by: Petar Dzepina <[email protected]>
@petardz petardz force-pushed the dynamic_rollup_target_index branch from dbb884d to 6dc3385 Compare August 4, 2022 16:33
Signed-off-by: Petar Dzepina <[email protected]>
@downsrob downsrob merged commit e858ab2 into opensearch-project:main Aug 5, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 5, 2022
* added support for mustache scripting of rollup.target_index field

Signed-off-by: Petar Dzepina <[email protected]>

* defekt fixes

Signed-off-by: Petar Dzepina <[email protected]>

* tests

Signed-off-by: Petar Dzepina <[email protected]>

* small refactor/improvements

Signed-off-by: Petar Dzepina <[email protected]>

* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards

Signed-off-by: Petar Dzepina <[email protected]>

* lint fixes

Signed-off-by: Petar Dzepina <[email protected]>

* moved target_index validation in getRollup resp handler

Signed-off-by: Petar Dzepina <[email protected]>

* added using toMap()

Signed-off-by: Petar Dzepina <[email protected]>

* removed catch block

Signed-off-by: Petar Dzepina <[email protected]>

* exception fix

Signed-off-by: Petar Dzepina <[email protected]>

* linter fix

Signed-off-by: Petar Dzepina <[email protected]>

* fixed IT fail

Signed-off-by: Petar Dzepina <[email protected]>

* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit e858ab2)
downsrob pushed a commit that referenced this pull request Aug 5, 2022
…) (#444)

* added support for mustache scripting of rollup.target_index field

Signed-off-by: Petar Dzepina <[email protected]>

* defekt fixes

Signed-off-by: Petar Dzepina <[email protected]>

* tests

Signed-off-by: Petar Dzepina <[email protected]>

* small refactor/improvements

Signed-off-by: Petar Dzepina <[email protected]>

* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards

Signed-off-by: Petar Dzepina <[email protected]>

* lint fixes

Signed-off-by: Petar Dzepina <[email protected]>

* moved target_index validation in getRollup resp handler

Signed-off-by: Petar Dzepina <[email protected]>

* added using toMap()

Signed-off-by: Petar Dzepina <[email protected]>

* removed catch block

Signed-off-by: Petar Dzepina <[email protected]>

* exception fix

Signed-off-by: Petar Dzepina <[email protected]>

* linter fix

Signed-off-by: Petar Dzepina <[email protected]>

* fixed IT fail

Signed-off-by: Petar Dzepina <[email protected]>

* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit e858ab2)

Co-authored-by: Petar Dzepina <[email protected]>
@downsrob downsrob mentioned this pull request Aug 6, 2022
6 tasks
petardz added a commit to petardz/index-management that referenced this pull request Aug 10, 2022
…ensearch-project#435)

* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* exception fix
* linter fix
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
petardz added a commit to petardz/index-management that referenced this pull request Aug 10, 2022
…ensearch-project#435)

* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* exception fix
* linter fix
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
Angie-Zhang pushed a commit to Angie-Zhang/index-management that referenced this pull request Sep 12, 2022
…ensearch-project#435) (opensearch-project#444)

* added support for mustache scripting of rollup.target_index field

Signed-off-by: Petar Dzepina <[email protected]>

* defekt fixes

Signed-off-by: Petar Dzepina <[email protected]>

* tests

Signed-off-by: Petar Dzepina <[email protected]>

* small refactor/improvements

Signed-off-by: Petar Dzepina <[email protected]>

* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards

Signed-off-by: Petar Dzepina <[email protected]>

* lint fixes

Signed-off-by: Petar Dzepina <[email protected]>

* moved target_index validation in getRollup resp handler

Signed-off-by: Petar Dzepina <[email protected]>

* added using toMap()

Signed-off-by: Petar Dzepina <[email protected]>

* removed catch block

Signed-off-by: Petar Dzepina <[email protected]>

* exception fix

Signed-off-by: Petar Dzepina <[email protected]>

* linter fix

Signed-off-by: Petar Dzepina <[email protected]>

* fixed IT fail

Signed-off-by: Petar Dzepina <[email protected]>

* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit e858ab2)

Co-authored-by: Petar Dzepina <[email protected]>
Signed-off-by: Angie Zhang <[email protected]>
khushbr pushed a commit that referenced this pull request Nov 3, 2022
* added support for mustache scripting of rollup.target_index field (#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 3, 2022
* added support for mustache scripting of rollup.target_index field (#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit 70cf4ea)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 3, 2022
* added support for mustache scripting of rollup.target_index field (#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit 70cf4ea)
khushbr pushed a commit that referenced this pull request Nov 3, 2022
* added support for mustache scripting of rollup.target_index field (#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit 70cf4ea)

Co-authored-by: Petar Dzepina <[email protected]>
khushbr pushed a commit that referenced this pull request Nov 3, 2022
* added support for mustache scripting of rollup.target_index field (#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit 70cf4ea)

Co-authored-by: Petar Dzepina <[email protected]>
wuychn pushed a commit to ochprince/index-management that referenced this pull request Mar 16, 2023
…ensearch-project#435) (opensearch-project#444)

* added support for mustache scripting of rollup.target_index field

Signed-off-by: Petar Dzepina <[email protected]>

* defekt fixes

Signed-off-by: Petar Dzepina <[email protected]>

* tests

Signed-off-by: Petar Dzepina <[email protected]>

* small refactor/improvements

Signed-off-by: Petar Dzepina <[email protected]>

* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards

Signed-off-by: Petar Dzepina <[email protected]>

* lint fixes

Signed-off-by: Petar Dzepina <[email protected]>

* moved target_index validation in getRollup resp handler

Signed-off-by: Petar Dzepina <[email protected]>

* added using toMap()

Signed-off-by: Petar Dzepina <[email protected]>

* removed catch block

Signed-off-by: Petar Dzepina <[email protected]>

* exception fix

Signed-off-by: Petar Dzepina <[email protected]>

* linter fix

Signed-off-by: Petar Dzepina <[email protected]>

* fixed IT fail

Signed-off-by: Petar Dzepina <[email protected]>

* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit e858ab2)

Co-authored-by: Petar Dzepina <[email protected]>
wuychn pushed a commit to ochprince/index-management that referenced this pull request Mar 16, 2023
…ch-project#586)

* added support for mustache scripting of rollup.target_index field (opensearch-project#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit 70cf4ea)

Co-authored-by: Petar Dzepina <[email protected]>
ronnaksaxena pushed a commit to ronnaksaxena/index-management that referenced this pull request Jul 19, 2023
…ensearch-project#435) (opensearch-project#444)

* added support for mustache scripting of rollup.target_index field

Signed-off-by: Petar Dzepina <[email protected]>

* defekt fixes

Signed-off-by: Petar Dzepina <[email protected]>

* tests

Signed-off-by: Petar Dzepina <[email protected]>

* small refactor/improvements

Signed-off-by: Petar Dzepina <[email protected]>

* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards

Signed-off-by: Petar Dzepina <[email protected]>

* lint fixes

Signed-off-by: Petar Dzepina <[email protected]>

* moved target_index validation in getRollup resp handler

Signed-off-by: Petar Dzepina <[email protected]>

* added using toMap()

Signed-off-by: Petar Dzepina <[email protected]>

* removed catch block

Signed-off-by: Petar Dzepina <[email protected]>

* exception fix

Signed-off-by: Petar Dzepina <[email protected]>

* linter fix

Signed-off-by: Petar Dzepina <[email protected]>

* fixed IT fail

Signed-off-by: Petar Dzepina <[email protected]>

* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit e858ab2)

Co-authored-by: Petar Dzepina <[email protected]>
Signed-off-by: Ronnak Saxena <[email protected]>
ronnaksaxena pushed a commit to ronnaksaxena/index-management that referenced this pull request Jul 19, 2023
…ch-project#586)

* added support for mustache scripting of rollup.target_index field (opensearch-project#435)
* tests
* small refactor/improvements
* added wildcard check when creating rollup job; removed resolving targetIndex on Rollup init; added test for wildcards
* lint fixes
* moved target_index validation in getRollup resp handler
* removed catch block
* fixed IT fail
* added Exception catch block

Signed-off-by: Petar Dzepina <[email protected]>
(cherry picked from commit 70cf4ea)

Co-authored-by: Petar Dzepina <[email protected]>
Signed-off-by: Ronnak Saxena <[email protected]>
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.

3 participants