-
Notifications
You must be signed in to change notification settings - Fork 1
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
Keystone ldap #7
base: master
Are you sure you want to change the base?
Conversation
shell: ldapadd -Y EXTERNAL -H ldapi:/// -f /etc/ldap/schema/ppolicy.ldif | ||
when: ppolicy_out.stdout.count("ppolicy,") < 1 and keystone.ldap.password_policy_enabled | bool | ||
|
||
|
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.
Extra line here.
2766406
to
d4e5663
Compare
I have recommitted all the changes(along with the review comments) by fixing the author name. |
- mod_hdb1_syncprov.ldif | ||
|
||
- name: Set both oid | ||
shell: ldapmodify -Y EXTERNAL -H ldapi:/// -f {{ keystone.ldap.config_dir }}/set_both_oid.ldif |
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.
Can you clean up some of the spacing in this file? I notice there are two spaces after ldapmodify
.
Where did the Swift stuff come from? Was that there yesterday? |
This branch needs to be rebased so that all changes that were merged in to master (including swift) don't show up as changes here |
@ryshah @MaheshIBM Can y'all point out which commits should not be in this PR so we can can remove those? |
60a01d0
to
8afe719
Compare
user_allow_delete = true | ||
user_tree_dn = {{ keystone.ldap.users_dn }} | ||
user_objectclass = inetOrgPerson | ||
user_id_attribute = cn |
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.
Extra spaces here.
Removed extra spaces as per review and made a new commit c109d34 |
- name: Copy templates - replication post oid | ||
template: src={{ item }} | ||
dest={{ keystone.ldap.config_dir }}/{{ item | basename }} | ||
with_fileglob: ../templates/* |
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.
Can you try referencing this file without using ..
? They didn't like that style when we tried to commit. Think there are a few more instances of this.
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.
Its a relative path and should not be a problem, from ansible documentation "When using a relative path with with_fileglob in a role, Ansible resolves the path relative to the roles//files directory."
The pattern is used in almost all the component roles, is it ok for you to point this out to them?
If they still do not agree, I will work on an alternate fix. I would really not like to put template names in defaults (templates are not defaults, they are the only value, the template is made over-rideable by putting variables in defaults..), but if that is the only way out then be it :)
./roles/logging/tasks/main.yml: with_fileglob: ../templates/etc/rsyslog.d/*
./roles/keystone/tasks/main.yml: with_fileglob: ../templates/etc/keystone/*
./roles/glance/tasks/main.yml: with_fileglob: ../templates/etc/glance/*
./roles/openldap/tasks/password_policy.yml: with_fileglob: ../templates/password_policy/*
./roles/openldap/tasks/setup_sync_provider.yml: with_fileglob: ../templates/sync_prov/*
./roles/openldap/tasks/install-configure.yml: with_fileglob: ../templates/install_configure/*
./roles/cinder-common/tasks/main.yml: with_fileglob: ../templates/etc/cinder/*
./roles/openldap-replication/tasks/main.yml: with_fileglob: ../templates/*
./roles/neutron-common/tasks/main.yml: with_fileglob: ../templates/etc/neutron/*
./roles/nova-common/tasks/main.yml: with_fileglob: ../templates/etc/nova/*
root@ursula-deployer:~/development/ldap/ursula#
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.
Hey, no problem. I think we were using only one dot, not two. Sorry for the confusion.
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.
👍
421afb6
to
7d04b7c
Compare
Michael, any new comments? Or this is ready for a merge? |
Hi Mahesh, sorry I didn't get back to you sooner. Go ahead and open a PR against blueboxgroup/ursula, and let's them review it. (In the future I think we'll open PRs directly there.) |
e0a91e3
to
27ff8c7
Compare
66d8ab7
to
3f2c12f
Compare
8911aa5
to
88416f6
Compare
…signment will be still be sql driver, password policies for passwords can be enabled, and password syntax check controlled.
88416f6
to
6c0e508
Compare
Enablement of openldap for identity for BlueBox. This allows us to set password policies on the user passwords, save the identities in ldap backend.