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

v2.12 update roles.yml with new API for experimental alerting plugin feature #4027

Merged

Conversation

AWSHurneyt
Copy link
Contributor

Description

We added a new API to the alerting plugin to support enhancements to the UI (link). The enhancements allow users to configure per query, and per bucket monitors that can query remote clusters via the UI. Previously, such monitors could only be configured using devtools/API commands.

In addition, we added support for configuring cluster metrics monitors which can execute various API (link to full list of supported API) against remote clusters. The new API is used by the frontend to also support creating these monitors via the UI.

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
    • New experimental feature
    • Enhancement to existing features
  • Why these changes are required?
    • To include this API permission in the reserved role that's shipped with the default distribution of OpenSearch.
  • What is the old behavior before changes and new behavior after changes?
    • This API was not available before the experimental launch. With the experimental feature enabled, this API can be used to retrieve a list of connect remote clusters, the indexes/aliases on those clusters along with their health statuses, and optionally the mappings for those indexes.

Issues Resolved

opensearch-project/alerting-dashboards-plugin#796

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Executed E2E tests with and without the experimental feature enabled to help ensure no regressions.

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
@AWSHurneyt
Copy link
Contributor Author

Could someone please apply the backport 2.x, and backport 2.12 labels to this PR. I don't seem to have the necessary permissions.

cwperks
cwperks previously approved these changes Feb 8, 2024
@cwperks
Copy link
Member

cwperks commented Feb 8, 2024

@AWSHurneyt Should this be added to alerting_full_access as well?

@cwperks cwperks added backport 2.x backport to 2.x branch backport 2.12 Backport to 2.12 branch labels Feb 8, 2024
@AWSHurneyt
Copy link
Contributor Author

AWSHurneyt commented Feb 8, 2024

@AWSHurneyt Should this be added to alerting_full_access as well?

@cwperks Wouldn't the cluster:admin/opensearch/alerting/* cover for this API as well?

@cwperks
Copy link
Member

cwperks commented Feb 8, 2024

Wouldn't the cluster:admin/opensearch/alerting/* cover for this API as well?

Yes it will, disregard my comment. This LGTM. The BWC tests will start passing once #4026 is merged

Copy link

codecov bot commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (321604c) 65.60% compared to head (06480e5) 65.61%.
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4027   +/-   ##
=======================================
  Coverage   65.60%   65.61%           
=======================================
  Files         298      298           
  Lines       21247    21278   +31     
  Branches     3457     3460    +3     
=======================================
+ Hits        13940    13962   +22     
- Misses       5585     5593    +8     
- Partials     1722     1723    +1     

see 4 files with indirect coverage changes

DarshitChanpura
DarshitChanpura previously approved these changes Feb 8, 2024
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

@AWSHurneyt Is this targeted for 2.12? if so, to unblock, the feature, would you mind creating the PR against 2.12 branch to avoid delays. The code freeze date is in the past but if this is needed I think we can still get this in with a request.

@DarshitChanpura
Copy link
Member

@AWSHurneyt could you run this: node check-permissions-order.js ./config/roles.yml --fix to fix Code-Hygiene check failure?

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
@AWSHurneyt AWSHurneyt dismissed stale reviews from DarshitChanpura and cwperks via 06480e5 February 8, 2024 18:50
@AWSHurneyt
Copy link
Contributor Author

@AWSHurneyt could you run this: node check-permissions-order.js ./config/roles.yml --fix to fix Code-Hygiene check failure?

Revised the PR.

@AWSHurneyt
Copy link
Contributor Author

AWSHurneyt commented Feb 8, 2024

@AWSHurneyt Is this targeted for 2.12? if so, to unblock, the feature, would you mind creating the PR against 2.12 branch to avoid delays. The code freeze date is in the past but if this is needed I think we can still get this in with a request.

@DarshitChanpura @cwperks Manual backport to the 2.12 branch
#4029

The backport 2.12 label can probably be removed from this PR.

@DarshitChanpura DarshitChanpura removed the backport 2.12 Backport to 2.12 branch label Feb 8, 2024
cwperks pushed a commit that referenced this pull request Feb 8, 2024
…alerting plugin feature #4027 (#4029)

### Description
We added a new API to the alerting plugin to support enhancements to the
UI
([link](https://github.com/opensearch-project/alerting/blob/main/alerting/src/main/kotlin/org/opensearch/alerting/action/GetRemoteIndexesAction.kt#L13)).
The enhancements allow users to configure per query, and per bucket
monitors that can query remote clusters via the UI. Previously, such
monitors could only be configured using devtools/API commands.

In addition, we added support for configuring cluster metrics monitors
which can execute various API ([link to full list of supported
API](https://opensearch.org/docs/latest/observing-your-data/alerting/per-cluster-metrics-monitors/#supported-apis))
against remote clusters. The new API is used by the frontend to also
support creating these monitors via the UI.

* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)
  * New experimental feature
  * Enhancement to existing features
* Why these changes are required?
* To include this API permission in the reserved role that's shipped
with the default distribution of OpenSearch.
* What is the old behavior before changes and new behavior after
changes?
* This API was not available before the experimental launch. With the
experimental feature enabled, this API can be used to retrieve a list of
connect remote clusters, the indexes/aliases on those clusters along
with their health statuses, and optionally the mappings for those
indexes.

### Issues Resolved

opensearch-project/alerting-dashboards-plugin#796

Is this a backport? If so, please add backport PR # and/or commits #
#4027

### Testing
Executed E2E tests with and without the experimental feature enabled to
help ensure no regressions.

### Check List
- [x] New functionality includes testing
- [x] New functionality has been documented
- [x] 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](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
@DarshitChanpura DarshitChanpura merged commit ca4eedf into opensearch-project:main Feb 9, 2024
82 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 9, 2024
…feature (#4027)

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
(cherry picked from commit ca4eedf)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
DarshitChanpura pushed a commit that referenced this pull request Feb 9, 2024
…lerting plugin feature (#4035)

Backport ca4eedf from #4027.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dlin2028 pushed a commit to dlin2028/security that referenced this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants