community.general.gitlab_group module fails without optional arguments

[lupa95]

Summary

When i try to run community.general.gitlab_group module without specifying "subgroup_creation_level" or "project_creation_level", the task fails with gitlab error 500. Both arguments are not specified as "required" in the module documentation. I ran this task against Gitlab EE 14.7.6.

Example without specifying "subgroup_creation_level":

The full traceback is:
  File "/tmp/ansible_community.general.gitlab_group_payload_1um8dni8/ansible_community.general.gitlab_group_payload.zip/ansible_collections/community/general/plugins/modules/gitlab_group.py", line 258, in create_group
  File "/usr/local/lib/python3.8/dist-packages/gitlab/exceptions.py", line 313, in wrapped_f
    raise error(e.error_message, e.response_code, e.response_body) from e
fatal: [ttps://git.example.com]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "api_job_token": null,
            "api_oauth_token": null,
            "api_password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "api_token": null,
            "api_url": "https://git.example.com",
            "api_username": "root",
            "auto_devops_enabled": null,
            "avatar_path": null,
            "description": "test",
            "name": "mys2_first_group",
            "parent": "parent",
            "path": "mys2_first_group",
            "project_creation_level": "maintainer",
            "require_two_factor_authentication": null,
            "state": "present",
            "subgroup_creation_level": null,
            "validate_certs": false,
            "visibility": "private"
        }
    },
    "msg": "Failed to create group: 500: 500 Internal Server Error "
}

Issue Type

Bug Report

Component Name

community.general.gitlab_group

Ansible Version

$ ansible --version
ansible [core 2.12.2]
  config file = /ansible/ansible.cfg
  configured module search path = ['/ansible/modules']
  ansible python module location = /usr/local/lib/python3.8/dist-packages/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.8.10 (default, Mar 15 2022, 12:22:08) [GCC 9.4.0]
  jinja version = 3.1.1
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general
ansible [core 2.12.2]
  config file = /ansible/ansible.cfg
  configured module search path = ['/ansible/modules']
  ansible python module location = /usr/local/lib/python3.8/dist-packages/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.8.10 (default, Mar 15 2022, 12:22:08) [GCC 9.4.0]
  jinja version = 3.1.1
  libyaml = True
root@81f25ddf34f8:/ansible# community.general.gitlab_group^C
root@81f25ddf34f8:/ansible# ansible-galaxy collection list community.general

# /root/.ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 4.7.0

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

Run this task and comment out either one of the arguments mentioned in the description

- name: "Create GitLab Group"
  community.general.gitlab_group:
    api_url: https://git.example.com/
    validate_certs: False
    parent: "parent"
    subgroup_creation_level: maintainer # comment out one of those
    project_creation_level: maintainer # comment out one of those
    api_username: "test"
    description: "test"
    api_password: "test"
    name: my_first_group
    path: my_first_group
    state: present

Expected Results

I expect the module to work without specifying both arguments, because they are not marked as "required" in the module documentation and they are not part of the first three examples. (They don't work)

Actual Results

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/source_control/gitlab/gitlab_group.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @Lunik @Shaps @dj-wasabi @lgatellier @marwatk @metanovii @nejch @scodeman @sh0shin @suukit @waheedi @zanssa
click here for bot help

[nejch]

The GitLab instance probably does not like getting sent null parameters as payload, so these'll need to be added only when defined - like some of the other options just below:

community.general/plugins/modules/source_control/gitlab/gitlab_group.py

Lines 204 to 217 in 7ee38ea

	payload = {
	'name': name,
	'path': options['path'],
	'parent_id': parent_id,
	'visibility': options['visibility'],
	'project_creation_level': options['project_creation_level'],
	'auto_devops_enabled': options['auto_devops_enabled'],
	'subgroup_creation_level': options['subgroup_creation_level'],
	}
	if options.get('description'):
	payload['description'] = options['description']
	if options.get('require_two_factor_authentication'):
	payload['require_two_factor_authentication'] = options['require_two_factor_authentication']
	group = self.create_group(payload)

[lupa95]

@nejch i saw this in the gitlab server logs. I think your assumption is correct. Gitlab does not like to see null on some of these options.

==> /var/log/gitlab/gitlab-rails/exceptions_json.log <==
{"severity":"ERROR","time":"2022-04-26T13:11:32.106Z","correlation_id":"01G1JZZ6ZTK2HG8Y8WDZK01G8T","exception.class":"KeyError","exception.message":"key not found: nil","exception.backtrace":["app/models/concerns/group_api_compatibility.rb:12:in `fetch'","app/models/concerns/group_api_compatibility.rb:12:in `project_creation_level_str='","app/services/groups/create_service.rb:16:in `execute'","lib/api/groups.rb:63:in `create_group'","lib/api/groups.rb:206:in `block (2 levels) in \u003cclass:Groups\u003e'","lib/api/api_guard.rb:213:in `call'","lib/gitlab/metrics/elasticsearch_rack_middleware.rb:16:in `call'","lib/gitlab/middleware/rails_queue_duration.rb:33:in `call'","lib/gitlab/middleware/speedscope.rb:13:in `call'","lib/gitlab/request_profiler/middleware.rb:17:in `call'","lib/gitlab/database/load_balancing/rack_middleware.rb:23:in `call'","lib/gitlab/metrics/rack_middleware.rb:16:in `block in call'","lib/gitlab/metrics/web_transaction.rb:46:in `run'","lib/gitlab/metrics/rack_middleware.rb:16:in `call'","lib/gitlab/jira/middleware.rb:19:in `call'","lib/gitlab/middleware/go.rb:20:in `call'","lib/gitlab/etag_caching/middleware.rb:21:in `call'","lib/gitlab/middleware/multipart.rb:173:in `call'","lib/gitlab/middleware/read_only/controller.rb:50:in `call'","lib/gitlab/middleware/read_only.rb:18:in `call'","lib/gitlab/middleware/same_site_cookies.rb:27:in `call'","lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call'","lib/gitlab/middleware/basic_health_check.rb:25:in `call'","lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'","lib/gitlab/middleware/request_context.rb:21:in `call'","lib/gitlab/middleware/webhook_recursion_detection.rb:15:in `call'","config/initializers/fix_local_cache_middleware.rb:11:in `call'","lib/gitlab/middleware/compressed_json.rb:26:in `call'","lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call'","lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call'","lib/gitlab/metrics/requests_rack_middleware.rb:75:in `call'","lib/gitlab/middleware/release_env.rb:13:in `call'"],"user.username":"root","tags.program":"web","tags.locale":"en","tags.feature_category":"subgroups","tags.correlation_id":"01G1JZZ6ZTK2HG8Y8WDZK01G8T"}

[nejch]

Before someone starts a PR with a hardcoded fix, I'll just point out - it's hard to keep up with all the parameters that can be sent to GitLab and many are added (potentially every month) or deprecated and removed (once a year) with new GitLab versions.

Take a look at the create group API:
https://docs.gitlab.com/ee/api/groups.html#new-group

Or worse the create project API:
https://docs.gitlab.com/ee/api/projects.html#create-project

That's over 60 parameters of which the gitlab_project module currently covers around 20, and it's a moving target.

I'd almost prefer a looser schema where the user defines any extra attributes: in its own key, and have Ansible only validate those needed to find the resource and track its state etc. For example:

- name: "Create GitLab Group"
  community.general.gitlab_group:
    api_url: https://git.example.com/
    validate_certs: False
    api_username: "test"
    api_password: "test"
    state: present
    attributes:
      name: my_first_group
      path: my_first_group
      description: "test"
      parent: "parent"
      subgroup_creation_level: maintainer
      project_creation_level: maintainer

The downside is there's less validation, though I think the module could provide some helpful feedback based on the server's response as well. The 500 is unfortunate in the logs above, I think usually it would be a 405 with a more descriptive error.

You can also see this dilemma in https://github.com/ansible-collections/community.general/pull/2129/files#diff-9718af78820fd1f29c9b83d5d861a1271a46c4266d49dc6267466e0a5d0ea68cR240 where there's a snapshot of many options that will only really be true for a certain version of GitLab. I think instead it's possibly easier to point to the upstream API docs instead so that multiple versions can be supported in a kind of agnostic way (at least where possible).

Not sure if any other modules already use this approach, and obviously this would be a breaking change at some point. Any thoughts on this @felixfontein? (Edit: this might deserve its own issue, just wanted to start a discussion)

[felixfontein]

@nejch that's a really good question, and I don't have a good answer to it. It's probably best to start a new discussion in a new issue :) (Sorry for taking so long to answer, I've looked at it repeatedly and never had a really great idea...)

I think there are two perspectives:

1. As a user who wants to use only some basic functions, I would like to have a stable interface which abstracts some of the API's things away (like internal names change, or a specific way to set an option) so I can write my task now and it will works in a couple of years.
2. As a user I want to use a specific feature that most other users might not care about, and likely isn't suppoted by the module's stable interface. Then I'm basically screwed.

Combining both perspectives in the same module can be hard. Also handling perspective 2 if there are options that take more complex values (say a list of dictionaries), handling these generically is hard and probably next to impossible for all cases these values can be used for. So maybe there should be two modules? On the other hand, having two modules for the same thing potentially causes even more confusion and unnecessary complexity...

[ansibullbot]

Files identified in the description:

• plugins/modules/gitlab_group.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help