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.

Sign up for GitHub

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

Jump to bottom

Allow setting MIQ admin password during deployment #250

Merged
merged 4 commits into from
Dec 7, 2017
Merged

Allow setting MIQ admin password during deployment #250

merged 4 commits into from
Dec 7, 2017

Conversation

fbladilo
Copy link
Contributor

@fbladilo fbladilo commented Nov 30, 2017 •
edited
Loading

  • Defines new APPLICATION_ADMIN_PASSWORD param
  • Adds necessary element to existing manageiq secrets object
  • Adds shell function calling rails runner, seeds minimal models and changes password
  • Function is called via appliance-initialize.sh, last step for new_deployment case
  • By default password is set to default MIQ value but customizable as desired at deployment time
  • Related to setting MIQ admin password during installation / template processing #237

Usage : oc new-app --template=manageiq -p APPLICATION_ADMIN_PASSWORD=mypasswd

@carbonin @bdunne

@fbladilo
Allow setting of MIQ admin password during deployment
2952bd8 
- Added admin-password element to existing secrets object
- Added parameter for APPLICATION_ADMIN_PASSWORD
- Added shell function to set password via rails runner
@fbladilo
Apply necessary changes to ext-db template
73f988a
@fbladilo
Minor parameter fix to ext-db template
542869d
@fbladilo fbladilo requested a review from carbonin November 30, 2017 20:00
carbonin
carbonin reviewed Nov 30, 2017
View reviewed changes
templates/miq-template.yaml Outdated
required: true
description: Admin password that will be set on the application.
from: "[a-zA-Z0-9]{8}"
generate: expression
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we should keep the default value here rather than using a generator.

If someone wants to change the password on deploy they can set the parameter, otherwise I think we should just keep "smartvm"

images/miq-app/docker-assets/container-scripts/container-deploy-common.sh Outdated
function set_admin_pwd() {
echo "== Setting admin password =="

cd ${APP_ROOT} && bin/rails runner -e production "EvmDatabase.seed_primordial; user = User.find_by_userid('admin'); user.password = ENV['APPLICATION_ADMIN_PASSWORD']; user.save; exit;"
Copy link
Member

Choose a reason for hiding this comment

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

I think we can simplify the ruby here.

EvmDatabase.seed_primordial; User.find_by_userid('admin').update_attributes!(:password => ENV['APPLICATION_ADMIN_PASSWORD'])

@bdunne this look good to you?

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think RAILS_ENV=production is exported in /etc/default/evm so we shouldn't need the -e production here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carbonin Yep , will go ahead and test with the simplied version. /etc/default/evm is already sourced so should work without -e.

@carbonin carbonin requested a review from bdunne November 30, 2017 21:46
@chessbyte
Copy link
Member

@fbladilo will you have time to respond to the comments in this PR?

carbonin
carbonin reviewed Dec 6, 2017
View reviewed changes
images/miq-app/docker-assets/appliance-initialize.sh Outdated
@@ -34,6 +34,9 @@ case $? in

# Restore symlinks from PV to application rootdir
restore_pv_data

# Set admin pwd
Copy link
Member

Choose a reason for hiding this comment

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

This comment hardly seems necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chessbyte Surely , already commented and working on requested changes..

@fbladilo
- Simplified password set call
896e0b7 
- Removed unnecessary comments
- Switched from auto-gen to default pwd on templates
@miq-bot
Copy link
Member

miq-bot commented Dec 7, 2017

Checked commits fbladilo/manageiq-pods@2952bd8~...896e0b7 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

@carbonin carbonin added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 7, 2017
@carbonin carbonin merged commit 941a134 into ManageIQ:master Dec 7, 2017
@carbonin carbonin mentioned this pull request Dec 7, 2017
Closed
simaishi pushed a commit that referenced this pull request Dec 11, 2017
@carbonin @simaishi
Merge pull request #250 from fbladilo/allow_set_app_admin_pwd
8786454 
Allow setting MIQ admin password during deployment
(cherry picked from commit 941a134)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 8786454e167e9833c40ca6acc446b27af0242cd1
Author: Nick Carboni <[email protected]>
Date:   Thu Dec 7 09:42:57 2017 -0500

    Merge pull request #250 from fbladilo/allow_set_app_admin_pwd
    
    Allow setting MIQ admin password during deployment
    (cherry picked from commit 941a134ca488427e4830933b53af3a60f69dd806)

@elad661 elad661 mentioned this pull request Jan 18, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carbonin carbonin carbonin approved these changes

@bdunne bdunne Awaiting requested review from bdunne bdunne is a code owner

Assignees

@gtanzillo gtanzillo

Labels
enhancement gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@fbladilo @chessbyte @miq-bot @simaishi @carbonin @gtanzillo