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

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

Closed
1 task done
gabriel-preda-adswizz opened this issue Mar 28, 2023 · 8 comments · Fixed by #1764
Closed
1 task done

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

gabriel-preda-adswizz opened this issue Mar 28, 2023 · 8 comments · Fixed by #1764
Labels
bug This issue/PR relates to a bug easyfix Good for new comers and easy to start with contribution waiting_on_contributor Needs help. Feel free to engage to get things unblocked

Comments

@gabriel-preda-adswizz
Copy link

gabriel-preda-adswizz commented Mar 28, 2023

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: [email protected]
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

  • I agree to follow the Ansible Code of Conduct
@markuman markuman added the bug This issue/PR relates to a bug label Mar 29, 2023
@markuman
Copy link
Member

@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?

@markuman markuman added waiting_on_contributor Needs help. Feel free to engage to get things unblocked easyfix Good for new comers and easy to start with contribution labels Mar 29, 2023
@gabriel-preda-adswizz
Copy link
Author

gabriel-preda-adswizz commented Mar 29, 2023

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
Copy link
Member

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
Copy link
Member

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

@gabriel-preda-adswizz
Copy link
Author

So your patch is ok for me.

What next? It's your contribution 🥇

@markuman
Copy link
Member

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

  • the patch/fix
  • a changelog fragment
  • and expand the integration test that covers the bug

@eRadical
Copy link
Contributor

eRadical commented Mar 30, 2023

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
Copy link
Member

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.

softwarefactory-project-zuul bot pushed a commit that referenced this issue Apr 19, 2023
…thenticated clients (#1764)

Fix SASL/SCRAM + add option for SASL IAM & add option to disable unauthenticated clients

SUMMARY

fix SASL/SCRAM - Fixes #1761
add IAM authentication
add option to disable unauthenticated clients

Many thanx to @markuman for throwing me into this issue.
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
msk_cluster
ADDITIONAL INFORMATION
I will probably add more tests after working w/ this.

Reviewed-by: Mark Chappell
Reviewed-by: Gabriel PREDA
Reviewed-by: Markus Bergholz <[email protected]>
patchback bot pushed a commit that referenced this issue Apr 19, 2023
…thenticated clients (#1764)

Fix SASL/SCRAM + add option for SASL IAM & add option to disable unauthenticated clients

SUMMARY

fix SASL/SCRAM - Fixes #1761
add IAM authentication
add option to disable unauthenticated clients

Many thanx to @markuman for throwing me into this issue.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
msk_cluster
ADDITIONAL INFORMATION
I will probably add more tests after working w/ this.

Reviewed-by: Mark Chappell
Reviewed-by: Gabriel PREDA
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit 2fe39ba)
softwarefactory-project-zuul bot pushed a commit that referenced this issue Apr 19, 2023
…thenticated clients (#1764) (#1780)

[PR #1764/2fe39baa backport][stable-5] Fix SASL/SCRAM + add option for SASL IAM & add option to disable unauthenticated clients

This is a backport of PR #1764 as merged into main (2fe39ba).
SUMMARY

fix SASL/SCRAM - Fixes #1761
add IAM authentication
add option to disable unauthenticated clients

Many thanx to @markuman for throwing me into this issue.
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
msk_cluster
ADDITIONAL INFORMATION
I will probably add more tests after working w/ this.

Reviewed-by: Mark Chappell
abikouo pushed a commit to abikouo/community.aws that referenced this issue Oct 24, 2023
…e_iam_mfa_device_info

Promote iam_mfa_device_info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug easyfix Good for new comers and easy to start with contribution waiting_on_contributor Needs help. Feel free to engage to get things unblocked
Projects
None yet
3 participants