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

znode: add options for authentication #5306

Merged
merged 13 commits into from
Oct 5, 2022
Merged

znode: add options for authentication #5306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4c143af
add options for authentication
henkwiedig Sep 24, 2022
7f00d8f
Update changelogs/fragments/5306-add-options-for-authentication.yml
henkwiedig Sep 25, 2022
9c8f334
Update plugins/modules/clustering/znode.py
henkwiedig Sep 25, 2022
4367464
Update plugins/modules/clustering/znode.py
henkwiedig Sep 25, 2022
72fdf59
Update plugins/modules/clustering/znode.py
henkwiedig Sep 25, 2022
dbc82a8
Update plugins/modules/clustering/znode.py
henkwiedig Sep 25, 2022
e76328d
rename scheme to auth_scheme, credential to auth_credential
henkwiedig Sep 25, 2022
d5904e4
make pycodestyle happy
henkwiedig Sep 27, 2022
9adc54c
Update plugins/modules/clustering/znode.py
henkwiedig Sep 29, 2022
f7c52fe
Update plugins/modules/clustering/znode.py
henkwiedig Sep 29, 2022
ae3bfad
remove unneeded quotes
henkwiedig Sep 29, 2022
ec5a8d6
Update changelogs/fragments/5306-add-options-for-authentication.yml
henkwiedig Oct 4, 2022
79a4201
Update version_added.
felixfontein Oct 5, 2022
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 changelogs/fragments/5306-add-options-for-authentication.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- znode - possibility to use ZooKeeper ACL authentication (https://github.com/ansible-collections/community.general/pull/5306).
29 changes: 28 additions & 1 deletion plugins/modules/clustering/znode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@
- Recursively delete node and all its children.
type: bool
default: false
auth_scheme:
description:
- 'Authentication scheme.'
choices: [ digest, sasl ]
type: str
default: "digest"
required: false
henkwiedig marked this conversation as resolved.
Show resolved Hide resolved
version_added: 5.8.0
auth_credential:
description:
- The authentication credential value. Depends on I(auth_scheme).
- The format for I(auth_scheme=digest) is C(user:password),
and the format for I(auth_scheme=sasl) is C(user:password).
type: str
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering whether it's better to provide a dict here, respectively have two options username and password instead of one (credential).

Copy link
Contributor Author

@henkwiedig henkwiedig Sep 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

To be honest i don't know nothing about sasl but reading the wiki about sasl i can imagine that it does not depend on user and password but some other kind of secret based on the actual used sasl implmentation. I'm just exposing the underlying kazoo api here which also dosen't make this distinction. Besides that: this would leave us with a user password dict in case of digest and the credentail option in case of sasl. This might confuse people and increases the code to handle each case. I assume that users of zookeeper are familiar with this kind of notation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤷 I'm not using zookeeper, so no idea. If nobody else chimes in who knows anything about zookeeper, let's keep this as you suggested :)

required: false
henkwiedig marked this conversation as resolved.
Show resolved Hide resolved
version_added: 5.8.0
requirements:
- kazoo >= 2.1
- python >= 2.6
Expand All @@ -69,6 +85,13 @@
name: /mypath
op: get

- name: Getting the value and stat structure for a znode using digest authentication
community.general.znode:
hosts: 'localhost:2181'
auth_credential: 'user1:s3cr3t'
name: /secretmypath
op: get

- name: Listing a particular znode's children
community.general.znode:
hosts: 'localhost:2181'
Expand Down Expand Up @@ -122,7 +145,9 @@ def main():
op=dict(choices=['get', 'wait', 'list']),
state=dict(choices=['present', 'absent']),
timeout=dict(default=300, type='int'),
recursive=dict(default=False, type='bool')
recursive=dict(default=False, type='bool'),
auth_scheme=dict(default='digest', choices=['digest', 'sasl']),
auth_credential=dict(type='str', no_log=True),
),
supports_check_mode=False
)
Expand Down Expand Up @@ -201,6 +226,8 @@ def shutdown(self):

def start(self):
self.zk.start()
if self.module.params['auth_credential']:
self.zk.add_auth(self.module.params['auth_scheme'], self.module.params['auth_credential'])

def wait(self):
return self._wait(self.module.params['name'], self.module.params['timeout'])
Expand Down