Ansieng 4748 jolokia ep #2194

hrishikj · 2025-08-08T14:13:01Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist:

Any variable/code changes have been validated to be backwards compatible (doesn't break upgrade)
I have added tests that prove my fix is effective or that my feature works
If required, I have ensured the changes can be discovered by cp-ansible discovery codebase
My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

Copilot

Pull Request Overview

This PR adds Jolokia access control functionality to secure Jolokia endpoints, particularly for Kafka Controller components during ZooKeeper to KRaft migration. The changes introduce new configuration options for managing Jolokia access control policies and enhance the migration playbook with proper security handling.

Adds new variables for configuring Jolokia access control with options for custom or default XML policies
Implements validation logic to ensure proper security configuration when Jolokia is enabled
Enhances ZK to KRaft migration playbook with endpoint validation and temporary policy deployment/restoration

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
roles/variables/defaults/main.yml	Adds new Jolokia access control configuration variables for global and Kafka Controller-specific settings
roles/kafka_controller/templates/kafka_jolokia.properties.j2	Updates Jolokia properties template to include access control policy configuration
roles/kafka_controller/templates/jolokia_access_control_default.xml	Provides default restrictive access control XML policy for Jolokia endpoints
roles/common/tasks/config_validations.yml	Implements comprehensive validation logic for Jolokia access control configuration
playbooks/ZKtoKraftMigration.yml	Enhances migration playbook with endpoint validation and temporary policy management
molecule/zookeeper-digest-rhel/molecule.yml	Updates test configuration to include Jolokia access control settings
molecule/mtls-debian12/molecule.yml	Updates test configuration to include Jolokia access control settings

Copilot · 2025-08-08T14:14:29Z

roles/kafka_controller/templates/kafka_jolokia.properties.j2

 password={{kafka_controller_jolokia_password}}
 {% endif %}
+{% if kafka_controller_jolokia_access_control_enabled|bool %}
+policy={{ lookup('file', kafka_controller_jolokia_access_control_file_src_path) | replace('\n', '') | replace(' ', '') }}


The policy configuration line removes all newlines and spaces from the XML file, which will likely break XML structure and make it invalid. XML requires proper formatting with spaces and newlines for valid parsing.

Suggested change

policy={{ lookup('file', kafka_controller_jolokia_access_control_file_src_path) | replace('\n', '') | replace(' ', '') }}

policy={{ lookup('file', kafka_controller_jolokia_access_control_file_src_path) }}

Copilot · 2025-08-08T14:14:30Z

playbooks/ZKtoKraftMigration.yml

 - import_playbook: kafka_controller.yml
  tags: migrate_to_dual_write

+#todo:should check if we can hit the jolokia endppint or not, if no the fail and print the error message, continue on yes


There's a typo in 'endppint' which should be 'endpoint'.

Suggested change

#todo:should check if we can hit the jolokia endppint or not, if no the fail and print the error message, continue on yes

#todo:should check if we can hit the jolokia endpoint or not, if no then fail and print the error message, continue on yes

Copilot · 2025-08-08T14:14:30Z

playbooks/ZKtoKraftMigration.yml

+
+    - name: Backup Current Jolokia Access Control XML (if exists)
+      copy:
+        src: "{{ kafka_controller_jolokia_access_control_dest_path }}"


The variable 'kafka_controller_jolokia_access_control_dest_path' is used but not defined in the visible code. This will cause the task to fail.

Copilot · 2025-08-08T14:14:30Z

playbooks/ZKtoKraftMigration.yml

+
+    - name: Deploy Temporary Migration-Specific Jolokia Access Control XML
+      template:
+        src: jolokia_migration_temp.xml.j2


The template 'jolokia_migration_temp.xml.j2' is referenced but this file is not included in the PR changes, which will cause the template task to fail.

Copilot · 2025-08-08T14:14:30Z

molecule/zookeeper-digest-rhel/molecule.yml

+        kafka_controller_jolokia_enabled: true
+        kafka_controller_jolokia_access_control_enabled: true
+        kafka_controller_jolokia_access_control_custom_file_enabled: true
+        kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"


The path references 'jolokia_migration_temp.xml.j2' which is not included in the PR changes and likely doesn't exist, causing test failures.

Suggested change

kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"

# kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"

Copilot · 2025-08-08T14:14:31Z

molecule/mtls-debian12/molecule.yml

+        jolokia_access_control_enabled: true
+        kafka_controller_jolokia_access_control_enabled: true
+        kafka_controller_jolokia_access_control_custom_file_enabled: true
+        kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"


The path references 'jolokia_migration_temp.xml.j2' which is not included in the PR changes and likely doesn't exist, causing test failures.

Suggested change

kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"

kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_access_control.xml.j2"

rrbadiani · 2025-08-18T16:12:18Z

roles/variables/defaults/main.yml

+kafka_controller_jolokia_access_control_enabled: "{{ jolokia_access_control_enabled }}"
+
+### Boolean to use custom Jolokia access control file for Kafka Controller. Inherits from global setting
+kafka_controller_jolokia_access_control_custom_file_enabled: "{{ jolokia_access_control_custom_file_enabled }}"


why do we have this variable for kraft specifically and not other components ?
we can take a call but I think it should either be in all components or not for any component specific.

Having only one var and not for each component will make it easier to handle. So I think we can remove this kafka controller prefix and use it in all components

rrbadiani · 2025-08-18T16:16:04Z

roles/kafka_controller/templates/jolokia_access_control_default.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  Jolokia Access Control Policy for ANSIENG-4748


we will be writing this file in user env so no need to write internal ticket number in this file

rrbadiani · 2025-08-18T16:19:48Z

roles/common/tasks/config_validations.yml

+    - "'kafka_controller' in group_names"
+    - kafka_controller_jolokia_enabled|bool
+    - kafka_controller_jolokia_access_control_enabled|bool
+    - kafka_controller_jolokia_access_control_custom_file_enabled is not defined or kafka_controller_jolokia_access_control_custom_file_enabled is none


we have this var defined in defaults so it should never be not defined. So this can be removed.
Also we are we setting default null as default so is this kafka_controller_jolokia_access_control_custom_file_enabled is none true for kafka_controller_jolokia_access_control_custom_file_enabled: null ?

rrbadiani · 2025-08-18T16:25:17Z

roles/common/tasks/config_validations.yml

+
+- name: Set Default Jolokia Access Control File Path - Kafka Controller
+  set_fact:
+    kafka_controller_jolokia_access_control_file_src_path: "roles/kafka_controller/templates/jolokia_access_control_default.xml"


this is not a validation so we can move it to roles/variables/tasks

rrbadiani · 2025-08-18T16:27:13Z

playbooks/ZKtoKraftMigration.yml

+    - name: Test Jolokia Migration Endpoint Access (Basic Auth)
+      uri:
+        url: "{{ 'https' if kafka_controller_jolokia_ssl_enabled|bool else 'http' }}://localhost:{{kafka_controller_jolokia_port}}/jolokia/read/kafka.controller:type=KafkaController,name=ZkMigrationState"
+        validate_certs: false


we should combine this and no auth into a single task

rrbadiani · 2025-08-18T16:31:45Z

playbooks/ZKtoKraftMigration.yml

+
+    - name: Success - Jolokia Endpoint is Accessible
+      debug:
+        msg: |


Can you please attach the result of curl request to jolokia endpoint when the access control.xml file doesnt have the mbean for allowing this request

"json": {"error": "java.lang.Exception : Reading attribute Verbose is forbidden for MBean kafka.controller:name=ZkMigrationState,type=KafkaController", "redirected": false, "status": 403, "transfer_encoding": "chunked", "url": "http://127.0.0.1:7777/jolokia/read/kafka.controller:name=ZkMigrationState,type=KafkaController"}

rrbadiani · 2025-08-18T16:34:35Z

roles/variables/defaults/main.yml

+kafka_controller_jolokia_access_control_custom_file_enabled: "{{ jolokia_access_control_custom_file_enabled }}"
+
+### Full path on Ansible Controller to custom Jolokia access control XML file for Kafka Controller. Inherits from global setting
+kafka_controller_jolokia_access_control_file_src_path: "{{ jolokia_access_control_file_src_path }}"


where is the task to move this file to cp node from ansible controller node ?

rrbadiani · 2025-08-18T16:36:32Z

molecule/mtls-debian12/molecule.yml

        jolokia_password: pass

+        # Enable Jolokia access control for security
+        jolokia_access_control_enabled: true


this and above test both are validating same case but we dont have any case for access control enabled false

it's by default false only, so we need to check for jolokia_access_control_custom_file_enabled true and false, which i have added tc in pr - https://github.com/confluentinc/cp-ansible/pull/2197/files

…ath at correct place

hrishikj added 7 commits August 6, 2025 00:38

temp jolokia changes

8503cf4

tasks for config valid

c2bd017

adding in molecule test to verify jolokia tasks

39e203a

adding in molecule test to verify jolokia tasks

3f8ab2a

test update for zookeeper to kraft

aca2561

jolokia config change

c0fe247

correcting success msg

6275979

Copilot AI review requested due to automatic review settings August 8, 2025 14:13

hrishikj requested a review from a team as a code owner August 8, 2025 14:13

Copilot AI reviewed Aug 8, 2025

View reviewed changes

hrishikj added 10 commits August 11, 2025 16:19

adding in tc

efe2789

updated xml file to block url

2be628e

changes for validation logic

12b0318

changes for xml

a5eaa68

changes for xml

9438256

changes for xml

9cacc08

policy changed

49b2ad1

policy changed

c7c395d

correcting path

5adf206

correcting the policy embedding

175cb37

rrbadiani reviewed Aug 18, 2025

View reviewed changes

hrishikj added 2 commits August 19, 2025 16:26

changes for introducing copy file tasks and setting jolokia default p…

e247a01

…ath at correct place

changes for introducing copy file tasks and setting jolokia default p…

f0c38e9

…ath at correct place

changes for naming convention kafka controller jolokia vars

2faea62

hrishikj closed this Nov 2, 2025

	policy={{ lookup('file', kafka_controller_jolokia_access_control_file_src_path) \| replace('\n', '') \| replace(' ', '') }}
	policy={{ lookup('file', kafka_controller_jolokia_access_control_file_src_path) }}

	#todo:should check if we can hit the jolokia endppint or not, if no the fail and print the error message, continue on yes
	#todo:should check if we can hit the jolokia endpoint or not, if no then fail and print the error message, continue on yes

	kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"
	# kafka_controller_jolokia_access_control_file_src_path: "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/roles/kafka_controller/templates/jolokia_migration_temp.xml.j2"

Ansieng 4748 jolokia ep #2194

Ansieng 4748 jolokia ep #2194

Uh oh!

Conversation

hrishikj commented Aug 8, 2025

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants