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

module mongodb_user fix roles default value (#46443) #46526

Merged
merged 6 commits into from Jan 22, 2019
Merged

module mongodb_user fix roles default value (#46443) #46526

merged 6 commits into from Jan 22, 2019

Conversation

jorijinnall
Copy link
Contributor

SUMMARY

Fixes issue #46443

Add default value for the parameter "roles"

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

mongodb_user module

ANSIBLE VERSION
ansible 2.8.0.dev0 (jori_fix_46443 51667a5753) last updated 2018/10/05 11:30:00 (GMT +200)
  config file = None
  configured module search path = ['/Users/jdelannoye/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/jdelannoye/Projects/Ansible/ansible/lib/ansible
  executable location = ./bin/ansible
  python version = 3.7.0 (default, Jun 29 2018, 20:13:53) [Clang 8.0.0 (clang-800.0.42.1)]

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) needs_triage Needs a first human triage before being processed. small_patch support:community This issue/PR relates to code supported by the Ansible community. labels Oct 5, 2018
@ansibot
Copy link
Contributor

ansibot commented Oct 5, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

test/sanity/validate-modules/ignore.txt:518:1: A102 Remove since "lib/ansible/modules/database/mongodb/mongodb_user.py" passes "E324" test

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Oct 5, 2018
@ansibot
Copy link
Contributor

ansibot commented Oct 5, 2018

@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 5, 2018
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Oct 5, 2018
Copy link
Contributor

@pilou- pilou- left a comment

Choose a reason for hiding this comment

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

LGTM (not tested). This fix should be backported.

@maxamillion
Copy link
Contributor

This should probably get a changelog snippet since it alters default behavior.

@pilou-
Copy link
Contributor

pilou- commented Oct 8, 2018

@jorijinnall changelog are located in changelogs/fragments directory, the related documentation is available here.

@maxamillion
Copy link
Contributor

This was discussed in #ansible-devel on freenode and the consensus was that the change would incur negative impact on users as well as providing new/different (potentially more permissive) permissions than expected. Therefore, instead of updating the module, this should simply be a documentation change so that the docs reflect what the module is actually doing.

@jorijinnall
Copy link
Contributor Author

@maxamillion Thanks for having a look to this PR. Do you recommend to close this PR and open a new one with the change you recommend, or just change this current PR ?

Thanks

@maxamillion
Copy link
Contributor

@jorijinnall just changing this one should be fine, we'll squash the commits upon merge.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 19, 2018
@jorijinnall
Copy link
Contributor Author

@maxamillion done

@ansibot
Copy link
Contributor

ansibot commented Oct 27, 2018

The test ansible-test sanity --test changelog [explain] failed with the error:

Command "/usr/bin/python test/sanity/code-smell/changelog.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "packaging/release/changelogs/changelog.py", line 814, in <module>
    main()
  File "packaging/release/changelogs/changelog.py", line 98, in main
    args.func(args)
  File "packaging/release/changelogs/changelog.py", line 109, in command_lint
    lint_fragments(fragments, exceptions)
  File "packaging/release/changelogs/changelog.py", line 227, in lint_fragments
    errors += linter.lint(fragment)
  File "packaging/release/changelogs/changelog.py", line 307, in lint
    errors += [(fragment.path, 0, 0, result[1]) for result in results]
  File "packaging/release/changelogs/changelog.py", line 307, in <listcomp>
    errors += [(fragment.path, 0, 0, result[1]) for result in results]
  File "/usr/local/lib/python3.6/dist-packages/rstcheck.py", line 169, in check
    find_ignored_languages(source)
  File "/usr/local/lib/python3.6/dist-packages/rstcheck.py", line 235, in find_ignored_languages
    for (index, line) in enumerate(source.splitlines()):
AttributeError: 'dict' object has no attribute 'splitlines'
Traceback (most recent call last):
  File "test/sanity/code-smell/changelog.py", line 14, in <module>
    main()
  File "test/sanity/code-smell/changelog.py", line 10, in main
    subprocess.check_call(cmd)
  File "/usr/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['packaging/release/changelogs/changelog.py', 'lint', 'changelogs/fragments/2.8-core-deprecations.yaml', 'changelogs/fragments/2.8-removed-modules.yaml', 'changelogs/fragments/42866-galaxy-search-unicode.yaml', 'changelogs/fragments/43123-add_support_for_per_host_no_stats.yaml', 'changelogs/fragments/43874-docker_container-stop_timeout.yaml', 'changelogs/fragments/44278-pamd_valid_simple_controls.yaml', 'changelogs/fragments/44789-docker_container-comparisons.yaml', 'changelogs/fragments/45628-fetch_url-error-headers.yaml', 'changelogs/fragments/46322-docker_container-image-not-given.yaml', 'changelogs/fragments/46443-mongodb_user-fix-roles-default-value.yaml', 'changelogs/fragments/46594-docker_container-publish-all-ports.yml', 'changelogs/fragments/46595-docker_container-expected_ports.yml', 'changelogs/fragments/46596-docker_container-published_ports.yml', 'changelogs/fragments/46598-docker_container-volume-modes.yml', 'changelogs/fragments/46739-gcp-compute-instance-metadata.yaml', 'changelogs/fragments/46743-fix-native-jinja-newlines.yaml', 'changelogs/fragments/46961_fix_aws_ec2_cache.yaml', 'changelogs/fragments/47247-docker_container-add-runtime-option.yaml', 'changelogs/fragments/47281-pamd-dont-delete-named_temporary_file_on_close.yaml', 'changelogs/fragments/47307-handler-include-task.yml', 'changelogs/fragments/add-elapsed-return-value-to-select-modules.yaml', 'changelogs/fragments/agnostic-become-prompt.yaml', 'changelogs/fragments/ajson-nested-decode.yaml', 'changelogs/fragments/ansible-doc-fixes.yml', 'changelogs/fragments/async-dir.yaml', 'changelogs/fragments/async_statys_pyx_compat_fix.yml', 'changelogs/fragments/azure_rm_appgateway-probe.yaml', 'changelogs/fragments/azure_rm_deployment_fix_45941.yaml', 'changelogs/fragments/blockinfile-bytes-fix.yaml', 'changelogs/fragments/code-cleanup-no-get-exception.yaml', 'changelogs/fragments/copy-diff-text.yaml', 'changelogs/fragments/copy-recursive-remote-src.yml', 'changelogs/fragments/dd-put-empty-files.yaml', 'changelogs/fragments/delegate_to_loop_hostvars.yaml', 'changelogs/fragments/dnf-group-removal.yaml', 'changelogs/fragments/docker-image-ids.yaml', 'changelogs/fragments/docker_container-idempotency.yaml', 'changelogs/fragments/drop-pkg_resources.yaml', 'changelogs/fragments/ec2_asg-launch-template-support.yml', 'changelogs/fragments/ec2_group_fix_target_containing_list_within_list.yaml', 'changelogs/fragments/elb_target_group_fix_KeyError.yaml', 'changelogs/fragments/fix_ec2_group_target_vpc_precedence.yaml', 'changelogs/fragments/fix_ec2_group_vpc_precedence_classic.yaml', 'changelogs/fragments/free-strategy-include-var-tags.yaml', 'changelogs/fragments/get-url-fix-idempotency.yaml', 'changelogs/fragments/get_url.yaml', 'changelogs/fragments/inv_fixes.yml', 'changelogs/fragments/lineinfile-insertbefore-index-out-of-range.yaml', 'changelogs/fragments/loop-empty-literal-list.yaml', 'changelogs/fragments/loop_undefined_delegate_to.yaml', 'changelogs/fragments/macports-upgrade-selfupdate.yml', 'changelogs/fragments/mysql-migrate_to_pymysql.yaml', 'changelogs/fragments/no-mutable-fieldattribute-defaults.yaml', 'changelogs/fragments/no_empty_groups.yml', 'changelogs/fragments/openssl-python3.yaml', 'changelogs/fragments/openstack_inventory_fix.yml', 'changelogs/fragments/piped-transfer-empty-files.yaml', 'changelogs/fragments/plugin-docs-list-fix.yaml', 'changelogs/fragments/plugin-filters-cfg.yaml', 'changelogs/fragments/postgresql_user-not-sup-error.yaml', 'changelogs/fragments/psexec-handle-socket-errors.yaml', 'changelogs/fragments/psexec-imp-error.yaml', 'changelogs/fragments/psrp-utf8.yaml', 'changelogs/fragments/reboot-show-timeout.yaml', 'changelogs/fragments/reboot-unicode-string.yaml', 'changelogs/fragments/reboot_missing_parameter.yaml', 'changelogs/fragments/reboot_openbsd_support.yaml', 'changelogs/fragments/restore_sigpipe_dfl.yml', 'changelogs/fragments/run-command-expand-shell.yaml', 'changelogs/fragments/s3_bucket_fix_non_str_tags.yaml', 'changelogs/fragments/script-module-no-file-path.yaml', 'changelogs/fragments/sns-boto3.yaml', 'changelogs/fragments/solaris-prtdiag-path.yaml', 'changelogs/fragments/tower_credential_ssh_key_data.yaml', 'changelogs/fragments/unsafe-set-wrap.yaml', 'changelogs/fragments/user-do-not-pass-ssh_key_passphrase-on-cmdline.yaml', 'changelogs/fragments/user-docs-underlying-tools.yaml', 'changelogs/fragments/v2.8.0-initial-commit.yaml', 'changelogs/fragments/win_copy-dest-quote.yaml', 'changelogs/fragments/win_group_membership-com-marshal.yaml', 'changelogs/fragments/win_package_chdir.yaml', 'changelogs/fragments/win_say-fix.yaml', 'changelogs/fragments/win_scheduled_task-repetition.yaml', 'changelogs/fragments/win_script-become.yaml', 'changelogs/fragments/windows-deprecated-functionality.yaml', 'changelogs/fragments/windows-exec-changes.yaml', 'changelogs/fragments/windows-psrp-unreachable.yaml', 'changelogs/fragments/winrm_pexpect.yaml', 'changelogs/fragments/yumdnf-update-cache.yaml']' returned non-zero exit status 1.

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_iam_password_policy.py:12:1: E302 expected 2 blank lines, found 1

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/system/user.py:876:21: E210 subprocess.Popen call found. Should be module.run_command

click here for bot help

@jorijinnall
Copy link
Contributor Author

rebase done

@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 13, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 13, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

test/sanity/validate-modules/ignore.txt:380:1: A102 Remove since "lib/ansible/modules/database/mongodb/mongodb_user.py" passes "E325" test

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Dec 13, 2018
@maxamillion
Copy link
Contributor

needs_revision

@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Dec 13, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 13, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

test/sanity/validate-modules/ignore.txt:380:1: A102 Remove since "lib/ansible/modules/database/mongodb/mongodb_user.py" passes "E325" test

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Dec 13, 2018
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Dec 20, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 28, 2018
@maxamillion
Copy link
Contributor

rebuild_merge

@jorijinnall
Copy link
Contributor Author

@maxamillion is this one can be merged or do you expect me something ?

@maxamillion
Copy link
Contributor

@jorijinnall the bot was supposed to merge it, apologies.

@maxamillion
Copy link
Contributor

bot_status

@maxamillion
Copy link
Contributor

rebuild_merge

@ansibot
Copy link
Contributor

ansibot commented Jan 21, 2019

Components

changelogs/fragments/46443-mongodb_user-fix-roles-default-value.yaml
support: community
maintainers:

lib/ansible/modules/database/mongodb/mongodb_user.py
support: community
maintainers: Lujeni elliotttf

test/sanity/validate-modules/ignore.txt
support: core
maintainers:

Metadata

waiting_on: jorijinnall
changes_requested_by: null
needs_info: False
needs_revision: True
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:
automerge: automerge shipit test failed

click here for bot help

@maxamillion maxamillion removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 22, 2019
@maxamillion
Copy link
Contributor

rebuild_merge

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jan 22, 2019
@ansibot ansibot merged commit 20936bb into ansible:devel Jan 22, 2019
@dagwieers dagwieers added the mongodb MongoDB community label Jan 28, 2019
@dagwieers dagwieers added the database Database category label Feb 13, 2019
@ansible ansible locked and limited conversation to collaborators Jul 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. database Database category module This issue/PR relates to a module. mongodb MongoDB community shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants