Fix SASL/SCRAM + add option for SASL IAM & add option to disable unau…

…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

changelogs/fragments/1761-msk_cluster-fix-authentication.yml

@@ -0,0 +1,6 @@
+bugfixes:
+- msk_cluster - fix creating a cluster with sasl_scram authentication (https://github.com/ansible-collections/community.aws/issues/1761).
+minor_changes:
+- msk_cluster - add option for sasl_iam authentication and add support to disable "unauthenticated" clients (https://github.com/ansible-collections/community.aws/issues/1761)
+trivial:
+- msk_cluster - changed test to 2.8.1 the recommended default version from MSK

plugins/modules/msk_cluster.py

@@ -125,7 +125,15 @@
             sasl_scram:
                 description: SASL/SCRAM authentication is enabled or not.
                 type: bool
-                default: False
+            sasl_iam:
+                version_added: 5.5.0
+                description: IAM authentication is enabled or not.
+                type: bool
+            unauthenticated:
+                version_added: 5.5.0
+                description: Option to explicitly turn on or off authentication
+                type: bool
+                default: True
     enhanced_monitoring:
         description: Specifies the level of monitoring for the MSK cluster.
         choices:
@@ -385,13 +393,21 @@ def prepare_create_options(module):
 
     if module.params["authentication"]:
         c_params["ClientAuthentication"] = {}
-        if module.params["authentication"].get("sasl_scram"):
-            c_params["ClientAuthentication"]["Sasl"] = {
-                "Scram": module.params["authentication"]["sasl_scram"]
-            }
+        if module.params["authentication"].get("sasl_scram") or module.params["authentication"].get("sasl_iam"):
+            sasl = {}
+            if module.params["authentication"].get("sasl_scram"):
+                sasl["Scram"] = {"Enabled": True}
+            if module.params["authentication"].get("sasl_iam"):
+                sasl["Iam"] = {"Enabled": True}
+            c_params["ClientAuthentication"]["Sasl"] = sasl
         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,
+            }
+        if module.params["authentication"].get("unauthenticated"):
+            c_params["ClientAuthentication"] = {
+                "Unauthenticated": {"Enabled": True},
             }
 
     c_params.update(prepare_enhanced_monitoring_options(module))
@@ -716,7 +732,9 @@ def main():
             type="dict",
             options=dict(
                 tls_ca_arn=dict(type="list", elements="str", required=False),
-                sasl_scram=dict(type="bool", default=False),
+                sasl_scram=dict(type="bool", required=False),
+                sasl_iam=dict(type="bool", required=False),
+                unauthenticated=dict(type="bool", default=True, required=False),
             ),
         ),
         enhanced_monitoring=dict(

tests/integration/targets/msk_cluster-auth/aliases

@@ -0,0 +1,4 @@
+cloud/aws
+time=46m
+
+msk_cluster

tests/integration/targets/msk_cluster-auth/defaults/main.yml

@@ -0,0 +1,19 @@
+---
+vpc_name: "{{ resource_prefix }}-mskc-a"
+vpc_cidr: '10.{{ 256 | random(seed=resource_prefix) }}.0.0/16'
+vpc_subnets:
+  - '10.{{ 256 | random(seed=resource_prefix) }}.100.0/24'
+  - '10.{{ 256 | random(seed=resource_prefix) }}.101.0/24'
+vpc_subnet_name_prefix: "{{ resource_prefix }}"
+
+msk_config_name: "{{ resource_prefix }}-msk-cluster-auth"
+msk_cluster_name: "{{ tiny_prefix }}-msk-cluster-auth"
+msk_version: 2.8.1
+msk_broker_nodes: 2
+
+tags_create:
+  key1: "value1"
+  key2: "value2"
+tags_update:
+  key2: "value2"
+  key3: "value3"

tests/integration/targets/msk_cluster-auth/meta/main.yml

@@ -0,0 +1 @@
+dependencies: []

tests/integration/targets/msk_cluster-auth/tasks/main.yml

@@ -0,0 +1,91 @@
+---
+- name: aws_msk_cluster integration tests
+  module_defaults:
+    group/aws:
+      aws_access_key: "{{ aws_access_key }}"
+      aws_secret_key: "{{ aws_secret_key }}"
+      security_token: "{{ security_token | default(omit) }}"
+      region: "{{ aws_region }}"
+  collections:
+    - amazon.aws
+  block:
+    - name: collect availability zone info
+      aws_az_info:
+      register: az_info
+
+    - name: assert there are at least two zones
+      assert:
+        that: az_info.availability_zones | length >= 2
+
+    - name: create vpc
+      ec2_vpc_net:
+        state: present
+        cidr_block: '{{ vpc_cidr }}'
+        name: '{{ vpc_name }}'
+      register: vpc
+
+    - name: create subnets
+      ec2_vpc_subnet:
+        state: present
+        cidr: '{{ item }}'
+        az: '{{ az_info.availability_zones[index].zone_name }}'
+        vpc_id: '{{ vpc.vpc.id }}'
+        tags:
+          Name: '{{ vpc_subnet_name_prefix }}-subnet-{{ index }}'
+      loop: "{{ vpc_subnets }}"
+      loop_control:
+        index_var: index
+      register: subnets
+
+    - set_fact:
+        subnet_ids: '{{ subnets | community.general.json_query("results[].subnet.id") | list }}'
+
+     # ============================================================
+    - name: create msk configuration
+      aws_msk_config:
+        name: "{{ msk_config_name }}"
+        state: "present"
+        kafka_versions:
+          - "{{ msk_version }}"
+      register: msk_config
+
+    - name: create test with sasl_iam
+      include_tasks: test_create_auth.yml
+
+  always:
+
+    - name: delete msk cluster
+      aws_msk_cluster:
+        name: "{{ msk_cluster_name }}"
+        state: absent
+        wait: true
+      ignore_errors: yes
+
+    - name: remove msk configuration
+      aws_msk_config:
+        name: "{{ msk_config_name }}"
+        state: absent
+      ignore_errors: yes
+
+    - name: remove subnets
+      ec2_vpc_subnet:
+        state: absent
+        cidr: '{{ item }}'
+        vpc_id: '{{ vpc.vpc.id }}'
+      loop: "{{ vpc_subnets }}"
+      ignore_errors: yes
+      register: removed_subnets
+      until: removed_subnets is succeeded
+      retries: 5
+      delay: 5
+
+    - name: remove the vpc
+      ec2_vpc_net:
+        state: absent
+        cidr_block: '{{ vpc_cidr }}'
+        name: '{{ vpc_name }}'
+      ignore_errors: yes
+      register: removed_vpc
+      until: removed_vpc is success
+      retries: 5
+      delay: 5

tests/integration/targets/msk_cluster-auth/tasks/test_create_auth.yml

@@ -0,0 +1,101 @@
+---
+- name: create a msk cluster with authentication flipped from default (check mode)
+  aws_msk_cluster:
+    name: "{{ msk_cluster_name }}"
+    state: "present"
+    version: "{{ msk_version }}"
+    nodes: "{{ msk_broker_nodes }}"
+    ebs_volume_size: 10
+    authentication:
+      sasl_iam: true
+      sasl_scram: true
+      unauthenticated: false
+    subnets: "{{ subnet_ids }}"
+    wait: true
+    tags: "{{ tags_create }}"
+    configuration_arn: "{{ msk_config.arn }}"
+    configuration_revision: "{{ msk_config.revision }}"
+  check_mode: yes
+  register: msk_cluster
+
+- name: assert that the msk cluster be created
+  assert:
+    that:
+      - msk_cluster is changed
+
+- name: create a msk cluster with authentication flipped from default
+  aws_msk_cluster:
+    name: "{{ msk_cluster_name }}"
+    state: "present"
+    version: "{{ msk_version }}"
+    nodes: "{{ msk_broker_nodes }}"
+    ebs_volume_size: 10
+    authentication:
+      sasl_iam: true
+      sasl_scram: true
+      unauthenticated: false
+    subnets: "{{ subnet_ids }}"
+    wait: true
+    tags: "{{ tags_create }}"
+    configuration_arn: "{{ msk_config.arn }}"
+    configuration_revision: "{{ msk_config.revision }}"
+  register: msk_cluster
+
+- name: assert that the msk cluster is created
+  assert:
+    that:
+      - msk_cluster is changed
+
+- name: validate return values
+  assert:
+    that:
+      - "'cluster_info' in msk_cluster"
+      - "'bootstrap_broker_string' in msk_cluster"
+      - "'key1' in msk_cluster.cluster_info.tags"
+      - "msk_cluster.cluster_info.tags.key1 == 'value1'"
+      - "msk_cluster.cluster_info.cluster_name == msk_cluster_name"
+      - "msk_cluster.cluster_info.number_of_broker_nodes == msk_broker_nodes"
+      - "msk_cluster.cluster_info.broker_node_group_info.instance_type == 'kafka.t3.small'"
+      - "msk_cluster.cluster_info.broker_node_group_info.storage_info.ebs_storage_info.volume_size == 10"
+      - "msk_cluster.cluster_info.client_authentication.sasl.iam.enabled == true"
+      - "msk_cluster.cluster_info.client_authentication.sasl.scram.enabled == true"
+      # Not always returned by API
+      # - "msk_cluster.cluster_info.client_authentication.unauthenticated.enabled == false"
+      - "msk_cluster.cluster_info.open_monitoring.prometheus.jmx_exporter.enabled_in_broker == false"
+      - "msk_cluster.cluster_info.cluster_arn.startswith('arn:aws:kafka:{{ aws_region }}:')"
+
+- name: create a msk cluster with authentication flipped from default (idempotency)
+  aws_msk_cluster:
+    name: "{{ msk_cluster_name }}"
+    state: "present"
+    version: "{{ msk_version }}"
+    nodes: "{{ msk_broker_nodes }}"
+    ebs_volume_size: 10
+    authentication:
+      sasl_iam: true
+      sasl_scram: true
+      unauthenticated: false
+    subnets: "{{ subnet_ids }}"
+    wait: true
+    tags: "{{ tags_create }}"
+    configuration_arn: "{{ msk_config.arn }}"
+    configuration_revision: "{{ msk_config.revision }}"
+  register: msk_cluster
+
+- name: assert that the msk cluster wasn't changed
+  assert:
+    that:
+      - msk_cluster is not changed
+
+### Keep delete simple as we're not checking delete here
+- name: delete msk cluster
+  aws_msk_cluster:
+    name: "{{ msk_cluster_name }}"
+    state: "absent"
+    wait: true
+  register: msk_cluster
+
+- name: assert that the msk cluster is changed
+  assert:
+    that:
+      - msk_cluster is changed

tests/integration/targets/msk_cluster/defaults/main.yml

@@ -8,7 +8,7 @@ vpc_subnet_name_prefix: "{{ resource_prefix }}"
 
 msk_config_name: "{{ resource_prefix }}-msk-cluster"
 msk_cluster_name: "{{ tiny_prefix }}-msk-cluster"
-msk_version: 2.6.0
+msk_version: 2.8.1
 msk_broker_nodes: 2
 
 tags_create: