Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add match and count APIs #324

Merged
merged 4 commits into from
Oct 22, 2020

Conversation

ohltyler
Copy link
Contributor

Issue #, if available:

Description of changes:

This PR adds frontend support for the implemented match and count APIs (see here).

These apis are used in 2 different parts for validation on the create/edit detector page, replacing the search API which caused a security leak by returning extra detector information for a user which they may not be supposed to have access to.

Notes:

  • confirmed no UT breaks
  • confirmed both APIs return correct information
  • tested on edit and create to confirm the validation for duplicate name worked appropriately

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ohltyler ohltyler added the enhancement Enhance current feature for better performance, user experience, etc label Oct 22, 2020
@@ -735,6 +737,45 @@ const getAnomalyResults = async (
}
};

const matchDetector = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious: what does this API do? search detector by name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

basically returns true or false if the passed params match any existing detector name, without returning any extra detector information that a user may not have access to if using FGAC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yizheliu-amazon
Copy link
Contributor

can you quickly check if e2e test passes?

@ohltyler
Copy link
Contributor Author

can you quickly check if e2e test passes?

Getting a flaky test failing on my local, but these changes shouldn't affect it, I imagine it should still pass fine in the github runner

Copy link
Contributor

@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 Tyler for the change!

@ohltyler ohltyler merged commit 69ff997 into opendistro-for-elasticsearch:master Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhance current feature for better performance, user experience, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants