Change Delete name path to OpenSearch

[davidcui1225]

Signed-off by: David Cui davidcui@amazon.com

Description

Refactor the name path for DeleteReportDefinitionAction from opendistro to opensearch.

Issues Resolved

#187

Check List

• New functionality includes testing.
  • All tests pass, including unit test, integration test and doctest
• New functionality has been documented.
  • New functionality has javadoc added
  • New functionality has user manual doc added
• 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.

[zhongnansu]

Fixing #187

[davidcui1225]

Fixing #187

Updated PR description, good catch

[zhongnansu]

What's the user impact of this issue? can't delete report definition?
Also I searched "cluster:admin/opensearch/reports/definition/delete" in secuity plugin repo, couldn't find anything, but cluster:admin/opendistro/reports/definition/delete gives me results.
https://github.com/opensearch-project/security/blob/99e157122f66462499e0a9c9b45e404ccf052ba5/securityconfig/roles.yml#L108

[joshuali925]

when did we rename it to opensearch? I thought we are keeping cluster permissions same as in ODFE

[davidcui1225]

when did we rename it to opensearch? I thought we are keeping cluster permissions same as in ODFE

Pretty sure this was part of the original migration- delete is the only path within org.opensearch.reportsscheduler.action that still has opendistro, the rest have all been named to opensearch.

[davidcui1225]

What's the user impact of this issue? can't delete report definition? Also I searched "cluster:admin/opensearch/reports/definition/delete" in secuity plugin repo, couldn't find anything, but cluster:admin/opendistro/reports/definition/delete gives me results. https://github.com/opensearch-project/security/blob/99e157122f66462499e0a9c9b45e404ccf052ba5/securityconfig/roles.yml#L108

No reported customer impact yet, but I feel we should not have leftover opendistro references in the codebase. Will have to look into what implications there are for security, but as pointed out in response to Josh's comment, Delete is the only action that still has opendistro in the path name. Everything else says opensearch

[zhongnansu]

What's the user impact of this issue? can't delete report definition? Also I searched "cluster:admin/opensearch/reports/definition/delete" in secuity plugin repo, couldn't find anything, but cluster:admin/opendistro/reports/definition/delete gives me results. https://github.com/opensearch-project/security/blob/99e157122f66462499e0a9c9b45e404ccf052ba5/securityconfig/roles.yml#L108

No reported customer impact yet, but I feel we should not have leftover opendistro references in the codebase. Will have to look into what implications there are for security, but as pointed out in response to Josh's comment, Delete is the only action that still has opendistro in the path name. Everything else says opensearch

I still feel like if secuirty plugin defaults are still using opendistro, while we use opensearch. There will be issue with permissioning. I suggest you try it out using our build-in artifacts, or docker image. Create a user with only predefined role for reporting, just enough permission for reporting, to see if it breaks @davidcui1225

[joshuali925]

What's the user impact of this issue? can't delete report definition? Also I searched "cluster:admin/opensearch/reports/definition/delete" in secuity plugin repo, couldn't find anything, but cluster:admin/opendistro/reports/definition/delete gives me results. https://github.com/opensearch-project/security/blob/99e157122f66462499e0a9c9b45e404ccf052ba5/securityconfig/roles.yml#L108

No reported customer impact yet, but I feel we should not have leftover opendistro references in the codebase. Will have to look into what implications there are for security, but as pointed out in response to Josh's comment, Delete is the only action that still has opendistro in the path name. Everything else says opensearch

I still feel like if secuirty plugin defaults are still using opendistro, while we use opensearch. There will be issue with permissioning. I suggest you try it out using our build-in artifacts, or docker image. Create a user with only predefined role for reporting, just enough permission for reporting, to see if it breaks @davidcui1225

#55 (comment) I commented in your PR previously, I thought you reverted them? we should not be using opensearch for cluster permissions

[davidcui1225]

What's the user impact of this issue? can't delete report definition? Also I searched "cluster:admin/opensearch/reports/definition/delete" in secuity plugin repo, couldn't find anything, but cluster:admin/opendistro/reports/definition/delete gives me results. https://github.com/opensearch-project/security/blob/99e157122f66462499e0a9c9b45e404ccf052ba5/securityconfig/roles.yml#L108

No reported customer impact yet, but I feel we should not have leftover opendistro references in the codebase. Will have to look into what implications there are for security, but as pointed out in response to Josh's comment, Delete is the only action that still has opendistro in the path name. Everything else says opensearch

I still feel like if secuirty plugin defaults are still using opendistro, while we use opensearch. There will be issue with permissioning. I suggest you try it out using our build-in artifacts, or docker image. Create a user with only predefined role for reporting, just enough permission for reporting, to see if it breaks @davidcui1225

#55 (comment) I commented in your PR previously, I thought you reverted them? we should not be using opensearch for cluster permissions

I guess I accidentally just reverted Delete at the time. So all of the paths should say opendistro and not opensearch, correct? I'm surprised we haven't had any reported breaking changes here.

[joshuali925]

I guess I accidentally just reverted Delete at the time. So all of the paths should say opendistro and not opensearch, correct? I'm surprised we haven't had any reported breaking changes here.

I see, might need some discussion. I'm worried that users are using the new cluster permissions now

[davidcui1225]

I guess I accidentally just reverted Delete at the time. So all of the paths should say opendistro and not opensearch, correct? I'm surprised we haven't had any reported breaking changes here.

I see, might need some discussion. I'm worried that users are using the new cluster permissions now

Do we loop in someone from security on this? I'm not too familiar with the reporting security permissions

[davidcui1225]

Closing this PR as it is superceded by #218