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

Separate secrets in openshift.json #4763

Closed
danmcp opened this issue Jun 20, 2018 · 9 comments
Closed

Separate secrets in openshift.json #4763

danmcp opened this issue Jun 20, 2018 · 9 comments
Assignees

Comments

@danmcp
Copy link
Contributor

danmcp commented Jun 20, 2018

openshift.json today uses hardcoded values for usernames and passwords. Ex:

https://github.com/IQSS/dataverse/blob/develop/conf/openshift/openshift.json#L182

This is obviously a bad security practice. OpenShift templates have the ability to generate values. Ex:

https://github.com/openshift/origin/blob/master/examples/sample-app/application-template-dockerbuild.json#L20

https://github.com/openshift/origin/blob/master/examples/sample-app/application-template-dockerbuild.json#L417

That's the easy part. What is also necessary is to make the usernames/passwords and other secret information configurable to be passed (through the secrets) to the install script(s).

@pdurbin
Copy link
Member

pdurbin commented Jul 3, 2018

@tkmonson thanks for making pull request #4809

I moved this issue to code review at https://waffle.io/IQSS/dataverse

@pdurbin pdurbin assigned pdurbin and unassigned tkmonson Jul 9, 2018
@djbrooke djbrooke assigned tkmonson and unassigned pdurbin Jul 9, 2018
@pdurbin
Copy link
Member

pdurbin commented Jul 9, 2018

@tkmonson hi! We discussed this issue and pull request #4809 at standup this morning. It looks like @danmcp asked you to update doc/sphinx-guides/source/developers/containers.rst over at #4809 (comment) ("update the openshift section of the readme about how this affects the experience. Ex: How would I login to postgres on the command line from the glassfish container.") and I'm happy to help you if you have questions. At http://guides.dataverse.org/en/4.9.1/developers/documentation.html you can read about how we created the Dataverse guides using Sphinx. I guess I would suggest adding to the "Troubleshooting" section:

screen shot 2018-07-09 at 11 43 31 am

@pdurbin
Copy link
Member

pdurbin commented Jul 9, 2018

@tkmonson thanks for adding those doc changes. I move this issue to QA.

@pdurbin
Copy link
Member

pdurbin commented Jul 9, 2018

Oh, while doing QA we should bear in mind that @pameyer noted at #4809 (comment) that Solr wasn't working for him:

"attempted to execute; new-app failed with solr pod deployment error. checked develop (cb98419); still seeing solr pod deployment error there as well."

@pdurbin
Copy link
Member

pdurbin commented Jul 10, 2018

Pull request #4809 was merged prematurely and the change was backed out from the "develop" branch with pull request #4820.

New pull request #4827 was just made and contains the same commit I approved for QA in the old pull request so I'm moving this issue to QA.

@tkmonson if you could write a bit here about how to test, it would be much appreciated.

@tkmonson
Copy link
Contributor

I wrote some pointers in containers.rst, but I'll paste the code here. You can log in to psql from the command line of the Glassfish pod using environment variables as parameters like so:

PGPASSWORD=$POSTGRES_PASSWORD; export PGPASSWORD; /usr/bin/psql -h $POSTGRES_SERVER.$POSTGRES_SERVICE_HOST -U $POSTGRES_USER -d $POSTGRES_DATABASE

This is the command for a regular user. For the admin user, the username is postgres and the password variable is $POSTGESQL_ADMIN_PASSWORD.

The secret information is now hidden within the code, but within the Glassfish pod it is still readable via echoing the environment variables. This could be a problem, depending on how susceptible the pod is to attack. If it is not secure enough as is, the secret information could be stored in the environment as a cryptographic hash instead.

@pdurbin
Copy link
Member

pdurbin commented Jul 10, 2018

@tkmonson thanks! I often do the same, adding the new information to the guides.

@kcondon kcondon self-assigned this Jul 11, 2018
@pdurbin pdurbin assigned tkmonson and thaorell and unassigned kcondon Jul 20, 2018
@pdurbin
Copy link
Member

pdurbin commented Jul 20, 2018

@tkmonson your pull request #4827 has merge conflicts now that the pull request #4805 has been merged. Can you please work with @thaorell (who made the pull request that got merged) to resolve the merge conflicts? Thanks!

@pdurbin
Copy link
Member

pdurbin commented Jul 20, 2018

@kcondon good news. Pull request #4827 no longer touches the installer. I didn't realize this yesterday but @tkmonson or @thaorell already merged the changes into pull request #4805 which you tested yesterday.

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

No branches or pull requests

6 participants