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

Admin Users must be able to access all monitors #139

Closed
skkosuri-amzn opened this issue Aug 9, 2021 · 12 comments
Closed

Admin Users must be able to access all monitors #139

skkosuri-amzn opened this issue Aug 9, 2021 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@skkosuri-amzn
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Admin Users must be able to access all monitors.

Describe the solution you'd like
Admin Users must be able to access all monitors. <TODO: add more details>

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

@skkosuri-amzn skkosuri-amzn added the enhancement New feature or request label Aug 9, 2021
@skkosuri-amzn skkosuri-amzn changed the title Admin Users must be able to access all monitors. Admin Users must be able to access all monitors Aug 9, 2021
@skkosuri-amzn skkosuri-amzn self-assigned this Aug 31, 2021
@dbbaughe
Copy link

Should it just be "Admin" users that we support this for? Or should it be at the permission level, i.e. anyone with a superset of permissions that overlap a job should be able to view it? And what happens when someone w/ more permissions or a single unique role/permission updates the job and now other people can't view it? I think that can be solved w/ allowing the user to choose what roles/permissions should be stored on the job?

@skkosuri-amzn
Copy link
Contributor Author

skkosuri-amzn commented Sep 20, 2021

"admin" users here refer to users with all_access role and OR admin_dn users configured in OpenSearch.yml .

Along with this change we plan to implement #128, which will remove role/permission updates to the job.

Also roles selection is also planned during the monitor creation, and plan to support only in API first.
#138

@dbbaughe
Copy link

Will you do 138 and 128 at the same time? Otherwise if you do 128 first it might cause issues, i.e. someone creates a monitor w/ all the triggers and then realize they are missing a permission -> get the required permission and try to update the monitor -> and now they can't -> they have to recreate the entire monitor instead.

@skkosuri-amzn
Copy link
Contributor Author

Yes targeting all three #139 , #138, #128 for 1.2.0

@ryn9
Copy link

ryn9 commented Sep 29, 2021

@skkosuri-amzn

Do you mean "OR" ...

"admin" users here refer to users with all_access role and admin_dn users configured in OpenSearch.yml .

IE - "admin" users here refer to users with all_access role OR admin_dn users configured in OpenSearch.yml .
This is otherwise limited in environments where the opensearch.yml is able to be edited.

Would you consider a new role (IE - alerting_admin_access) that has this access?

@ryn9
Copy link

ryn9 commented Sep 29, 2021

@skkosuri-amzn

Yes targeting all three #139 , #138, #128 for 1.2.0

I don't see any references to these on the roadmap for 1.2.0.
Can the roadmap be updated to the reflect these upcoming changes?

@skkosuri-amzn
Copy link
Contributor Author

@skkosuri-amzn

Yes targeting all three #139 , #138, #128 for 1.2.0

I don't see any references to these on the roadmap for 1.2.0. Can the roadmap be updated to the reflect these upcoming changes?

Thanks for the callout. Will add to the roadmap by early next week.

@ryn9
Copy link

ryn9 commented Oct 4, 2021

@skkosuri-amzn

Would you be able to answer ...

Do you mean "OR" ...

"admin" users here refer to users with all_access role and admin_dn users configured in OpenSearch.yml .

IE - "admin" users here refer to users with all_access role OR admin_dn users configured in OpenSearch.yml .
This is otherwise limited in environments where the opensearch.yml is able to be edited.

Would you consider a new role (IE - alerting_admin_access) that has this access?

@skkosuri-amzn
Copy link
Contributor Author

@skkosuri-amzn

Do you mean "OR" ...

"admin" users here refer to users with all_access role and admin_dn users configured in OpenSearch.yml .

IE - "admin" users here refer to users with all_access role OR admin_dn users configured in OpenSearch.yml . This is otherwise limited in environments where the opensearch.yml is able to be edited.

Would you consider a new role (IE - alerting_admin_access) that has this access?

@ryn9 yes, OR. Thanks for catching it.

@ryn9
Copy link

ryn9 commented Oct 12, 2021

@skkosuri-amzn

Would you consider a new role (IE - alerting_admin_access) that also has this access?

.

Will add to the roadmap by early next week.

Have all the additions to the roadmap been completed?

@skkosuri-amzn
Copy link
Contributor Author

Added #202 to the roadmap.

skkosuri-amzn added a commit to skkosuri-amzn/alerting-2 that referenced this issue Oct 29, 2021
skkosuri-amzn added a commit that referenced this issue Nov 2, 2021
* Admin Users must be able to access all monitors #139

* Refactored
rishabhmaurya pushed a commit to rishabhmaurya/alerting-1 that referenced this issue Nov 5, 2021
rishabhmaurya pushed a commit to rishabhmaurya/alerting-1 that referenced this issue Nov 5, 2021
rishabhmaurya added a commit that referenced this issue Nov 6, 2021
* Update copyright notice (#222)

Signed-off-by: Mohammad Qureshi <[email protected]>

* Admin Users must be able to access all monitors #139 (#220)

* Admin Users must be able to access all monitors #139

* Refactored

Co-authored-by: Mohammad Qureshi <[email protected]>
Co-authored-by: Sriram <[email protected]>
rishabhmaurya added a commit that referenced this issue Nov 8, 2021
* Admin Users must be able to access all monitors #139 (#220)

* Admin Users must be able to access all monitors #139

* Refactored

* Update copyright notice (#222)

Signed-off-by: Mohammad Qureshi <[email protected]>

Co-authored-by: Sriram <[email protected]>
Co-authored-by: Mohammad Qureshi <[email protected]>
rishabhmaurya added a commit that referenced this issue Nov 9, 2021
* Cherry-pick commits to 1.x (#227)

* Update copyright notice (#222)

Signed-off-by: Mohammad Qureshi <[email protected]>

* Admin Users must be able to access all monitors #139 (#220)

* Admin Users must be able to access all monitors #139

* Refactored

Co-authored-by: Mohammad Qureshi <[email protected]>
Co-authored-by: Sriram <[email protected]>

* Updates alerting version to 1.2 (#192)

* Updates alerting version to 1.2

* Adds snapshot repo to the repository file

Signed-off-by: Clay Downs <[email protected]>

* Update build to use public Maven repo (#184)

Signed-off-by: Abbas Hussain <[email protected]>

* Publish notification JARs checksums. (#196)

* Publish notification JARs checksums.

Signed-off-by: dblock <[email protected]>

* Remove sonatype staging.

Signed-off-by: dblock <[email protected]>

* Updates testCompile mockito version to match OpenSearch changes (#204)

Signed-off-by: Clay Downs <[email protected]>

* Update maven publication to include cksums. (#224)

This change adds a task to publish to a local staging repo under build/ that includes cksums.  It also updates build.sh to use this new task and copy the contents of the staging repo to the output directory.
The maven publish plugin will not include these cksums when publishing to maven local but will when published to a separate folder.

Signed-off-by: Marc Handalian <[email protected]>

* Add release notes for 1.2.0.0 release (#225)

* Create opensearch-alerting.release-notes-1.2.0.0.md

Signed-off-by: Annie Lee <[email protected]>

* Update opensearch-alerting.release-notes-1.2.0.0.md

* Update opensearch-alerting.release-notes-1.2.0.0.md

* Add backwards compatibility tests (#199)

* Initial commit for BWC tests

Signed-off-by: Mohammad Qureshi <[email protected]>

* Update bwc test to check Monitor stats and add bwc tests to GitHub Actions

Signed-off-by: Mohammad Qureshi <[email protected]>

* Use current version plugin bundle from build for bwc tests instead of manually uploading

Signed-off-by: Mohammad Qureshi <[email protected]>

* Update mockito-core dependency to 3.12.4 to prevent conflict

Signed-off-by: Mohammad Qureshi <[email protected]>

* Remove disabling security manager flag when running BWC tests

Signed-off-by: Mohammad Qureshi <[email protected]>

Co-authored-by: Mohammad Qureshi <[email protected]>
Co-authored-by: Sriram <[email protected]>
Co-authored-by: Clay Downs <[email protected]>
Co-authored-by: Abbas Hussain <[email protected]>
Co-authored-by: Daniel Doubrovkine (dB.) <[email protected]>
Co-authored-by: Marc Handalian <[email protected]>
Co-authored-by: Annie Lee <[email protected]>
@qreshi
Copy link
Contributor

qreshi commented Nov 17, 2021

Closing this issue as the change has been merged.

lezzago pushed a commit to lezzago/alerting-opensearch that referenced this issue Mar 9, 2022
lezzago pushed a commit to lezzago/alerting-opensearch that referenced this issue Mar 9, 2022
lezzago pushed a commit to lezzago/alerting-opensearch that referenced this issue Mar 9, 2022
AWSHurneyt pushed a commit to AWSHurneyt/OpenSearch-Alerting that referenced this issue Mar 30, 2022
* Update copyright notice (opensearch-project#222)

Signed-off-by: Mohammad Qureshi <[email protected]>

* Admin Users must be able to access all monitors opensearch-project#139 (opensearch-project#220)

* Admin Users must be able to access all monitors opensearch-project#139

* Refactored

Co-authored-by: Mohammad Qureshi <[email protected]>
Co-authored-by: Sriram <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>
AWSHurneyt pushed a commit to AWSHurneyt/OpenSearch-Alerting that referenced this issue Mar 30, 2022
opensearch-trigger-bot bot pushed a commit that referenced this issue Sep 23, 2022
)

Signed-off-by: skkosuri-amzn <[email protected]>
(cherry picked from commit 0351155)
qreshi pushed a commit that referenced this issue Sep 23, 2022
qreshi pushed a commit that referenced this issue Sep 23, 2022
) (#565)

Signed-off-by: Sriram <[email protected]>
(cherry picked from commit 0351155)

Co-authored-by: Sriram <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants