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

Bumped version to 1.3.0.0 #884

Merged
merged 7 commits into from
Jan 6, 2022
Merged

Conversation

davidlago
Copy link

@davidlago davidlago commented Dec 30, 2021

Signed-off-by: Dave Lago davelago@amazon.com

opensearch-project/security-dashboards-plugin pull request intake form

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

This PR takes care of the version bump along a few minor changes needed for it to work with core 1.3.0. It also takes care of, as part of a separate commit, downloading instead of building the security plugin. This will shave off a decent chunk of time from our CI.

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

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

  3. Description of changes:
    Updated version to 1.3.0.0 and dependency on dashboards to 1.3.0

  4. Why these changes are required?

  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)

  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.

Signed-off-by: Dave Lago <davelago@amazon.com>
@@ -28,7 +28,7 @@ module.exports = {
'@osd/eslint/require-license-header': [
'error',
{
license: LICENSE_HEADER,
licenses: [LICENSE_HEADER],
Copy link
Author

Choose a reason for hiding this comment

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

@davidlago davidlago requested a review from a team December 30, 2021 00:23
Dave Lago added 2 commits December 29, 2021 19:33
Signed-off-by: Dave Lago <davelago@amazon.com>
Signed-off-by: Dave Lago <davelago@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 30, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@394ad26). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #884   +/-   ##
=======================================
  Coverage        ?   71.98%           
=======================================
  Files           ?       87           
  Lines           ?     1906           
  Branches        ?      242           
=======================================
  Hits            ?     1372           
  Misses          ?      480           
  Partials        ?       54           

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 394ad26...17a24bf. Read the comment docs.

palashhedau
palashhedau previously approved these changes Jan 3, 2022
…n tests

Signed-off-by: Dave Lago <davelago@amazon.com>
Signed-off-by: Dave Lago <davelago@amazon.com>
- name: Download OpenSearch Core
run: |
wget https://artifacts.opensearch.org/snapshots/core/opensearch/1.2.0-SNAPSHOT/opensearch-min-1.2.0-SNAPSHOT-linux-x64-latest.tar.gz
wget https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/1.3.0/488/linux/x64/builds/opensearch/dist/opensearch-min-1.3.0-linux-x64.tar.gz
Copy link
Author

Choose a reason for hiding this comment

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

This is a temporary fix, moving forward we want to point to a symlinked latest build of each SNAPSHOT version, after they are provided here

Copy link
Member

Choose a reason for hiding this comment

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

It looks like your integration tests are running docker, can you pick up the latest docker opensearch distribution image as starting point instead - https://hub.docker.com/r/opensearchstaging/opensearch/tags?page=1&name=1.3.0?

I think this would clean up this integration test code sustainably by removing the interaction with build artifacts.

dblock
dblock previously approved these changes Jan 6, 2022
peternied
peternied previously approved these changes Jan 6, 2022
- name: Download OpenSearch Core
run: |
wget https://artifacts.opensearch.org/snapshots/core/opensearch/1.2.0-SNAPSHOT/opensearch-min-1.2.0-SNAPSHOT-linux-x64-latest.tar.gz
wget https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/1.3.0/488/linux/x64/builds/opensearch/dist/opensearch-min-1.3.0-linux-x64.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

It looks like your integration tests are running docker, can you pick up the latest docker opensearch distribution image as starting point instead - https://hub.docker.com/r/opensearchstaging/opensearch/tags?page=1&name=1.3.0?

I think this would clean up this integration test code sustainably by removing the interaction with build artifacts.

@davidlago davidlago dismissed stale reviews from peternied and dblock via 69b0af9 January 6, 2022 12:34
Signed-off-by: Dave Lago <davelago@amazon.com>
@davidlago
Copy link
Author

@peternied I tried pointing to the latest 1.3.0 docker image, but it is not working well. Is this a known issue? Right now it fails even when I try the command locally:

> docker run -p 9200:9200 -p 9600:9600 -e "discovery.type=single-node" opensearchstaging/opensearch:1.3.0

Enabling execution of install_demo_configuration.sh for OpenSearch Security Plugin
OpenSearch Security Demo Installer
 ** Warning: Do not use on production or public reachable systems **
Basedir: /usr/share/opensearch
OpenSearch install type: rpm/deb on NAME="Amazon Linux"
OpenSearch config dir: /usr/share/opensearch/config
OpenSearch config file: /usr/share/opensearch/config/opensearch.yml
OpenSearch bin dir: /usr/share/opensearch/bin
OpenSearch plugins dir: /usr/share/opensearch/plugins
OpenSearch lib dir: /usr/share/opensearch/lib
Detected OpenSearch Version: x-content-1.3.0
Detected OpenSearch Security Version: 1.3.0.0

### Success
### Execute this script now on all your nodes and then start all nodes
### OpenSearch Security will be automatically initialized.
### If you like to change the runtime configuration
### change the files in ../securityconfig and execute:
"/usr/share/opensearch/plugins/opensearch-security/tools/securityadmin.sh" -cd "/usr/share/opensearch/plugins/opensearch-security/securityconfig" -icl -key "/usr/share/opensearch/config/kirk-key.pem" -cert "/usr/share/opensearch/config/kirk.pem" -cacert "/usr/share/opensearch/config/root-ca.pem" -nhnv
### or run ./securityadmin_demo.sh
### To use the Security Plugin ConfigurationGUI
### To access your secured cluster open https://<hostname>:<HTTP port> and log in with admin/admin.
### (Ignore the SSL certificate warning because we installed self-signed demo certificates)
Enabling OpenSearch Security Plugin
Killing opensearch process 105
OpenSearch exited with code 143
Performance analyzer exited with code 127

So it fails right after trying to enable the security plugin, at least that's the last log line before the kill frenzy.

@peternied
Copy link
Member

@peternied I tried pointing to the latest 1.3.0 docker image, but it is not working well. Is this a known issue?

We are in bootstrapping loop. Security is an integral part to the distribution, but of course we do not have it in yet because this change attempting to increment the version so it can be added to the manifest. I think we might need to put some thought into how we can disconnect this loop or at least minimize many separate solutions for similar problems. Could you open up an issue? This clearly impacts security, but is a shared problem with the other plugins.

@dblock
Copy link
Member

dblock commented Jan 6, 2022

@peternied I tried pointing to the latest 1.3.0 docker image, but it is not working well. Is this a known issue?

We are in bootstrapping loop. Security is an integral part to the distribution, but of course we do not have it in yet because this change attempting to increment the version so it can be added to the manifest. I think we might need to put some thought into how we can disconnect this loop or at least minimize many separate solutions for similar problems. Could you open up an issue? This clearly impacts security, but is a shared problem with the other plugins.

The docker container should start without security, this is opensearch-project/opensearch-build#1185. And then security that just built should (re)install itself in that container to run tests. @davidlago want to try to do that?

@davidlago
Copy link
Author

This is weird... there should be no bootstrapping loop at this point (although I can totally see this happening as we move along with new versions), because this PR is for the Dashboards plugin, and what we are launching as a container is just the backend with security, which has been building correctly and added to the 1.3.0 manifest for a while... so something else might be up there.

@dblock we are not building security as part of this PR (well, security backend, to be clear). The only thing we need is a running container with security, but that is where the bootstraping dragons lie.

I'm going to revert my last commit on this PR while we figure out the longer term solution, so that we get unblocked and building Dashboards 1.3.0.

This reverts commit dba7d46.

Signed-off-by: Dave Lago <davelago@amazon.com>
@dblock
Copy link
Member

dblock commented Jan 6, 2022

Sorry, I may be wrong. I know the container doesn't start without security, but I don't know whether that's the currebnt problem on 1.3 (it may be PA, or whatever). Care to debug?

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Was able to build! Thanks!

@davidlago davidlago merged commit 415c95b into opensearch-project:main Jan 6, 2022
@davidlago davidlago deleted the bump-1-3-0 branch January 6, 2022 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants