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

Project plan for anomaly detection features #43

Merged
merged 3 commits into from
Jul 25, 2022

Conversation

peternied
Copy link
Member

Description

Project plan for anomaly detection features

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: Peter Nied <petern@amazon.com>
@peternied
Copy link
Member Author

@opensearch-project/security Could I get a review of this set of changes as well?

dbwiddis
dbwiddis previously approved these changes Jul 12, 2022
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

This looks really good! I've made a few suggested tweaks.

SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved

#### Delayed action API [sdk#42](https://github.com/opensearch-project/opensearch-sdk/issues/42)
When actions are triggered without an interactive user session OpenSearch will need to permit the action to occur or not. Create an API for these background tasks to get an identity associated with the session.
Copy link
Member

Choose a reason for hiding this comment

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

Probably want to mention Principal identities here.

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about moving this detail into the associated issue? Its a great requirement that should be part of use cases and acceptance criteria.

Signed-off-by: Peter Nied <petern@amazon.com>

Co-authored-by: Daniel Widdis <widdis@gmail.com>
@peternied peternied marked this pull request as ready for review July 12, 2022 22:35
@peternied peternied requested a review from a team July 12, 2022 22:35
SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Outdated Show resolved Hide resolved
SECURITY.md Show resolved Hide resolved
SECURITY.md Show resolved Hide resolved
Additional background avaliable from [Security#1895](https://github.com/opensearch-project/security/issues/1895)

### User identity [OpenSearch#3846](https://github.com/opensearch-project/OpenSearch/issues/3846) :negative_squared_cross_mark:
Copy link
Member

Choose a reason for hiding this comment

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

AD exposes a bunch of actions.
For example: cluster:admin/opensearch/ad/detector/delete[1] is a Transport Action API exposed by AD.
How do we plan to support authorization of user for extensions?

[1] https://github.com/opensearch-project/anomaly-detection/blob/4baf75ca46da02c4e70016d34982f2c5a34d0acd/src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorAction.java#L20

Copy link
Member Author

Choose a reason for hiding this comment

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

With the existing security plugin in place, no additional action is needed as these are already filtered. The assumption is that the security plugins hooks will still execute before the extensions hooks are triggered so the API calls can be intercepted as is already implemented.

If/When we rewrite the permissions evaluation system I think part of the backwards compatibility story will need to account for scenarios like this where no/minimal action is needed by plugin authors.

Signed-off-by: Peter Nied <petern@amazon.com>
Copy link
Member

@saratvemulapalli saratvemulapalli left a comment

Choose a reason for hiding this comment

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

Thanks @peternied for this.
Excited to see these pieces in action.

@peternied
Copy link
Member Author

peternied commented Jul 25, 2022

@dbwiddis / @owaiskazi19 I'd like to get this change merged, any thing you'd like to see updated beforehand?

@peternied peternied merged commit 5bcb5a5 into opensearch-project:main Jul 25, 2022
@peternied peternied deleted the ad-project branch July 25, 2022 17:10
kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
* Project plan for anomaly detection features

Signed-off-by: Peter Nied <petern@amazon.com>
Co-authored-by: Daniel Widdis <widdis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants