ansible-collections / community.aws

Ansible Collection for Community AWS
GNU General Public License v3.0
189 stars 398 forks source link

msk_cluster - Cannot create a cluster w/ authentication sasl_scram #1761

Closed gabriel-preda-adswizz closed 1 year ago

gabriel-preda-adswizz commented 1 year ago

Summary

When I do

  community.aws.msk_cluster:
    name: "{{ item.name }}"
....................................................................
    authentication:
      sasl_scram: true
....................................................................

i get an error:

"msg": "Failed to create kafka cluster: Parameter validation failed:\nInvalid type for parameter ClientAuthentication.Sasl.Scram, value: True, type: <class 'bool'>, valid types: <class 'dict'>"

Full traceback:

The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_community.aws.msk_cluster_payload_roovgjj3/ansible_community.aws.msk_cluster_payload.zip/ansible_collections/community/aws/plugins/modules/msk_cluster.py", line 484, in create_or_update_cluster
  File "/tmp/ansible_community.aws.msk_cluster_payload_roovgjj3/ansible_community.aws.msk_cluster_payload.zip/ansible_collections/amazon/aws/plugins/module_utils/modules.py", line 354, in deciding_wrapper
    return retrying_wrapper(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/ansible_community.aws.msk_cluster_payload_roovgjj3/ansible_community.aws.msk_cluster_payload.zip/ansible_collections/amazon/aws/plugins/module_utils/cloud.py", line 119, in _retry_wrapper
    return _retry_func(
           ^^^^^^^^^^^^
  File "/tmp/ansible_community.aws.msk_cluster_payload_roovgjj3/ansible_community.aws.msk_cluster_payload.zip/ansible_collections/amazon/aws/plugins/module_utils/cloud.py", line 69, in _retry_func
    return func()
           ^^^^^^
  File "/home/.../.local/lib/python3.11/site-packages/botocore/client.py", line 530, in _api_call
    return self._make_api_call(operation_name, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/.../.local/lib/python3.11/site-packages/botocore/client.py", line 919, in _make_api_call
    request_dict = self._convert_to_request_dict(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/.../.local/lib/python3.11/site-packages/botocore/client.py", line 990, in _convert_to_request_dict
    request_dict = self._serializer.serialize_to_request(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/.../.local/lib/python3.11/site-packages/botocore/validate.py", line 381, in serialize_to_request
    raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter ClientAuthentication.Sasl.Scram, value: True, type: <class 'bool'>, valid types: <class 'dict'>
failed: [localhost] (item={'name': '...-msk-dev-2', 'configuration': '...-msk-dev-conf', 'version': '2.8.1', 'nodes': 3, 'ebs_volume_gb': 256, 'enhanced_monitoring': 'PER_TOPIC_PER_BROKER', 'instance_type': 'kafka.t3.small', 'open_monitoring': {'jmx_exporter': False, 'node_exporter': True}, 'subnets': ['subnet-...', 'subnet-...', 'subnet-...'], 'security_groups': ['sg-...'], 'tags': {'Payer': '...'}}) => {
    "ansible_loop_var": "item",
    "boto3_version": "1.26.72",
    "botocore_version": "1.29.72",
    "changed": false,
    "invocation": {
        "module_args": {
            "access_key": null,
            "authentication": {
                "sasl_scram": true,
                "tls_ca_arn": null
            },

I can create the cluster w/o authentication but it creates an Unauthenticated cluster.

Issue Type

Bug Report

Component Name

msk_cluster

Ansible Version

ansible [core 2.14.3] config file = .../infrastructure/ansible.cfg configured module search path = ['/home/.../.ansible/plugins/modules', '/usr/share/ansible/plugins/modules'] ansible python module location = /usr/lib/python3.11/site-packages/ansible ansible collection location = /home/.../.ansible/collections:/usr/share/ansible/collections executable location = /usr/bin/ansible python version = 3.11.2 (main, Feb 8 2023, 00:00:00) [GCC 12.2.1 20221121 (Red Hat 12.2.1-4)] (/usr/bin/python3) jinja version = 3.0.3 libyaml = True


### Collection Versions

$ ansible-galaxy collection list [DEPRECATION WARNING]: DEFAULT_GATHER_SUBSET option, the module_defaults keyword is a more generic version and can apply to all calls to the M(ansible.builtin.gather_facts) or M(ansible.builtin.setup) actions, use module_defaults instead. This feature will be removed from ansible-core in version 2.18. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.

/home/.../.ansible/collections/ansible_collections

Collection Version


amazon.aws 5.4.0
ansible.posix 1.5.1
community.aws 5.4.0
community.general 6.5.0
community.mongodb 1.5.1
community.mysql 3.6.0

/usr/lib/python3.11/site-packages/ansible_collections

Collection Version


amazon.aws 5.2.0
ansible.netcommon 4.1.0
ansible.posix 1.5.1
ansible.utils 2.9.0
ansible.windows 1.13.0 arista.eos 6.0.0
awx.awx 21.12.0 azure.azcollection 1.14.0 check_point.mgmt 4.0.0
chocolatey.chocolatey 1.4.0
cisco.aci 2.4.0
cisco.asa 4.0.0
cisco.dnac 6.6.3
cisco.intersight 1.0.23 cisco.ios 4.3.1
cisco.iosxr 4.1.0
cisco.ise 2.5.12 cisco.meraki 2.15.1 cisco.mso 2.2.1
cisco.nso 1.0.3
cisco.nxos 4.1.0
cisco.ucs 1.8.0
cloud.common 2.1.2
cloudscale_ch.cloud 2.2.4
community.aws 5.2.0
community.azure 2.0.0
community.ciscosmb 1.0.5
community.crypto 2.11.0 community.digitalocean 1.23.0 community.dns 2.5.1
community.docker 3.4.2
community.fortios 1.0.0
community.general 6.4.0
community.google 1.0.0
community.grafana 1.5.4
community.hashi_vault 4.1.0
community.hrobot 1.7.0
community.libvirt 1.2.0
community.mongodb 1.5.1
community.mysql 3.6.0
community.network 5.0.0
community.okd 2.3.0
community.postgresql 2.3.2
community.proxysql 1.5.1
community.rabbitmq 1.2.3
community.routeros 2.7.0
community.sap 1.0.0
community.sap_libs 1.4.0
community.skydive 1.0.0
community.sops 1.6.1
community.vmware 3.4.0
community.windows 1.12.0 community.zabbix 1.9.2
containers.podman 1.10.1 cyberark.conjur 1.2.0
cyberark.pas 1.0.17 dellemc.enterprise_sonic 2.0.0
dellemc.openmanage 6.3.0
dellemc.os10 1.1.1
dellemc.os6 1.0.7
dellemc.os9 1.0.4
dellemc.powerflex 1.5.0
dellemc.unity 1.5.0
f5networks.f5_modules 1.22.1 fortinet.fortimanager 2.1.7
fortinet.fortios 2.2.2
frr.frr 2.0.0
gluster.gluster 1.0.2
google.cloud 1.1.2
grafana.grafana 1.1.1
hetzner.hcloud 1.10.0 hpe.nimble 1.1.4
ibm.qradar 2.1.0
ibm.spectrum_virtualize 1.11.0 infinidat.infinibox 1.3.12 infoblox.nios_modules 1.4.1
inspur.ispim 1.3.0
inspur.sm 2.3.0
junipernetworks.junos 4.1.0
kubernetes.core 2.4.0
lowlydba.sqlserver 1.3.1
mellanox.onyx 1.0.0
netapp.aws 21.7.0 netapp.azure 21.10.0 netapp.cloudmanager 21.22.0 netapp.elementsw 21.7.0 netapp.ontap 22.3.0 netapp.storagegrid 21.11.1 netapp.um_info 21.8.0 netapp_eseries.santricity 1.4.0
netbox.netbox 3.11.0 ngine_io.cloudstack 2.3.0
ngine_io.exoscale 1.0.0
ngine_io.vultr 1.1.3
openstack.cloud 1.10.0 openvswitch.openvswitch 2.1.0
ovirt.ovirt 2.4.1
purestorage.flasharray 1.17.0 purestorage.flashblade 1.10.0 purestorage.fusion 1.3.0
sensu.sensu_go 1.13.2 splunk.es 2.1.0
t_systems_mms.icinga_director 1.32.0 theforeman.foreman 3.9.0
vmware.vmware_rest 2.2.0
vultr.cloud 1.7.0
vyos.vyos 4.0.0
wti.remote 1.0.4


### AWS SDK versions

$ pip show boto boto3 botocore Name: boto Version: 2.49.0 Summary: Amazon Web Services Library Home-page: https://github.com/boto/boto/ Author: Mitch Garnaat Author-email: mitch@garnaat.com License: MIT Location: /usr/local/lib/python3.11/site-packages Requires: Required-by:

Name: boto3 Version: 1.26.72 Summary: The AWS SDK for Python Home-page: https://github.com/boto/boto3 Author: Amazon Web Services Author-email: License: Apache License 2.0 Location: /home/.../.local/lib/python3.11/site-packages Requires: botocore, jmespath, s3transfer Required-by:

Name: botocore Version: 1.29.72 Summary: Low-level, data-driven core of boto 3. Home-page: https://github.com/boto/botocore Author: Amazon Web Services Author-email: License: Apache License 2.0 Location: /home/.../.local/lib/python3.11/site-packages Requires: jmespath, python-dateutil, urllib3 Required-by: awscli, boto3, s3transfer


### Configuration

```$ ansible-config dump --only-changed
[DEPRECATION WARNING]: DEFAULT_GATHER_SUBSET option, the module_defaults keyword is a more generic version and can apply to all calls to the M(ansible.builtin.gather_facts) or 
M(ansible.builtin.setup) actions, use module_defaults instead. This feature will be removed from ansible-core in version 2.18. Deprecation warnings can be disabled by setting 
deprecation_warnings=False in ansible.cfg.
CACHE_PLUGIN(/home/.../infrastructure/ansible.cfg) = jsonfile
CACHE_PLUGIN_CONNECTION(/home/.../infrastructure/ansible.cfg) = ~/.ansible/cache
CACHE_PLUGIN_TIMEOUT(/home/.../infrastructure/ansible.cfg) = 3600
CALLBACKS_ENABLED(/home/.../infrastructure/ansible.cfg) = ['timer', 'profile_tasks', 'profile_roles']
CONFIG_FILE() = /home/.../infrastructure/ansible.cfg
DEFAULT_ASK_PASS(/home/.../infrastructure/ansible.cfg) = False
DEFAULT_EXECUTABLE(/home/.../infrastructure/ansible.cfg) = /bin/bash
DEFAULT_FORCE_HANDLERS(/home/.../infrastructure/ansible.cfg) = True
DEFAULT_FORKS(/home/.../infrastructure/ansible.cfg) = 15
DEFAULT_GATHERING(/home/.../infrastructure/ansible.cfg) = smart
DEFAULT_GATHER_SUBSET(/home/.../infrastructure/ansible.cfg) = ['all']
DEFAULT_HOST_LIST(/home/.../infrastructure/ansible.cfg) = ['/home/.../infrastructure/envs']
DEFAULT_LOG_PATH(/home/.../infrastructure/ansible.cfg) = /home/.../.ansible/ansible.log
DEFAULT_MANAGED_STR(/home/.../infrastructure/ansible.cfg) = Ansible managed! DON'T CHANGE THIS FILE BY HAND! You were warned!
DEFAULT_ROLES_PATH(/home/.../infrastructure/ansible.cfg) = ['/home/.../infrastructure/roles']
DEFAULT_TIMEOUT(/home/.../infrastructure/ansible.cfg) = 30
DEPRECATION_WARNINGS(/home/.../infrastructure/ansible.cfg) = True
HOST_KEY_CHECKING(/home/.../infrastructure/ansible.cfg) = False
INVENTORY_ENABLED(/home/.../infrastructure/ansible.cfg) = ['yaml', 'aws_ec2', 'ini']
RETRY_FILES_ENABLED(/home/.../infrastructure/ansible.cfg) = False
SHOW_CUSTOM_STATS(/home/.../infrastructure/ansible.cfg) = True

OS / Environment

Fedora release 37 (Thirty Seven)

Steps to Reproduce

- name: Create MSK cluster
  community.aws.msk_cluster:
    name: "{{ item.name }}"
....................................................................
    authentication:
      sasl_scram: true

Expected Results

Create a MSK cluster w/ SASL/SCRAM authentication.

Actual Results

"msg": "Failed to create kafka cluster: Parameter validation failed:\nInvalid type for parameter ClientAuthentication.Sasl.Scram, value: True, type: <class 'bool'>, valid types: <class 'dict'>"

As I'm working on this I can jump in at any time to further debug this.

Code of Conduct

markuman commented 1 year ago

@gabriel-preda-adswizz thanks for your bug report.

https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/msk_cluster.py#L383-L392
the generated object is wrong.

ref https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/kafka/client/create_cluster.html

diff --git a/plugins/modules/msk_cluster.py b/plugins/modules/msk_cluster.py
index 65c9edea..1b045c05 100644
--- a/plugins/modules/msk_cluster.py
+++ b/plugins/modules/msk_cluster.py
@@ -384,11 +384,13 @@ def prepare_create_options(module):
         c_params["ClientAuthentication"] = {}
         if module.params["authentication"].get("sasl_scram"):
             c_params["ClientAuthentication"]["Sasl"] = {
-                "Scram": module.params["authentication"]["sasl_scram"]
+                "Scram": {
+                    "Enable": module.params["authentication"]["sasl_scram"]
             }
         if module.params["authentication"].get("tls_ca_arn"):
             c_params["ClientAuthentication"]["Tls"] = {
-                "CertificateAuthorityArnList": module.params["authentication"]["tls_ca_arn"]
+                "CertificateAuthorityArnList": module.params["authentication"]["tls_ca_arn"],
+                'Enabled': True
             }

     c_params.update(prepare_enhanced_monitoring_options(module))

this might work.
@gabriel-preda-adswizz do you have some time for work on a fix to contribute or can you test if this patch fixes the issue for you?

gabriel-preda-adswizz commented 1 year ago

Thanx @markuman.

The patch is working for me.

However aside from the above I found this in output:

   "invocation": {
        "module_args": {
            "access_key": null,
            "authentication": {
                "sasl_scram": true,
                "tls_ca_arn": null
            }

I didn't set anything about tls_ca_arn, I don't understand why I have that line in there. I only did:

authentication:
  sasl_scram: true

Thanx for the fast turnout (I can work w/ the patched version for some time).

markuman commented 1 year ago

I didn't set anything about tls_ca_arn, I don't understand why I have that line in there.

That's because the module treated tls_ca_arn as default(false) if not provided.

See: https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/msk_cluster.py#L715

So this is also a documentation bug, because the default value is missing there.

markuman commented 1 year ago

Ah not. It's wrong. There is no default. Basically the empty key must/can be popped out.

gabriel-preda-adswizz commented 1 year ago

So your patch is ok for me.

What next? It's your contribution :1st_place_medal:

markuman commented 1 year ago

@gabriel-preda-adswizz If you have time for it, feel free to make a pull request with

eRadical commented 1 year ago

Hi @markuman,

In preparing the pull request I've extended the fix for IAM authentication and also the posibility to disable unauthenticated clients that were not previously covered.

Now I'm puzzled about the tests. There are a myriad of combinations :) and I'm still thinking in how to reorganize or only add those.

markuman commented 1 year ago

Now I'm puzzled about the tests. There are a myriad of combinations :) and I'm still thinking in how to reorganize or only add those.

Mostly it helps to create a new task file that covers just that case/bug.
Example: https://github.com/ansible-collections/community.mysql/pull/503/files
New task file for the scenario revoke_only_grant.yml is included in the main.yml of the integration test.