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

Still haven't quite stuck the landing on b5dc74e. #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Still haven't quite stuck the landing on b5dc74e. #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
273cfc5
Still haven't quite stuck the landing on b5dc74e. One more change nee…
jdshewey May 28, 2018
4dac644
Trash from another project
jdshewey May 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions freeipa/map.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) }}
Copy link
Member

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?

Copy link
Member

@epcim epcim Jun 4, 2018 •
edited
Loading

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.

{%- endload %}

{%- set server = salt['grains.filter_by'](server_defaults, merge=salt['pillar.get']('freeipa:server')) %}
Expand Down
2 changes: 1 addition & 1 deletion freeipa/server/master.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Copy link
Member

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

--ssh-trust-dns
{%- if not server.get('ntp', {}).get('enabled', True) %} --no-ntp{%- endif %}
{%- if server.get('dns', {}).get('zonemgr', False) %} --zonemgr {{ server.dns.zonemgr }}{%- endif %}
Expand Down