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

gitlab: use config file & refactoring #54654

Open
wants to merge 12 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@kkoralsky
Copy link
Contributor

kkoralsky commented Mar 31, 2019

SUMMARY

This adds support to python-gitlab's configuration file. Using it has following advantages:

  • You save typing when describing a task,
  • It might be more secure, than storing api token on the controller and passing it back and forth on each task execution

If this change gets approved, I'm willing to add same code to other gitlab_* modules

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

gitlab_user

ADDITIONAL INFORMATION

before:

  tasks:
    - gitlab_user:
        api_url:  https://localhost:8443
        api_token: LUfFxzWWoYJqsY_W6JRk
        username: somebody
        validate_certs: no
        name: some body
        password: p@ssw0rd
        email: some@example.com

after:

  tasks:
    - gitlab_user:
        username: somebody
        name: somb body
        password: p@ssw0rd
        email: some@example.com
  • ~/.python-gitlab.cfg:
default = local
ssl_verify = false
[local]
url = https://localhost:8443
private_token = LUfFxzWWoYJqsY_W6JRk
api_version = 4
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 31, 2019

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:501:24: SyntaxError: if {gitlab_user, gitlab_password, gitlab_token} == {None}:

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:501:24: SyntaxError: invalid syntax

The test ansible-test sanity --test use-argspec-type-path [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:480:32: use argspec type="path" instead of type="str" to avoid use of `expanduser`

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

lib/ansible/modules/source_control/gitlab_user.py:0:0: E309 version_added for new option (config_files) should be '2.8'. Currently StrictVersion ('0.0')

click here for bot help

@Lunik

This comment has been minimized.

Copy link
Contributor

Lunik commented Mar 31, 2019

But you have to write down a config file somewhere outside of the Ansible scope

  • You save typing when describing a task,

This Ansible module don't require to be on the target server. Using delegate_to: localhost is recommended.

  • It might be more secure, than storing api token on the controller and passing it back and forth on each task execution

This change need to be done on all modules to keep consistency across them

If this change gets approved, I'm willing to add same code to other gitlab_* modules

In my opinion, this change could generate confusing error when the user forget to specify user/pass or api_token. It will result on non finding a config file that nobody told him that it should exist.

@ansibot ansibot removed the needs_triage label Mar 31, 2019

@Shaps

This comment has been minimized.

Copy link
Contributor

Shaps commented Apr 1, 2019

I wouldn't be entirely against this, AWS modules/boto do a similar thing.
The only thing I would like implemented though, is having the gitlab authentication/argument_spec in module_utils so we only have 1 common code to maintain.
I do agree with @Lunik that the change should be done in all the gitlab_* modules;
@kkoralsky If you're happy going forward with the suggestions above you can keep this PR open and set it to WIP, so we can track and help you in case you're stuck somewhere

@mattclay mattclay added the ci_verified label Apr 2, 2019

@kkoralsky kkoralsky changed the title gitlab_user: fallback to python-gitlab.cfg connection details gitlab: use config file & refactoring Apr 6, 2019

@kkoralsky kkoralsky changed the title gitlab: use config file & refactoring WIP: gitlab: use config file & refactoring Apr 6, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 6, 2019

@ansibot ansibot added WIP and removed ci_verified labels Apr 6, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 6, 2019

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:475:24: SyntaxError: if {gitlab_user, gitlab_password, gitlab_token} == {None}:

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:475:24: SyntaxError: invalid syntax

The test ansible-test sanity --test use-argspec-type-path [explain] failed with 1 error:

lib/ansible/modules/source_control/gitlab_user.py:454:32: use argspec type="path" instead of type="str" to avoid use of `expanduser`

The test ansible-test sanity --test validate-modules [explain] failed with 19 errors:

lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E322 Argument 'login_token' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_group.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_group.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E322 Argument 'login_token' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_project.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_project.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E322 Argument 'login_token' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_user.py:0:0: E309 version_added for new option (config_files) should be '2.8'. Currently StrictVersion ('0.0')

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 7, 2019

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

lib/ansible/module_utils/gitlab.py:59:26: SyntaxError: if {self.user, self.password, self.token} == {None}:

The test ansible-test sanity --test import --python 2.6 [explain] failed with 7 errors:

lib/ansible/module_utils/gitlab.py:59:26: SyntaxError: invalid syntax
lib/ansible/modules/source_control/gitlab_deploy_key.py:126:0: SyntaxError: invalid syntax (gitlab.py, line 59)
lib/ansible/modules/source_control/gitlab_group.py:147:0: SyntaxError: invalid syntax (gitlab.py, line 59)
lib/ansible/modules/source_control/gitlab_hook.py:169:0: SyntaxError: invalid syntax (gitlab.py, line 59)
lib/ansible/modules/source_control/gitlab_project.py:172:0: SyntaxError: invalid syntax (gitlab.py, line 59)
lib/ansible/modules/source_control/gitlab_runner.py:158:0: SyntaxError: invalid syntax (gitlab.py, line 59)
lib/ansible/modules/source_control/gitlab_user.py:186:0: SyntaxError: invalid syntax (gitlab.py, line 59)

The test ansible-test sanity --test validate-modules [explain] failed with 19 errors:

lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E322 Argument 'login_token' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_group.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_group.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E322 Argument 'login_token' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_project.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_project.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E322 Argument 'config_files' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E322 Argument 'login_token' is listed in the argument_spec, but not documented in the module documentation
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E324 Argument 'config_files' in argument_spec defines default as (['/etc/python-gitlab.cfg', '~/.python-gitlab.cfg']) but documentation defines default as (None)
lib/ansible/modules/source_control/gitlab_user.py:0:0: E309 version_added for new option (config_files) should be '2.8'. Currently StrictVersion ('0.0')

click here for bot help

@ansibot ansibot added the new_plugin label Apr 7, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 7, 2019

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

lib/ansible/module_utils/gitlab.py:72:26: SyntaxError: if {self.user, self.password, self.token} == {None}:

The test ansible-test sanity --test import --python 2.6 [explain] failed with 7 errors:

lib/ansible/module_utils/gitlab.py:72:26: SyntaxError: invalid syntax
lib/ansible/modules/source_control/gitlab_deploy_key.py:119:0: SyntaxError: invalid syntax (gitlab.py, line 72)
lib/ansible/modules/source_control/gitlab_group.py:145:0: SyntaxError: invalid syntax (gitlab.py, line 72)
lib/ansible/modules/source_control/gitlab_hook.py:162:0: SyntaxError: invalid syntax (gitlab.py, line 72)
lib/ansible/modules/source_control/gitlab_project.py:170:0: SyntaxError: invalid syntax (gitlab.py, line 72)
lib/ansible/modules/source_control/gitlab_runner.py:153:0: SyntaxError: invalid syntax (gitlab.py, line 72)
lib/ansible/modules/source_control/gitlab_user.py:178:0: SyntaxError: invalid syntax (gitlab.py, line 72)

The test ansible-test sanity --test validate-modules [explain] failed with 16 errors:

lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_deploy_key.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_group.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_group.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_group.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_hook.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_project.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_project.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_project.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_user.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_user.py:0:0: E323 Argument 'access_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/source_control/gitlab_user.py:0:0: E323 Argument 'private_token' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 9, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 4 errors:

lib/ansible/modules/source_control/gitlab_group.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_project.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_user.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'

click here for bot help

@ansibot ansibot added the ci_verified label Apr 9, 2019

@kkoralsky

This comment has been minimized.

Copy link
Contributor Author

kkoralsky commented Apr 9, 2019

lib/ansible/modules/source_control/gitlab_group.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_project.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_runner.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'
lib/ansible/modules/source_control/gitlab_user.py:0:0: E309 version_added for new option (api_token) should be None. Currently '2.8'

regarding these 4 errors, I belive this version_added: 2.8 should have been added earlier for these modules and because we have 2.8.0a1 now, this errors pops up.

Should I add relevant overrides to ansible/test/sanity/validate-modules/ignore.txt for them?

@kkoralsky kkoralsky changed the title WIP: gitlab: use config file & refactoring gitlab: use config file & refactoring Apr 10, 2019

@kkoralsky

This comment has been minimized.

Copy link
Contributor Author

kkoralsky commented Apr 10, 2019

@Lunik @Shaps
I think this is ready, please suggest what should I do with 'E309 errors'

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.