From 053a088339bb61c97b518ae0d1df7c1c82cfec28 Mon Sep 17 00:00:00 2001 From: Tobias Schnabel Date: Fri, 27 Mar 2026 18:28:59 +0100 Subject: [PATCH 1/6] feat: skip topic name compliance check for Kafka Streams internal topics Kafka Streams changelog and repartition topics use {applicationId}-{name}-(changelog|repartition) naming which doesn't follow the serviceName.dataIdentifier convention. Allow these through. --- .../gitops_values_validation.py | 10 ++++- .../gitops_data/app1/service2/Chart.yaml | 6 +++ ...ues-dev-topic_kafka_streams_changelog.yaml | 18 +++++++++ ...s-dev-topic_kafka_streams_repartition.yaml | 18 +++++++++ ...ues-dev-topic_with_wrong_service_name.yaml | 18 +++++++++ tests/test_gitops_values_validation.py | 39 +++++++++++++++++++ 6 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 tests/test_data/gitops_data/app1/service2/Chart.yaml create mode 100644 tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml create mode 100644 tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml create mode 100644 tests/test_data/gitops_data/app1/service2/values-dev-topic_with_wrong_service_name.yaml diff --git a/kp_pre_commit_hooks/gitops_values_validation.py b/kp_pre_commit_hooks/gitops_values_validation.py index f4ef2b2..51e1105 100755 --- a/kp_pre_commit_hooks/gitops_values_validation.py +++ b/kp_pre_commit_hooks/gitops_values_validation.py @@ -45,6 +45,8 @@ TOPIC_NAME_REGEXP = re.compile( r"^(private\.)?(?P[a-z][a-z0-9-]*)\.[a-z][a-z0-9-]*(-[0-9]+)?(\.[a-z0-9]+)?$" ) +# Kafka Streams internal topics: {applicationId}-{name}-(changelog|repartition) +KAFKA_STREAMS_INTERNAL_TOPIC_REGEXP = re.compile(r"^[a-z][a-z0-9-]*-(changelog|repartition)$") TWINGATE_DOC_URL = "https://kpler.atlassian.net/wiki/spaces/KSD/pages/243562083/Install+and+configure+the+Twingate+VPN+client" @@ -548,10 +550,14 @@ def validate_service_keys_match_service_folder(self, value, schema): ) def validate_topic_name_compliance(self, value, schema): + topic_name = str(value) + # Kafka Streams internal topics (changelog, repartition) don't follow the serviceName.dataIdentifier convention + if KAFKA_STREAMS_INTERNAL_TOPIC_REGEXP.match(topic_name): + return service_name = self._get_current_service_name() - match = TOPIC_NAME_REGEXP.match(str(value)) + match = TOPIC_NAME_REGEXP.match(topic_name) if match and match["serviceName"] not in (service_name, self.service_instance_config.service_group): - yield ValidationError(f"topicName '{value}' it not compliant, it should contain the service name '{service_name}'") + yield ValidationError(f"topicName '{topic_name}' it not compliant, it should contain the service name '{service_name}'") def validate_max_local_topic_bytes_compliance(self, value, schema): diff --git a/tests/test_data/gitops_data/app1/service2/Chart.yaml b/tests/test_data/gitops_data/app1/service2/Chart.yaml new file mode 100644 index 0000000..15ca3c7 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service2/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: service2 +version: 0.1.0 +dependencies: + - name: platform-managed-chart + version: 0.1.223-pr313 diff --git a/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml b/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml new file mode 100644 index 0000000..34ca4da --- /dev/null +++ b/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: Kafka Streams changelog internal topic +platform-managed-chart: + serviceName: service2 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: service2-changelog diff --git a/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml b/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml new file mode 100644 index 0000000..d46b88e --- /dev/null +++ b/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: Kafka Streams repartition internal topic +platform-managed-chart: + serviceName: service2 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: myapp-repartition diff --git a/tests/test_data/gitops_data/app1/service2/values-dev-topic_with_wrong_service_name.yaml b/tests/test_data/gitops_data/app1/service2/values-dev-topic_with_wrong_service_name.yaml new file mode 100644 index 0000000..3d1f7fe --- /dev/null +++ b/tests/test_data/gitops_data/app1/service2/values-dev-topic_with_wrong_service_name.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: Topic with wrong service name +platform-managed-chart: + serviceName: service2 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: other-service.testtopic diff --git a/tests/test_gitops_values_validation.py b/tests/test_gitops_values_validation.py index 4630d50..0177588 100644 --- a/tests/test_gitops_values_validation.py +++ b/tests/test_gitops_values_validation.py @@ -93,3 +93,42 @@ def test_env_specific_chart_version_is_used_for_validation( # THEN - Should use the base version from Chart.yaml (no Chart-prod.yaml exists) assert chart_version_prod == "0.1.157-pr195", f"Expected prod chart version '0.1.157-pr195', got '{chart_version_prod}'" + + +def test_kafka_streams_changelog_topic_is_accepted( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service2/values-dev-topic_kafka_streams_changelog.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert len(errors) == 0, f"Kafka Streams changelog topics should be accepted, got: {errors}" + + +def test_kafka_streams_repartition_topic_is_accepted( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service2/values-dev-topic_kafka_streams_repartition.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert len(errors) == 0, f"Kafka Streams repartition topics should be accepted, got: {errors}" + + +def test_topic_with_wrong_service_name_is_rejected( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service2/values-dev-topic_with_wrong_service_name.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert any("it not compliant" in e.message for e in errors), f"Expected topic name compliance error, got: {errors}" From fb2d953adc84336bcddf92b428c139b6ee55a76b Mon Sep 17 00:00:00 2001 From: Yann Rouillard Date: Tue, 31 Mar 2026 16:23:51 +0200 Subject: [PATCH 2/6] feat: validate internal topic name starts with serviceName-instanceName prefix Prevent copy-paste mistakes where an internal topic is reused across service instances by checking that changelog/repartition topics start with {serviceName}-{instanceName}-, matching the Kafka Streams applicationId convention. --- .../gitops_values_validation.py | 9 +++++++++ .../gitops_data/app1/service3/Chart.yaml | 6 ++++++ .../app1/service3/values-dev-changelog.yaml | 18 ++++++++++++++++++ .../app1/service3/values-dev-repartition.yaml | 18 ++++++++++++++++++ .../service3/values-dev-wrong-prefix.yaml | 18 ++++++++++++++++++ tests/test_gitops_values_validation.py | 19 +++++++++++++++++-- 6 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 tests/test_data/gitops_data/app1/service3/Chart.yaml create mode 100644 tests/test_data/gitops_data/app1/service3/values-dev-changelog.yaml create mode 100644 tests/test_data/gitops_data/app1/service3/values-dev-repartition.yaml create mode 100644 tests/test_data/gitops_data/app1/service3/values-dev-wrong-prefix.yaml diff --git a/kp_pre_commit_hooks/gitops_values_validation.py b/kp_pre_commit_hooks/gitops_values_validation.py index 51e1105..c8b47e0 100755 --- a/kp_pre_commit_hooks/gitops_values_validation.py +++ b/kp_pre_commit_hooks/gitops_values_validation.py @@ -552,7 +552,16 @@ def validate_service_keys_match_service_folder(self, value, schema): def validate_topic_name_compliance(self, value, schema): topic_name = str(value) # Kafka Streams internal topics (changelog, repartition) don't follow the serviceName.dataIdentifier convention + # but must start with {serviceName}-{instanceName}- to prevent copy-paste mistakes across instances if KAFKA_STREAMS_INTERNAL_TOPIC_REGEXP.match(topic_name): + service_name = self._get_current_service_name() + instance = self.service_instance_config.instance + expected_prefix = f"{service_name}-{instance}-" + if not topic_name.startswith(expected_prefix): + yield ValidationError( + f"Internal topic '{topic_name}' is not compliant, " + f"it must start with '{expected_prefix}' (serviceName-instanceName)" + ) return service_name = self._get_current_service_name() match = TOPIC_NAME_REGEXP.match(topic_name) diff --git a/tests/test_data/gitops_data/app1/service3/Chart.yaml b/tests/test_data/gitops_data/app1/service3/Chart.yaml new file mode 100644 index 0000000..467c1d4 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service3/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: service3 +version: 0.1.0 +dependencies: + - name: platform-managed-chart + version: 0.1.223-pr313 diff --git a/tests/test_data/gitops_data/app1/service3/values-dev-changelog.yaml b/tests/test_data/gitops_data/app1/service3/values-dev-changelog.yaml new file mode 100644 index 0000000..5371630 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service3/values-dev-changelog.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: valid Kafka Streams changelog internal topic (correct serviceName-instanceName prefix) +platform-managed-chart: + serviceName: service3 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: service3-changelog-mystore-changelog diff --git a/tests/test_data/gitops_data/app1/service3/values-dev-repartition.yaml b/tests/test_data/gitops_data/app1/service3/values-dev-repartition.yaml new file mode 100644 index 0000000..88043d8 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service3/values-dev-repartition.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: valid Kafka Streams repartition internal topic (correct serviceName-instanceName prefix) +platform-managed-chart: + serviceName: service3 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: service3-repartition-mystate-repartition diff --git a/tests/test_data/gitops_data/app1/service3/values-dev-wrong-prefix.yaml b/tests/test_data/gitops_data/app1/service3/values-dev-wrong-prefix.yaml new file mode 100644 index 0000000..93e1c6e --- /dev/null +++ b/tests/test_data/gitops_data/app1/service3/values-dev-wrong-prefix.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: Kafka Streams internal topic with wrong serviceName-instanceName prefix +platform-managed-chart: + serviceName: service3 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: service3-other-instance-store-changelog diff --git a/tests/test_gitops_values_validation.py b/tests/test_gitops_values_validation.py index 0177588..fa4635f 100644 --- a/tests/test_gitops_values_validation.py +++ b/tests/test_gitops_values_validation.py @@ -99,7 +99,7 @@ def test_kafka_streams_changelog_topic_is_accepted( create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], ) -> None: # GIVEN - validator = create_validator_for_test_file("app1/service2/values-dev-topic_kafka_streams_changelog.yaml") + validator = create_validator_for_test_file("app1/service3/values-dev-changelog.yaml") # WHEN errors = validator.validate_configuration() @@ -112,7 +112,7 @@ def test_kafka_streams_repartition_topic_is_accepted( create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], ) -> None: # GIVEN - validator = create_validator_for_test_file("app1/service2/values-dev-topic_kafka_streams_repartition.yaml") + validator = create_validator_for_test_file("app1/service3/values-dev-repartition.yaml") # WHEN errors = validator.validate_configuration() @@ -121,6 +121,21 @@ def test_kafka_streams_repartition_topic_is_accepted( assert len(errors) == 0, f"Kafka Streams repartition topics should be accepted, got: {errors}" +def test_kafka_streams_internal_topic_with_wrong_prefix_is_rejected( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service3/values-dev-wrong-prefix.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert any("must start with 'service3-wrong-prefix-'" in e.message for e in errors), ( + f"Expected internal topic prefix error, got: {errors}" + ) + + def test_topic_with_wrong_service_name_is_rejected( create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], ) -> None: From f47f80187cb71f383a34ff63d3205e2ef731cd27 Mon Sep 17 00:00:00 2001 From: Yann Rouillard Date: Tue, 31 Mar 2026 16:36:38 +0200 Subject: [PATCH 3/6] chore: update service1 test data to use recent stable chart versions Replace PR version 0.1.157-pr195 with stable 0.1.224 (base) and 0.1.225 (dev override), which are the latest stable releases and include the maxLocalTopicBytesCompliance custom validator support. This also properly demonstrates the env-specific chart loading feature with two distinct stable versions. --- tests/test_data/gitops_data/app1/service1/Chart-dev.yaml | 2 +- tests/test_data/gitops_data/app1/service1/Chart.yaml | 2 +- ...ues-dev-topic_with_authorized_max_local_topic_bytes.yaml | 2 +- ...c_with_max_local_topic_bytes_above_authorized_value.yaml | 2 +- .../values-dev-topic_with_no_max_local_topic_bytes_set.yaml | 2 +- ...s-dev-topic_with_unauthorized_max_local_topic_bytes.yaml | 2 +- ...authorized_max_local_topic_bytes_not_on_current_env.yaml | 2 +- tests/test_gitops_values_validation.py | 6 +++--- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_data/gitops_data/app1/service1/Chart-dev.yaml b/tests/test_data/gitops_data/app1/service1/Chart-dev.yaml index c585dc8..61f7d10 100644 --- a/tests/test_data/gitops_data/app1/service1/Chart-dev.yaml +++ b/tests/test_data/gitops_data/app1/service1/Chart-dev.yaml @@ -1,3 +1,3 @@ dependencies: - name: platform-managed-chart - version: 0.1.159 + version: 0.1.225 diff --git a/tests/test_data/gitops_data/app1/service1/Chart.yaml b/tests/test_data/gitops_data/app1/service1/Chart.yaml index 11d0afa..c93ef7d 100644 --- a/tests/test_data/gitops_data/app1/service1/Chart.yaml +++ b/tests/test_data/gitops_data/app1/service1/Chart.yaml @@ -3,4 +3,4 @@ name: service1 version: 0.1.0 dependencies: - name: platform-managed-chart - version: 0.1.157-pr195 + version: 0.1.224 diff --git a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_authorized_max_local_topic_bytes.yaml b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_authorized_max_local_topic_bytes.yaml index 6ead7e9..9442b89 100644 --- a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_authorized_max_local_topic_bytes.yaml +++ b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_authorized_max_local_topic_bytes.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.157-pr195/schema-platform-managed-chart.json +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.225/schema-platform-managed-chart.json # Test file: Whitelisted topic with valid size platform-managed-chart: serviceName: service1 diff --git a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_max_local_topic_bytes_above_authorized_value.yaml b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_max_local_topic_bytes_above_authorized_value.yaml index ed60d51..85ad125 100644 --- a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_max_local_topic_bytes_above_authorized_value.yaml +++ b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_max_local_topic_bytes_above_authorized_value.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.157-pr195/schema-platform-managed-chart.json +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.225/schema-platform-managed-chart.json # Test file: Whitelisted topic exceeding its specific limit platform-managed-chart: serviceName: service1 diff --git a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_no_max_local_topic_bytes_set.yaml b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_no_max_local_topic_bytes_set.yaml index a34b0cf..1c1a7e6 100644 --- a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_no_max_local_topic_bytes_set.yaml +++ b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_no_max_local_topic_bytes_set.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.157-pr195/schema-platform-managed-chart.json +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.225/schema-platform-managed-chart.json # Test file: Whitelisted topic with valid size platform-managed-chart: serviceName: service1 diff --git a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_unauthorized_max_local_topic_bytes.yaml b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_unauthorized_max_local_topic_bytes.yaml index d897ac8..4d9bb56 100644 --- a/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_unauthorized_max_local_topic_bytes.yaml +++ b/tests/test_data/gitops_data/app1/service1/values-dev-topic_with_unauthorized_max_local_topic_bytes.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.157-pr195/schema-platform-managed-chart.json +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.225/schema-platform-managed-chart.json platform-managed-chart: serviceName: service1 teamOwner: team1 diff --git a/tests/test_data/gitops_data/app1/service1/values-prod-topic_with_authorized_max_local_topic_bytes_not_on_current_env.yaml b/tests/test_data/gitops_data/app1/service1/values-prod-topic_with_authorized_max_local_topic_bytes_not_on_current_env.yaml index 11bfe5d..b56cd02 100644 --- a/tests/test_data/gitops_data/app1/service1/values-prod-topic_with_authorized_max_local_topic_bytes_not_on_current_env.yaml +++ b/tests/test_data/gitops_data/app1/service1/values-prod-topic_with_authorized_max_local_topic_bytes_not_on_current_env.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.157-pr195/schema-platform-managed-chart.json +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.224/schema-platform-managed-chart.json # Test file: Whitelisted topic with valid size platform-managed-chart: serviceName: service1 diff --git a/tests/test_gitops_values_validation.py b/tests/test_gitops_values_validation.py index fa4635f..a066153 100644 --- a/tests/test_gitops_values_validation.py +++ b/tests/test_gitops_values_validation.py @@ -82,8 +82,8 @@ def test_env_specific_chart_version_is_used_for_validation( # WHEN - We check the chart version used for validation chart_version_dev = validator_dev.service_instance_config.helm_chart.platform_managed_chart_version - # THEN - Should use the dev-specific version from Chart-dev.yaml (same as base for now) - assert chart_version_dev == "0.1.157-pr195", f"Expected dev chart version '0.1.157-pr195', got '{chart_version_dev}'" + # THEN - Should use the dev-specific version from Chart-dev.yaml + assert chart_version_dev == "0.1.225", f"Expected dev chart version '0.1.225', got '{chart_version_dev}'" # GIVEN - A prod environment configuration validator_prod = create_validator_for_test_file("app1/service1/values-prod-topic_with_authorized_max_local_topic_bytes_not_on_current_env.yaml") @@ -92,7 +92,7 @@ def test_env_specific_chart_version_is_used_for_validation( chart_version_prod = validator_prod.service_instance_config.helm_chart.platform_managed_chart_version # THEN - Should use the base version from Chart.yaml (no Chart-prod.yaml exists) - assert chart_version_prod == "0.1.157-pr195", f"Expected prod chart version '0.1.157-pr195', got '{chart_version_prod}'" + assert chart_version_prod == "0.1.224", f"Expected prod chart version '0.1.224', got '{chart_version_prod}'" def test_kafka_streams_changelog_topic_is_accepted( From 554839aa1da5bd2fdd437466a3490373ed319834 Mon Sep 17 00:00:00 2001 From: Yann Rouillard Date: Tue, 31 Mar 2026 16:46:02 +0200 Subject: [PATCH 4/6] fix: align TOPIC_NAME_REGEXP with chart schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Version suffix: (-[0-9]+)? → (-v[0-9]+)? to require the v prefix (e.g. -v1) - Instance suffix: (\.[a-z0-9]+)? → (\.[a-z0-9-]+)? to allow hyphens (e.g. .mirrored-prod) --- kp_pre_commit_hooks/gitops_values_validation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kp_pre_commit_hooks/gitops_values_validation.py b/kp_pre_commit_hooks/gitops_values_validation.py index c8b47e0..e5c8cb2 100755 --- a/kp_pre_commit_hooks/gitops_values_validation.py +++ b/kp_pre_commit_hooks/gitops_values_validation.py @@ -43,7 +43,7 @@ # Validate topic names follow pattern: (private.)?serviceName.topic(-version)?(.suffix)? TOPIC_NAME_REGEXP = re.compile( - r"^(private\.)?(?P[a-z][a-z0-9-]*)\.[a-z][a-z0-9-]*(-[0-9]+)?(\.[a-z0-9]+)?$" + r"^(private\.)?(?P[a-z][a-z0-9-]*)\.[a-z][a-z0-9-]*(-v[0-9]+)?(\.[a-z0-9-]+)?$" ) # Kafka Streams internal topics: {applicationId}-{name}-(changelog|repartition) KAFKA_STREAMS_INTERNAL_TOPIC_REGEXP = re.compile(r"^[a-z][a-z0-9-]*-(changelog|repartition)$") From c1f7005e5778244b39bf45ec5f27f3fede34bec6 Mon Sep 17 00:00:00 2001 From: Yann Rouillard Date: Tue, 31 Mar 2026 16:54:28 +0200 Subject: [PATCH 5/6] =?UTF-8?q?test:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20dead=20data,=20missing=20coverage,=20regexp=20comme?= =?UTF-8?q?nt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delete unused service2 Kafka Streams test data files (leftover from before the prefix check was introduced; would have failed if tested) - Add test + data for internal topic from a foreign service (wrong serviceName prefix, not just wrong instance) - Add tests + data pinning the TOPIC_NAME_REGEXP changes: -vN suffix and hyphenated suffix are now accepted - Clarify TOPIC_NAME_REGEXP comment: format validation is the schema's responsibility; the regexp only enforces service name ownership --- .../gitops_values_validation.py | 4 +- .../values-dev-hyphenated-suffix.yaml | 18 ++++++++ .../values-dev-versioned-topic.yaml} | 8 ++-- .../values-dev-foreign-service.yaml} | 6 +-- tests/test_gitops_values_validation.py | 41 +++++++++++++++++++ 5 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 tests/test_data/gitops_data/app1/service1/values-dev-hyphenated-suffix.yaml rename tests/test_data/gitops_data/app1/{service2/values-dev-topic_kafka_streams_repartition.yaml => service1/values-dev-versioned-topic.yaml} (63%) rename tests/test_data/gitops_data/app1/{service2/values-dev-topic_kafka_streams_changelog.yaml => service3/values-dev-foreign-service.yaml} (67%) diff --git a/kp_pre_commit_hooks/gitops_values_validation.py b/kp_pre_commit_hooks/gitops_values_validation.py index e5c8cb2..9ee798c 100755 --- a/kp_pre_commit_hooks/gitops_values_validation.py +++ b/kp_pre_commit_hooks/gitops_values_validation.py @@ -41,7 +41,9 @@ re.MULTILINE ) -# Validate topic names follow pattern: (private.)?serviceName.topic(-version)?(.suffix)? +# Matches standard topic names to extract the serviceName for ownership checks. +# Format validation (e.g. enforcing -vN over -N) is the chart schema's responsibility; +# this regexp only needs to match what the schema accepts so the service name check fires correctly. TOPIC_NAME_REGEXP = re.compile( r"^(private\.)?(?P[a-z][a-z0-9-]*)\.[a-z][a-z0-9-]*(-v[0-9]+)?(\.[a-z0-9-]+)?$" ) diff --git a/tests/test_data/gitops_data/app1/service1/values-dev-hyphenated-suffix.yaml b/tests/test_data/gitops_data/app1/service1/values-dev-hyphenated-suffix.yaml new file mode 100644 index 0000000..dba4134 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service1/values-dev-hyphenated-suffix.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.225/schema-platform-managed-chart.json +# Test file: Standard topic with hyphenated suffix (valid per chart schema) +platform-managed-chart: + serviceName: service1 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicName: service1.testtopic.sub-part diff --git a/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml b/tests/test_data/gitops_data/app1/service1/values-dev-versioned-topic.yaml similarity index 63% rename from tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml rename to tests/test_data/gitops_data/app1/service1/values-dev-versioned-topic.yaml index d46b88e..32452cf 100644 --- a/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_repartition.yaml +++ b/tests/test_data/gitops_data/app1/service1/values-dev-versioned-topic.yaml @@ -1,7 +1,7 @@ -# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json -# Test file: Kafka Streams repartition internal topic +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.225/schema-platform-managed-chart.json +# Test file: Standard topic with -vN version suffix (valid per chart schema) platform-managed-chart: - serviceName: service2 + serviceName: service1 teamOwner: team1 env: dev workloadType: stream @@ -15,4 +15,4 @@ platform-managed-chart: managedResources: mskTopics: output: - topicName: myapp-repartition + topicName: service1.testtopic-v2 diff --git a/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml b/tests/test_data/gitops_data/app1/service3/values-dev-foreign-service.yaml similarity index 67% rename from tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml rename to tests/test_data/gitops_data/app1/service3/values-dev-foreign-service.yaml index 34ca4da..fca498a 100644 --- a/tests/test_data/gitops_data/app1/service2/values-dev-topic_kafka_streams_changelog.yaml +++ b/tests/test_data/gitops_data/app1/service3/values-dev-foreign-service.yaml @@ -1,7 +1,7 @@ # yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json -# Test file: Kafka Streams changelog internal topic +# Test file: Kafka Streams internal topic from a foreign service (wrong serviceName prefix) platform-managed-chart: - serviceName: service2 + serviceName: service3 teamOwner: team1 env: dev workloadType: stream @@ -15,4 +15,4 @@ platform-managed-chart: managedResources: mskTopics: output: - topicName: service2-changelog + topicName: otherservice-foreign-service-mystore-changelog diff --git a/tests/test_gitops_values_validation.py b/tests/test_gitops_values_validation.py index a066153..4aa142a 100644 --- a/tests/test_gitops_values_validation.py +++ b/tests/test_gitops_values_validation.py @@ -147,3 +147,44 @@ def test_topic_with_wrong_service_name_is_rejected( # THEN assert any("it not compliant" in e.message for e in errors), f"Expected topic name compliance error, got: {errors}" + + +def test_kafka_streams_internal_topic_from_foreign_service_is_rejected( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service3/values-dev-foreign-service.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert any("must start with 'service3-foreign-service-'" in e.message for e in errors), ( + f"Expected internal topic prefix error for foreign service topic, got: {errors}" + ) + + +def test_versioned_topic_with_v_prefix_is_accepted( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service1/values-dev-versioned-topic.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert len(errors) == 0, f"Topic with -vN version suffix should be accepted, got: {errors}" + + +def test_topic_with_hyphenated_suffix_is_accepted( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service1/values-dev-hyphenated-suffix.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert len(errors) == 0, f"Topic with hyphenated suffix should be accepted, got: {errors}" From ea0b4b6df3d992f63fb89d124da8e36a447e0391 Mon Sep 17 00:00:00 2001 From: Yann Rouillard Date: Sun, 12 Apr 2026 20:26:36 +0200 Subject: [PATCH 6/6] feat: validate topicNameTemplate with context variable resolution Extend validate_topic_name_compliance to handle topicNameTemplate fields. When validating a template, known context variables (serviceName, instanceName, serviceInstanceName, env) are resolved to their actual values before checking the topic name structure. This enables using $.context.serviceInstanceName in changelog/repartition topics while still verifying the serviceName-instanceName prefix is correct. --- .../gitops_values_validation.py | 34 ++++++++++++++++++- .../app1/service3/values-dev-tpl-valid.yaml | 18 ++++++++++ .../app1/service3/values-dev-tpl-wrong.yaml | 18 ++++++++++ tests/test_gitops_values_validation.py | 28 +++++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 tests/test_data/gitops_data/app1/service3/values-dev-tpl-valid.yaml create mode 100644 tests/test_data/gitops_data/app1/service3/values-dev-tpl-wrong.yaml diff --git a/kp_pre_commit_hooks/gitops_values_validation.py b/kp_pre_commit_hooks/gitops_values_validation.py index 9ee798c..ca53725 100755 --- a/kp_pre_commit_hooks/gitops_values_validation.py +++ b/kp_pre_commit_hooks/gitops_values_validation.py @@ -50,6 +50,9 @@ # Kafka Streams internal topics: {applicationId}-{name}-(changelog|repartition) KAFKA_STREAMS_INTERNAL_TOPIC_REGEXP = re.compile(r"^[a-z][a-z0-9-]*-(changelog|repartition)$") +# Matches any Go template block {{ ... }} +TEMPLATE_BLOCK_REGEXP = re.compile(r"\{\{[^}]*\}\}") + TWINGATE_DOC_URL = "https://kpler.atlassian.net/wiki/spaces/KSD/pages/243562083/Install+and+configure+the+Twingate+VPN+client" # Environment variables that should not be overridden @@ -551,8 +554,37 @@ def validate_service_keys_match_service_folder(self, value, schema): f" Must be either '{folder_name}' or '{folder_name}-'" ) + def _resolve_topic_name_template(self, template: str) -> str: + """Replace known context variables with actual values, others with a placeholder. + + This allows validating the structure of a topicNameTemplate as if it were + a static topicName, by substituting the context variables the chart would + inject at render time. + """ + service_name = self._get_current_service_name() + instance = self.service_instance_config.instance + + replacements = { + "$.context.serviceInstanceName": f"{service_name}-{instance}", + "$.context.serviceName": service_name, + "$.context.instanceName": instance, + "$.context.env": self.service_instance_config.env, + } + + result = template + for var, val in replacements.items(): + result = result.replace(f"{{{{ {var} }}}}", val) + + # Replace remaining unknown template blocks with a generic placeholder + result = TEMPLATE_BLOCK_REGEXP.sub("x", result) + return result + def validate_topic_name_compliance(self, value, schema): - topic_name = str(value) + current_path = self._get_current_path() + is_template = current_path and current_path[-1] == "topicNameTemplate" + + topic_name = self._resolve_topic_name_template(str(value)) if is_template else str(value) + # Kafka Streams internal topics (changelog, repartition) don't follow the serviceName.dataIdentifier convention # but must start with {serviceName}-{instanceName}- to prevent copy-paste mistakes across instances if KAFKA_STREAMS_INTERNAL_TOPIC_REGEXP.match(topic_name): diff --git a/tests/test_data/gitops_data/app1/service3/values-dev-tpl-valid.yaml b/tests/test_data/gitops_data/app1/service3/values-dev-tpl-valid.yaml new file mode 100644 index 0000000..f202376 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service3/values-dev-tpl-valid.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: valid Kafka Streams changelog internal topic using topicNameTemplate +platform-managed-chart: + serviceName: service3 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicNameTemplate: "{{ $.context.serviceName }}-{{ $.context.instanceName }}-mystore-changelog" diff --git a/tests/test_data/gitops_data/app1/service3/values-dev-tpl-wrong.yaml b/tests/test_data/gitops_data/app1/service3/values-dev-tpl-wrong.yaml new file mode 100644 index 0000000..5bac727 --- /dev/null +++ b/tests/test_data/gitops_data/app1/service3/values-dev-tpl-wrong.yaml @@ -0,0 +1,18 @@ +# yaml-language-server: $schema=https://kp-helmchart-stable-shared-main.s3.eu-west-1.amazonaws.com/schema/platform-managed-chart/v0.1.223-pr313/schema-platform-managed-chart.json +# Test file: invalid Kafka Streams changelog topic template with wrong instance name +platform-managed-chart: + serviceName: service3 + teamOwner: team1 + env: dev + workloadType: stream + + stream: {} + + image: + repository: dev/my-repo + tag: my-tag + + managedResources: + mskTopics: + output: + topicNameTemplate: "{{ $.context.serviceName }}-other-instance-mystore-changelog" diff --git a/tests/test_gitops_values_validation.py b/tests/test_gitops_values_validation.py index 4aa142a..bc44df9 100644 --- a/tests/test_gitops_values_validation.py +++ b/tests/test_gitops_values_validation.py @@ -188,3 +188,31 @@ def test_topic_with_hyphenated_suffix_is_accepted( # THEN assert len(errors) == 0, f"Topic with hyphenated suffix should be accepted, got: {errors}" + + +def test_kafka_streams_changelog_topic_template_is_accepted( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service3/values-dev-tpl-valid.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert len(errors) == 0, f"Kafka Streams changelog topic template should be accepted, got: {errors}" + + +def test_kafka_streams_changelog_topic_template_with_wrong_prefix_is_rejected( + create_validator_for_test_file: Callable[[str], ServiceInstanceConfigValidator], +) -> None: + # GIVEN + validator = create_validator_for_test_file("app1/service3/values-dev-tpl-wrong.yaml") + + # WHEN + errors = validator.validate_configuration() + + # THEN + assert any("must start with 'service3-tpl-wrong-'" in e.message for e in errors), ( + f"Expected internal topic prefix error for template with wrong instance, got: {errors}" + )