Closed ssi444 closed 2 years ago
@saravanan30erd @prudhvigodithi please take a look when you have time. Thanks.
@ssi444 Thanks for the great work.
Since it have lot of changes, I suggest to have separate PR for each of these features which will help us to review and test it in easier way. Becoz we are supporting many linux distro so we might need to test this against most of them.
@ssi444 Thanks for the great work.
Since it have lot of changes, I suggest to have separate PR for each of these features which will help us to review and test it in easier way. Becoz we are supporting many linux distro so we might need to test this against most of them.
@saravanan30erd The fact is that these changes are logically related and it is better to test them together. For example, OpenID alone makes little sense without custom configuration files.
Also, all new settings in the "default" state should lead to exactly the same result when starting the playbook as before
Calling @saravanan30erd @prudhvigodithi again for the review. Thanks.
@peterzhuamazon @saravanan30erd @prudhvigodithi ping
@saravanan30erd Please let me know if you have bandwidth to review this. @ssi444 Sorry we are focusing on two minor releases in the past weeks that is why this is pretty slow. Thanks.
@peterzhuamazon @saravanan30erd @prudhvigodithi ping
@peterzhuamazon @saravanan30erd @prudhvigodithi ping
Thanks @ssi444, LGTM! @saravanan30erd @peterzhuamazon any other thoughts and suggestions ?
@prudhvigodithi check please
Hi @ssi444,
Just so you know, I was reviewing the PR alongside @prudhvigodithi. And we think it is best to tackle this PR specifically before touching the other two.
Recently we are a bit busy on our tasks so review will be slow, especially July4 incoming. Thanks for the patience and we will get this through 😄
Thanks.
LGTM. @peterzhuamazon and @saravanan30erd
Hi team,
Just to let you know, and make cross reference. This PR introduced a few breaking changes on the default deployment. Please see below a list of issues and PRs for the fixes;
Regards
@ssi444 ^^ Thanks.
Hi team,
Just to let you know, and make cross reference. This PR introduced a few breaking changes on the default deployment. Please see below a list of issues and PRs for the fixes;
* Issue: [[BUG][Security Plugin Configuration] local action requesting unnecessary privilege escalation #82](https://github.com/opensearch-project/ansible-playbook/issues/82) * PR: [become: false on "Check that the files/internal_users.yml exists" #84](https://github.com/opensearch-project/ansible-playbook/pull/84) * Issue: [[BUG][Security Plugin Configuration] securityadmin.sh execution fails #83](https://github.com/opensearch-project/ansible-playbook/issues/83) * PR: [Fix securityadmin.sh when copy_custom_security_configs is False #85](https://github.com/opensearch-project/ansible-playbook/pull/85) * Issue: [[BUG][Security Plugin Configuration] Unecessary user left on internal_users.yml #86](https://github.com/opensearch-project/ansible-playbook/issues/86) * PR: [Remove unecessary logstash user from default deployment #87](https://github.com/opensearch-project/ansible-playbook/pull/87)
Regards
Thanks @rodolfovillordo we will take a look soon. Please bare with us as we are a bit busy with our Jenkins migration recently. Will definitely get it merged eventually 😄
Description
Issues Resolved
https://github.com/opensearch-project/ansible-playbook/issues/61
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.