Skip to content

feat(kafka): add TLS support via Strimzi listener configuration #2681

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.

Sign up for GitHub

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

Open
Arsolitt wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(kafka): add TLS support via Strimzi listener configuration #2681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ebd25b0
feat(kafka): add TLS schema and generated artifacts
Arsolitt May 15, 2026
2b196c3
feat(kafka): implement TLS via Strimzi listener configuration
Arsolitt May 15, 2026
42a672f
test(kafka): add TLS test coverage
Arsolitt May 15, 2026
bd49fe7
fix(kafka): decouple external listener from tls.enabled (tri-state)
Arsolitt May 19, 2026
eef13ea
chore(kafka): regenerate codegen deepcopy artifacts
Arsolitt May 19, 2026
3050b7c
fix(kafka): replace bash-ism in kafka-rd-schema bats test
Arsolitt May 19, 2026
c806691
fix(kafka): use printf in kafka-rd-schema bats test
Arsolitt May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/apps/v1alpha1/kafka/types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions api/apps/v1alpha1/kafka/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions hack/check-kafka-rd-schema.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bats
# Unit test: kafka-rd cozyrds embedded openAPISchema must declare
# tls.enabled as nullable (["boolean","null"]) so the ApplicationDefinition
# CRD validation does not reject an unset (null) value.

REPO_ROOT="$(cd "$(dirname "${BATS_TEST_FILENAME:-$0}")/.." && pwd)"
COZYRDS="$REPO_ROOT/packages/system/kafka-rd/cozyrds/kafka.yaml"

@test "kafka-rd cozyrds embedded schema has tls.enabled type as [boolean, null]" {
# The openAPISchema value is a single-line JSON string after "openAPISchema: |-"
SCHEMA_JSON="$(grep -A1 'openAPISchema: |-' "$COZYRDS" | tail -n1 | sed 's/^[[:space:]]*//')"
[ -n "$SCHEMA_JSON" ] || { echo "Could not extract openAPISchema from $COZYRDS" >&2; exit 1; }
printf '%s' "$SCHEMA_JSON" | jq -e '.properties.tls.properties.enabled.type == ["boolean", "null"]'
}
17 changes: 17 additions & 0 deletions hack/check-kafka-rd-secrets.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bats
# Unit test: kafka-rd cozyrds must reference the Strimzi-generated
# clients-ca-cert secret (not the bare clients-ca name, which does not exist).

REPO_ROOT="$(cd "$(dirname "${BATS_TEST_FILENAME:-$0}")/.." && pwd)"
COZYRDS="$REPO_ROOT/packages/system/kafka-rd/cozyrds/kafka.yaml"

@test "kafka-rd cozyrds references clients-ca-cert (Strimzi actual name)" {
grep -q "clients-ca-cert" "$COZYRDS"
}

@test "kafka-rd cozyrds does not reference bare clients-ca (wrong name)" {
if grep -qP "clients-ca(?!-)" "$COZYRDS"; then
echo "Found bare 'clients-ca' reference (missing '-cert' suffix)" >&2
exit 1
fi
}
Comment on lines +8 to +17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add cluster CA naming checks to match the same contract.

This file only guards clients-ca-cert; it should also assert cluster-ca-cert and reject bare cluster-ca to prevent partial regressions.

Suggested test additions 
 `@test` "kafka-rd cozyrds references clients-ca-cert (Strimzi actual name)" {
   grep -q "clients-ca-cert" "$COZYRDS"
 }
 
+@test "kafka-rd cozyrds references cluster-ca-cert (Strimzi actual name)" {
+  grep -q "cluster-ca-cert" "$COZYRDS"
+}
+
 `@test` "kafka-rd cozyrds does not reference bare clients-ca (wrong name)" {
   if grep -qP "clients-ca(?!-)" "$COZYRDS"; then
     echo "Found bare 'clients-ca' reference (missing '-cert' suffix)" >&2
     exit 1
   fi
 }
+
+@test "kafka-rd cozyrds does not reference bare cluster-ca (wrong name)" {
+  if grep -qP "cluster-ca(?!-)" "$COZYRDS"; then
+    echo "Found bare 'cluster-ca' reference (missing '-cert' suffix)" >&2
+    exit 1
+  fi
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@test "kafka-rd cozyrds references clients-ca-cert (Strimzi actual name)" {
grep -q "clients-ca-cert" "$COZYRDS"
}
@test "kafka-rd cozyrds does not reference bare clients-ca (wrong name)" {
if grep -qP "clients-ca(?!-)" "$COZYRDS"; then
echo "Found bare 'clients-ca' reference (missing '-cert' suffix)" >&2
exit 1
fi
}
`@test` "kafka-rd cozyrds references clients-ca-cert (Strimzi actual name)" {
grep -q "clients-ca-cert" "$COZYRDS"
}
`@test` "kafka-rd cozyrds references cluster-ca-cert (Strimzi actual name)" {
grep -q "cluster-ca-cert" "$COZYRDS"
}
`@test` "kafka-rd cozyrds does not reference bare clients-ca (wrong name)" {
if grep -qP "clients-ca(?!-)" "$COZYRDS"; then
echo "Found bare 'clients-ca' reference (missing '-cert' suffix)" >&2
exit 1
fi
}
`@test` "kafka-rd cozyrds does not reference bare cluster-ca (wrong name)" {
if grep -qP "cluster-ca(?!-)" "$COZYRDS"; then
echo "Found bare 'cluster-ca' reference (missing '-cert' suffix)" >&2
exit 1
fi
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/check-kafka-rd-secrets.bats` around lines 8 - 17, Add two tests
mirroring the clients-ca checks: create a test named like 'kafka-rd cozyrds
references cluster-ca-cert (Strimzi actual name)' that greps for
"cluster-ca-cert" in the $COZYRDS file, and a test named like 'kafka-rd cozyrds
does not reference bare cluster-ca (wrong name)' that fails if grep -qP
"cluster-ca(?!-)" finds a match; follow the same structure and exit behavior as
the existing clients-ca tests so the suite enforces presence of
"cluster-ca-cert" and rejects bare "cluster-ca".

6 changes: 6 additions & 0 deletions packages/apps/kafka/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,10 @@ PRESET_ENUM := ["nano","micro","small","medium","large","xlarge","2xlarge"]

generate:
cozyvalues-gen -m 'kafka' -v values.yaml -s values.schema.json -r README.md -g ../../../api/apps/v1alpha1/kafka/types.go
sed -i 's/Enabled bool `json:"enabled,omitempty"`/Enabled *bool `json:"enabled,omitempty"`/' ../../../api/apps/v1alpha1/kafka/types.go
jq '.properties.tls.properties.enabled.type = ["boolean", "null"]' values.schema.json > values.schema.json.tmp && mv values.schema.json.tmp values.schema.json
jq -e '.properties.tls.properties.enabled.type == ["boolean", "null"]' values.schema.json > /dev/null || (echo "ERROR: jq patch failed" && exit 1)
Comment on lines +6 to +8
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard the Go type patch with a hard assertion.

The sed mutation can silently no-op if cozyvalues-gen output formatting changes, which can reintroduce non-nullable Enabled bool while the schema patch still passes. Add an explicit post-patch check for the pointer type.

Suggested fix 
 generate:
 	cozyvalues-gen -m 'kafka' -v values.yaml -s values.schema.json -r README.md -g ../../../api/apps/v1alpha1/kafka/types.go
-	sed -i 's/Enabled bool `json:"enabled,omitempty"`/Enabled *bool `json:"enabled,omitempty"`/' ../../../api/apps/v1alpha1/kafka/types.go
+	sed -i -E 's/Enabled[[:space:]]+bool[[:space:]]+`json:"enabled,omitempty"`/Enabled *bool `json:"enabled,omitempty"`/' ../../../api/apps/v1alpha1/kafka/types.go
+	grep -Eq 'Enabled[[:space:]]+\*bool[[:space:]]+`json:"enabled,omitempty"`' ../../../api/apps/v1alpha1/kafka/types.go || (echo "ERROR: failed to patch TLS.Enabled to *bool" && exit 1)
 	jq '.properties.tls.properties.enabled.type = ["boolean", "null"]' values.schema.json > values.schema.json.tmp && mv values.schema.json.tmp values.schema.json
 	jq -e '.properties.tls.properties.enabled.type == ["boolean", "null"]' values.schema.json > /dev/null || (echo "ERROR: jq patch failed" && exit 1)
 	../../../hack/update-crd.sh
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/apps/kafka/Makefile` around lines 6 - 8, The sed patch that replaces
"Enabled bool `json:\"enabled,omitempty\"`" with "Enabled *bool
`json:\"enabled,omitempty\"`" can silently no-op; after running sed (the
existing sed command that targets types.go), add a hard assertion that verifies
the Go source now contains the pointer field (look for the exact token Enabled
*bool `json:"enabled,omitempty"` in types.go) and fail the Makefile target if
the check does not match; keep the existing jq schema changes but ensure the new
assertion runs immediately after the sed step so the build fails if the Go type
wasn't actually updated.

../../../hack/update-crd.sh

test:
helm unittest .
12 changes: 9 additions & 3 deletions packages/apps/kafka/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

### Common parameters

| Name | Description | Type | Value |
| ---------- | ------------------------------------------------ | ------ | ------- |
| `external` | Enable external access from outside the cluster. | `bool` | `false` |
| Name | Description | Type | Value |
| ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ------- |
| `external` | Enable external access from outside the cluster. | `bool` | `false` |
| `tls` | TLS configuration. Strimzi manages the cluster PKI automatically (no cert-manager is involved for this chart): the operator auto-creates `<release>-cluster-ca-cert` and `<release>-clients-ca-cert` secrets, both exposed for client trust setup. The internal TLS listener on 9093 is always on; this toggle only controls the external listener on 9094. | `object` | `{}` |
| `tls.enabled` | Enable TLS on the external listener. When unset, inherits the value of `external` (TLS is on when external access is enabled). Warning: setting this to false while external is true exposes Kafka over plaintext on a public IP via LoadBalancer. Strimzi does not provide authentication on this listener unless SCRAM, mTLS, or OAuth is separately configured. Use only in controlled networks. | `bool` | `false` |
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Document tls.enabled default as unset/null, not false.

The row describes tri-state inheritance but lists default false, which is misleading. This should reflect an unset/null default so behavior matches the chart contract.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/apps/kafka/README.md` at line 11, Update the README row for the
tls.enabled Helm value to reflect that its default is unset/null (tri-state
inheritance) rather than `false`; change the default column and the wording so
it clearly states the value is "unset/null" and explains it inherits from
`external` when not set, and ensure `tls.enabled` is referenced exactly as
written so the README aligns with the chart contract.



### Application-specific parameters
Expand Down Expand Up @@ -68,6 +70,10 @@ Presets follow a cloud-style `<series>.<size>` naming convention. Five series co

See [`docs/operations/resource-presets.md`](../../../docs/operations/resource-presets.md) for the full size matrix and the legacy-to-instance-type mapping.

### Authentication

This chart does not configure listener authentication. When TLS is enabled on the external listener, clients can connect without credentials. To require authentication, use Strimzi's `KafkaUser` resource with an appropriate `authentication` type (`tls`, `scram-sha-512`, or `oauth`) outside this chart. See the [Strimzi documentation on KafkaUser](https://strimzi.io/docs/operators/latest/overview.html#security-options_str) for details.

### topics

```yaml
Expand Down
14 changes: 12 additions & 2 deletions packages/apps/kafka/templates/dashboard-resourcemap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
{{- $tlsMap := .Values.tls | default dict -}}
{{- $tlsEnabledExplicit := hasKey $tlsMap "enabled" -}}
{{- $tlsEnabled := false -}}
{{- if $tlsEnabledExplicit -}}
{{- $tlsEnabled = index $tlsMap "enabled" -}}
{{- else -}}
{{- $tlsEnabled = .Values.external | default false -}}
{{- end -}}
{{- $showExternal := or .Values.external (and $tlsEnabledExplicit $tlsEnabled) -}}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Gate external-bootstrap RBAC only on external.

Line 9 still grants external-bootstrap access when external=false and tls.enabled=true. This reintroduces coupling and violates least-privilege for RBAC.

Proposed fix 
-{{- $tlsMap := .Values.tls | default dict -}}
-{{- $tlsEnabledExplicit := hasKey $tlsMap "enabled" -}}
-{{- $tlsEnabled := false -}}
-{{- if $tlsEnabledExplicit -}}
-  {{- $tlsEnabled = index $tlsMap "enabled" -}}
-{{- else -}}
-  {{- $tlsEnabled = .Values.external | default false -}}
-{{- end -}}
-{{- $showExternal := or .Values.external (and $tlsEnabledExplicit $tlsEnabled) -}}
+{{- $showExternal := .Values.external | default false -}}

Also applies to: 21-23

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/apps/kafka/templates/dashboard-resourcemap.yaml` at line 9, The RBAC
for external-bootstrap is being enabled when tls.enabled is true because the
template sets {{$showExternal := or .Values.external (and $tlsEnabledExplicit
$tlsEnabled)}}, so update the gating logic to require .Values.external
explicitly (remove the TLS-based OR) and apply the same change to the other
occurrences noted (the blocks around lines 21-23) so that external-bootstrap
roles/subjects are only created when .Values.external is true; search for
$showExternal and replace the expression and any conditional blocks that use it
to use .Values.external directly.

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
Expand All @@ -9,7 +18,7 @@ rules:
- services
resourceNames:
- {{ .Release.Name }}-kafka-bootstrap
{{- if .Values.external }}
{{- if $showExternal }}
- {{ .Release.Name }}-kafka-external-bootstrap
{{- end }}
verbs: ["get", "list", "watch"]
Expand All @@ -18,7 +27,8 @@ rules:
resources:
- secrets
resourceNames:
- {{ .Release.Name }}-clients-ca
- {{ .Release.Name }}-clients-ca-cert
- {{ .Release.Name }}-cluster-ca-cert
verbs: ["get", "list", "watch"]
- apiGroups:
- cozystack.io
Expand Down
26 changes: 24 additions & 2 deletions packages/apps/kafka/templates/kafka.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,25 @@
{{- /*
Tri-state TLS logic for the external listener (port 9094):
- .Values.external is the sole gate for the external LoadBalancer listener.
The listener is rendered if and only if external is true.
- tls.enabled controls TLS *on the external listener* (independent of whether
the listener exists):
- If tls.enabled is explicitly set (true or false), that value wins.
- If tls.enabled is unset (nil), TLS inherits from .Values.external
(auto-on when external is true, off otherwise).
- tls.enabled does NOT control whether the external listener is created.
Setting tls.enabled=true with external=false has no effect on listeners.
Internal TLS listener (port 9093) is always on; Strimzi manages the PKI.
*/ -}}
{{- $tlsMap := .Values.tls | default dict -}}
{{- $tlsEnabledExplicit := hasKey $tlsMap "enabled" -}}
{{- $tlsEnabled := false -}}
{{- if $tlsEnabledExplicit -}}
{{- $tlsEnabled = index $tlsMap "enabled" -}}
{{- else -}}
{{- $tlsEnabled = .Values.external | default false -}}
{{- end -}}
Comment on lines +15 to +21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Confirm schema allows null for tls.enabled:"
rg -n '"enabled".*"type":\["boolean","null"\]' packages/system/kafka-rd/cozyrds/kafka.yaml

echo
echo "Confirm template uses key-presence explicitness and renders tls from that value:"
rg -n 'hasKey \$tlsMap "enabled"|index \$tlsMap "enabled"|tls: \{\{ \$tlsEnabled \}\}' packages/apps/kafka/templates/kafka.yaml

Repository: cozystack/cozystack

Length of output: 5898

Handle tls.enabled: null as unset before rendering tls.

The schema allows enabled: null, but the current logic at line 15 only checks key presence via hasKey, not nil values. When tls.enabled: null is set, line 15 evaluates $tlsEnabledExplicit to true, then line 18 assigns the null value to $tlsEnabled, and line 47 renders tls: null—breaking the tri-state contract and producing an invalid Kafka CR.

The fix distinguishes between "key exists with a boolean" and "key exists but is null" (which should behave as unset). Capture the raw value, update the explicit check to and (hasKey $tlsMap "enabled") (ne $tlsEnabledRaw nil), and reuse it in the conditional.

Proposed fix 
 {{- $tlsMap := .Values.tls | default dict -}}
-{{- $tlsEnabledExplicit := hasKey $tlsMap "enabled" -}}
+{{- $tlsEnabledRaw := index $tlsMap "enabled" -}}
+{{- $tlsEnabledExplicit := and (hasKey $tlsMap "enabled") (ne $tlsEnabledRaw nil) -}}
 {{- $tlsEnabled := false -}}
 {{- if $tlsEnabledExplicit -}}
-  {{- $tlsEnabled = index $tlsMap "enabled" -}}
+  {{- $tlsEnabled = $tlsEnabledRaw -}}
 {{- else -}}
   {{- $tlsEnabled = .Values.external | default false -}}
 {{- end -}}

Also applies to: 47-47

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/apps/kafka/templates/kafka.yaml` around lines 15 - 21, The template
treats a present but null tls.enabled as explicit, causing tls: null to render;
fix by first capturing the raw value (e.g. introduce $tlsEnabledRaw := index
$tlsMap "enabled"), then change the explicit check to require both key presence
and non-nil (use and (hasKey $tlsMap "enabled") (ne $tlsEnabledRaw nil)), and
when that check passes assign $tlsEnabled = $tlsEnabledRaw, otherwise fall back
to the existing .Values.external | default false; update all uses of
$tlsEnabledExplicit/$tlsEnabled to the new logic so a null value is treated as
unset.

{{- $showExternal := .Values.external -}}
apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
metadata:
Expand All @@ -18,11 +40,11 @@ spec:
port: 9093
type: internal
tls: true
{{- if .Values.external }}
{{- if $showExternal }}
- name: external
port: 9094
type: loadbalancer
tls: false
tls: {{ $tlsEnabled }}
{{- end }}
config:
{{- if eq (int .Values.kafka.replicas) 1 }}
Expand Down
48 changes: 46 additions & 2 deletions packages/apps/kafka/tests/dashboard-resourcemap_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,31 @@ tests:
content: test-kafka-kafka-external-bootstrap
documentIndex: 0

# Role grants access to CA secret
- it: grants access to clients CA secret
# Role grants access to CA secrets
- it: grants access to clients CA cert secret
release:
name: test-kafka
namespace: tenant-test
asserts:
- contains:
path: rules[1].resourceNames
content: test-kafka-clients-ca-cert
documentIndex: 0
- notContains:
path: rules[1].resourceNames
content: test-kafka-clients-ca
documentIndex: 0

- it: grants access to cluster CA cert secret
release:
name: test-kafka
namespace: tenant-test
asserts:
- contains:
path: rules[1].resourceNames
content: test-kafka-cluster-ca-cert
documentIndex: 0

# Role grants access to workloadmonitors
- it: grants access to WorkloadMonitor
release:
Expand All @@ -104,6 +118,36 @@ tests:
value: cozystack.io
documentIndex: 0

# External bootstrap service present when tls.enabled=true and external=false ($showExternal is true)
- it: grants access to external bootstrap service when tls.enabled=true and external=false
release:
name: test-kafka
namespace: tenant-test
set:
external: false
tls:
enabled: true
asserts:
- contains:
path: rules[0].resourceNames
content: test-kafka-kafka-external-bootstrap
documentIndex: 0
Comment on lines +121 to +134
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

External bootstrap RBAC should not be granted when external=false.

This test enforces over-permissive behavior (external=false, tls.enabled=true still grants external bootstrap access). It contradicts the external-only gate and can preserve unnecessary RBAC rights.

Suggested test fix 
-  # External bootstrap service present when tls.enabled=true and external=false ($showExternal is true)
-  - it: grants access to external bootstrap service when tls.enabled=true and external=false
+  # External bootstrap service absent when external=false, regardless of tls.enabled
+  - it: does not grant access to external bootstrap service when tls.enabled=true and external=false
@@
-    asserts:
-      - contains:
+    asserts:
+      - notContains:
           path: rules[0].resourceNames
           content: test-kafka-kafka-external-bootstrap
         documentIndex: 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# External bootstrap service present when tls.enabled=true and external=false ($showExternal is true)
- it: grants access to external bootstrap service when tls.enabled=true and external=false
release:
name: test-kafka
namespace: tenant-test
set:
external: false
tls:
enabled: true
asserts:
- contains:
path: rules[0].resourceNames
content: test-kafka-kafka-external-bootstrap
documentIndex: 0
# External bootstrap service absent when external=false, regardless of tls.enabled
- it: does not grant access to external bootstrap service when tls.enabled=true and external=false
release:
name: test-kafka
namespace: tenant-test
set:
external: false
tls:
enabled: true
asserts:
- notContains:
path: rules[0].resourceNames
content: test-kafka-kafka-external-bootstrap
documentIndex: 0
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/apps/kafka/tests/dashboard-resourcemap_test.yaml` around lines 121 -
134, The test "grants access to external bootstrap service when tls.enabled=true
and external=false" is asserting over-permissive behavior; update the case to
assert that external bootstrap RBAC is NOT granted when external: false by
changing the spec under "asserts" to verify the resourceNames does not include
"test-kafka-kafka-external-bootstrap" (e.g., replace the contains check on path
rules[0].resourceNames/content test-kafka-kafka-external-bootstrap with a
not-contains or absence check), and adjust the test description to reflect the
corrected expectation.


# External bootstrap service absent when both external=false and tls.enabled=false
- it: does not grant access to external bootstrap when tls.enabled is explicitly false and external is false
release:
name: test-kafka
namespace: tenant-test
set:
external: false
tls:
enabled: false
asserts:
- notContains:
path: rules[0].resourceNames
content: test-kafka-kafka-external-bootstrap
documentIndex: 0

# RoleBinding references correct Role
- it: RoleBinding references correct Role
release:
Expand Down
11 changes: 9 additions & 2 deletions packages/apps/kafka/tests/kafka_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ tests:
# External listener #
#####################

- it: does not create external listener when external is false
- it: does not create external listener when external is false and tls.enabled is unset
release:
name: test-kafka
namespace: tenant-test
Expand All @@ -75,6 +75,13 @@ tests:
port: 9094
type: loadbalancer
tls: false
- notContains:
path: spec.kafka.listeners
content:
name: external
port: 9094
type: loadbalancer
tls: true

- it: creates external loadbalancer listener when external is true
release:
Expand All @@ -89,7 +96,7 @@ tests:
name: external
port: 9094
type: loadbalancer
tls: false
tls: true

- it: has exactly 2 listeners when external is false
release:
Expand Down
Loading
Loading