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

Dashboards rename related changes #1173

Conversation

hsiang9431-amzn
Copy link
Contributor

opendistro-for-elasticsearch/security pull request intake form

Please provide as much details as possible to get feedback/acceptance on your PR quickly

  1. Category: (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
    Enhancement, Documentation

  2. Github Issue # or road-map entry, if available:
    N/A

  3. Description of changes:
    Incorporate dashboards name change to security plugin

  4. Why these changes are required?
    filling gaps for dashboards name changes

  5. What is the old behavior before changes and new behavior after changes? (Please add any example/logs/screen-shot if available)

  6. Testing done: (Please provide details of testing done: Unit testing, integration testing and manual testing)
    unit tests, manual tests

  7. TO-DOs, if any: (Please describe pending items and provide Github issues# for each of them)

  8. Is it backport from main branch? (If yes, please add backport PR # and commits #)

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

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.

@hsiang9431-amzn hsiang9431-amzn force-pushed the dashboards-rename-related-changes branch from c403917 to b6bd905 Compare May 21, 2021 15:39
@hsiang9431-amzn hsiang9431-amzn requested a review from cliu123 May 21, 2021 23:28
@@ -20,7 +20,7 @@ admin:
kibanaserver:
hash: "$2a$12$4AcgAt3xwOWadA5s5blL6ev39OXDNhmOesEoo33eZtrq2N0YrU3H."
reserved: true
description: "Demo kibanaserver user"
description: "Demo opensearch dashboards server user"
Copy link
Contributor

Choose a reason for hiding this comment

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

We are changing the description but keeping the role name as "kibana" itself which causes confusion. Is this expected ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or should this PR be created at a later stage when roles are renamed also?

Copy link
Member

Choose a reason for hiding this comment

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

If this is not a magic word, write "OpenSearch Dashboards" with camelcase spelling, "server" is not a thing AFAIK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@hsiang9431-amzn hsiang9431-amzn force-pushed the dashboards-rename-related-changes branch 2 times, most recently from 7d15dee to 478bfff Compare May 26, 2021 22:59
@codecov-commenter
Copy link

Codecov Report

Merging #1173 (7d15dee) into main (9328d08) will decrease coverage by 0.03%.
The diff coverage is 40.00%.

❗ Current head 7d15dee differs from pull request most recent head 478bfff. Consider uploading reports for the commit 478bfff to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1173      +/-   ##
============================================
- Coverage     64.51%   64.47%   -0.04%     
- Complexity     3170     3171       +1     
============================================
  Files           245      245              
  Lines         17128    17128              
  Branches       3035     3035              
============================================
- Hits          11050    11044       -6     
- Misses         4538     4545       +7     
+ Partials       1540     1539       -1     
Impacted Files Coverage Δ
...opensearch/security/rest/DashboardsInfoAction.java 25.80% <0.00%> (ø)
...va/org/opensearch/security/support/ModuleType.java 89.09% <100.00%> (ø)
...org/opensearch/security/rest/TenantInfoAction.java 77.61% <0.00%> (-10.45%) ⬇️
...a/org/opensearch/security/tools/SecurityAdmin.java 47.52% <0.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9328d08...478bfff. Read the comment docs.

@cliu123 cliu123 added the maintenance Project maintenance label May 27, 2021
@hsiang9431-amzn hsiang9431-amzn force-pushed the dashboards-rename-related-changes branch from 478bfff to 13a461f Compare May 27, 2021 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Project maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants