-
Notifications
You must be signed in to change notification settings - Fork 34
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
Still haven't quite stuck the landing on b5dc74e. #9
base: master
Are you sure you want to change the base?
Conversation
@@ -47,6 +47,8 @@ RedHat: | |||
ldap_conf: /etc/openldap/ldap.conf | |||
sssd_conf: /etc/sssd/sssd.conf | |||
named_conf: /etc/named.conf | |||
admin: | |||
password: {{ salt['pillar.get']('freeipa:server:admin:password', pillar.freeipa.server.ldap.password) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I don't like the idea of requiring the password to be on pillar.
Can we make it "if admin:password" is defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it should be otherwise - pillar.freeipa.server.ldap.password should if not define should get the admin password from freeipa:server:admin:password.
For me, this is the reason not to merge it right now.
@@ -12,7 +12,7 @@ freeipa_server_install: | |||
--domain {{ server.domain }} | |||
--hostname {% if server.hostname is defined %}{{ server.hostname }}{% else %}{{ grains['fqdn'] }}{% endif %} | |||
--ds-password {{ server.ldap.password }} | |||
--admin-password {{ server.admin.get('password', server.ldap.password) }} | |||
--admin-password {{ server.admin.password }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now why, to use if server.admin.password id defined.
we are in 2018, we should treat passwords better. One option here might be update formula to declare what is credentials backend, cmd.run may conditionally read pillar or use sth like https://github.com/hashicorp/envconsul (that has ability to read admin password from vault/consul/etcd ..?
One more change needed.