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

Add DISABLE parameters for OpenSearch and Dashboards for demo certs and security plugins #436

Merged
merged 7 commits into from
Sep 10, 2021

Conversation

peterzhuamazon
Copy link
Member

@peterzhuamazon peterzhuamazon commented Sep 9, 2021

Signed-off-by: Peter Zhu zhujiaxi@amazon.com

Description

Add DISABLE parameters for OpenSearch and Dashboards for demo certs and security plugins.
See comments for more details on this.

Issues Resolved

#240
#254

Check List

  • 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.

…nd security plugins

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2021

Codecov Report

Merging #436 (ff309da) into main (6181027) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #436   +/-   ##
=======================================
  Coverage   60.36%   60.36%           
=======================================
  Files          39       39           
  Lines        1143     1143           
=======================================
  Hits          690      690           
  Misses        453      453           

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 6181027...ff309da. Read the comment docs.

@peterzhuamazon
Copy link
Member Author

peterzhuamazon commented Sep 9, 2021

Hi @danpawlik @nebulon42 please check the implementation in this PR for the issues in #240 #254.
I add 3 params for you to use:

2 for OpenSearch:
DISABLE_INSTALL_DEMO_CONFIG
DISABLE_SECURITY_PLUGIN

1 for Dashboards:
DISABLE_SECURITY_DASHBOARDS_PLUGIN

  • Scenario 1 Original behavior, demo certs + enable security on both OpenSearch and Dashboards:

    • OpenSearch:
      $ docker run -it -p 9200:9200 -p 9600:9600 -e "discovery.type=single-node" opensearchproject/opensearch:1.0.1-test
      
    • Dashboards:
      $ docker run -it --network="host" opensearchproject/opensearch-dashboards:1.0.1-test
      
  • Scenario 2, no demo certs + disable security on both OpenSearch and Dashboards:

    • OpenSearch:
      $ docker run -it -p 9200:9200 -p 9600:9600 -e "discovery.type=single-node" -e "DISABLE_INSTALL_DEMO_CONFIG=true" -e "DISABLE_SECURITY_PLUGIN=true" opensearchproject/opensearch:1.0.1-test
      
    • Dashboards:
      $ docker run -it --network="host" -e "DISABLE_SECURITY_DASHBOARDS_PLUGIN=true" opensearchproject/opensearch-dashboards:1.0.1-test
      
  • Scenario 3, no demo certs + enable security on both OpenSearch and Dashboards (cluster exit 1 as security plugin cannot find certs and the configs since demo script is not run):

    • OpenSearch:
      $ docker run -it -p 9200:9200 -p 9600:9600 -e "discovery.type=single-node" -e "DISABLE_INSTALL_DEMO_CONFIG=true" opensearchproject/opensearch:1.0.1-test
      
    • Dashboards:
      $ docker run -it --network="host" -e opensearchproject/opensearch-dashboards:1.0.1-test
      

Let me know your thoughts on this, and whether this is a good path to continue.

Thanks.

@peterzhuamazon peterzhuamazon added the bug Something isn't working label Sep 9, 2021
@danpawlik
Copy link

Applied your change and switch current container image with new one with your change and it works.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Changes look good, but I don't see updates to documentation can you roll those into this PR as well?

@peterzhuamazon
Copy link
Member Author

Changes look good, but I don't see updates to documentation can you roll those into this PR as well?

Added a detail documentation right here: https://github.com/opensearch-project/opensearch-build/blob/d6db520a637001eb85265b667e14433cbc5077b4/release/docker/README.md

peternied
peternied previously approved these changes Sep 10, 2021
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Looks great!

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
@gaiksaya gaiksaya requested review from a team and removed request for a team September 10, 2021 19:40
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Docker Image support running without security and PA plugin
5 participants