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.

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

New module: manage Memset Memstore cloudstorage users #56280

Open
wants to merge 11 commits into
base: devel
from

Conversation

Projects
None yet
7 participants
@glitchcrab
Copy link
Contributor

commented May 9, 2019

SUMMARY

This PR provides a module to manage Memstore users (Memset's cloudstorage product).

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

memset_memstore_user

ADDITIONAL INFORMATION

Further testing by internal users will be carried out shortly.

@glitchcrab

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

I'll address the shippable failures in the morning.

@timethrow

This comment has been minimized.

Copy link

commented May 10, 2019

Tested PR and all works fine for me with no issues.

@grchap

This comment has been minimized.

Copy link

commented May 13, 2019

tested, works for me.

password:
required: false
description:
- A password for the user. Required when the user is present.

This comment has been minimized.

Copy link
@jamescassell

jamescassell May 17, 2019

Contributor

Does it really have to be required? Would it make sense to auto generate a random one?

This comment has been minimized.

Copy link
@glitchcrab

glitchcrab May 17, 2019

Author Contributor

We kicked that idea around internally but most customer use-cases (in our experience) were for long-lived passwords. I've no strong opinions either way, but from an ease-of-use perspective I lean towards them providing a password they want to use.

description:
- A password for the user. Required when the user is present.
- I(update_password) must be True for an existing user's password to be updated.
update_password:

This comment has been minimized.

Copy link
@jamescassell

jamescassell May 17, 2019

Contributor

This should behave the same as the identically named option on the user module.

required: false
description:
- A password for the user. Required when the user is present.
- I(update_password) must be True for an existing user's password to be updated.

This comment has been minimized.

Copy link
@felixfontein

felixfontein May 18, 2019

Contributor
Suggested change
- I(update_password) must be True for an existing user's password to be updated.
- I(update_password) must be C(true) for an existing user's password to be updated.
if args['state'] == 'present':
try:
args['password']
except KeyError:

This comment has been minimized.

Copy link
@felixfontein

felixfontein May 18, 2019

Contributor

This exception will never be raised. args['password'] is always set; it has the value None if the user didn't specify password.

if user['enabled'] != args['enabled']:
if args['check_mode']:
retvals['changed'] = True
retvals['diff'] = "enabled: {0}" . format(args['enabled'])

This comment has been minimized.

Copy link
@felixfontein

felixfontein May 18, 2019

Contributor

That's not how diff should look like. It should be a dict with elements before and after. These will be converted (depending on the callback plugin, i.e. JSONify or YAMLify) and then diffed.

@ansibot ansibot removed the needs_triage label May 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.