[WIP] Add TLS/SASL authentication support for Kafka functions - #3975
[WIP] Add TLS/SASL authentication support for Kafka functions#3975aliok wants to merge 7 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aliok The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR extends the function-level Kafka configuration to support TLS and SASL authentication, and propagates the resulting settings into the various deploy/run paths (Kubernetes/Knative deployers and local runners).
Changes:
- Extend
run.kafkaschema withsecurityProtocol,tls, andsasl(including validation). - Emit additional
KAFKA_SECURITY_PROTOCOL,KAFKA_TLS_*, andKAFKA_SASL_*environment variables during deployment/run. - Support
{{ secret:name:key }}-style value references for Kafka SASL user/password in the k8s deployer env var generation.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/knative/deployer.go | Updates Knative deploy path to use the new error-returning Kafka env injection (and track referenced resources). |
| pkg/k8s/deployer.go | Extends Kafka env var generation to include TLS/SASL fields and secret/configMap key refs for SASL values. |
| pkg/k8s/deployer_test.go | Adapts existing tests to new signature and adds coverage for TLS/SASL and secret-ref cases. |
| pkg/functions/runner.go | Propagates Kafka TLS/SASL env vars for the host runner (func run). |
| pkg/functions/function.go | Adds new Kafka config types/fields and validation rules for protocol/TLS/SASL combinations. |
| pkg/functions/function_test.go | Adds validation test cases for the new Kafka TLS/SASL config combinations. |
| pkg/docker/runner.go | Propagates Kafka TLS/SASL env vars for the Docker runner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
schema/func_yaml-schema.json:313
- schema/func_yaml-schema.json appears to be a generated artifact (see Makefile:393-403 and schema/generator/main.go:23-55). Editing it manually can easily drift from the Go struct tags in pkg/functions/function.go; it should be regenerated via
make schema-generateand the regenerated output committed instead of hand-maintaining this section.
"securityProtocol": {
"enum": [
"PLAINTEXT",
"SSL",
"SASL_PLAINTEXT",
"SASL_SSL"
],
"type": "string",
"description": "Security protocol: PLAINTEXT SSL SASL_PLAINTEXT or SASL_SSL"
},
"tls": {
"$schema": "http://json-schema.org/draft-04/schema#",
"$ref": "#/definitions/KafkaTLS",
"description": "TLS configuration for SSL or SASL_SSL"
},
"sasl": {
"$schema": "http://json-schema.org/draft-04/schema#",
"$ref": "#/definitions/KafkaSASL",
"description": "SASL authentication for SASL_PLAINTEXT or SASL_SSL"
}
pkg/functions/function.go:265
- Kafka SASL template refs are validated against templateRefPattern, but the current regex doesn’t allow trailing whitespace after the closing
}}while the deploy-time parser (pkg/k8s/deployer.go) effectively tolerates it via TrimSpace/Trim. This can cause valid-looking values (e.g."{{ secret:n:k }} ") to fail validation even though they’d parse during deploy. Consider trimming before validation and allowing optional trailing whitespace so validation and parsing accept the same inputs.
var templateRefPattern = regexp.MustCompile(`^\{\{\s*(secret|configMap):[^:]+:[^:]+\s*\}\}$`)
func validateTemplateRef(field, value string) (errors []string) {
if strings.HasPrefix(value, "{{") && !templateRefPattern.MatchString(value) {
errors = append(errors, fmt.Sprintf("%s has invalid reference format %q, expected {{ secret:name:key }} or {{ configMap:name:key }}", field, value))
}
pkg/k8s/deployer.go:808
- appendKafkaEnvValue only treats values as template refs when the raw string starts with
"{{", so leading whitespace (e.g." {{ secret:n:k }}") will silently be treated as a literal and won’t produce a ValueFrom SecretKeyRef/ConfigMapKeyRef. Since validation/parsing already tolerates whitespace elsewhere, it’s safer to TrimSpace before checking for{{and to parse the innersecret|configMap:name:keyby stripping{{/}}explicitly.
func appendKafkaEnvValue(envVars []corev1.EnvVar, name, value string, referencedSecrets, referencedConfigMaps *sets.Set[string]) ([]corev1.EnvVar, error) {
if strings.HasPrefix(value, "{{") {
if !strings.HasSuffix(strings.TrimSpace(value), "}}") {
return nil, fmt.Errorf("invalid reference format %q, expected {{ secret:name:key }} or {{ configMap:name:key }}", value)
}
slices := strings.Split(strings.Trim(value, "{} "), ":")
if len(slices) == 3 {
valueFrom, err := createEnvVarSource(slices, referencedSecrets, referencedConfigMaps)
if err != nil {
return nil, err
}
return append(envVars, corev1.EnvVar{Name: name, ValueFrom: valueFrom}), nil
}
return nil, fmt.Errorf("invalid reference format %q, expected {{ secret:name:key }} or {{ configMap:name:key }}", value)
}
return append(envVars, corev1.EnvVar{Name: name, Value: value}), nil
pkg/functions/runner.go:344
- This adds new Kafka TLS/SASL environment variable propagation for the host runner, but there are already unit tests covering Kafka env construction (pkg/functions/runner_test.go:42-78) and they don’t assert the new variables. Adding a test case for SecurityProtocol/TLS/SASL here would prevent regressions and keep runner behavior aligned with the k8s deployer tests.
if k.SecurityProtocol != "" && k.SecurityProtocol != "PLAINTEXT" {
env = append(env, "KAFKA_SECURITY_PROTOCOL="+k.SecurityProtocol)
}
if k.TLS != nil {
if k.TLS.CACert != "" {
env = append(env, "KAFKA_TLS_CA_CERT="+k.TLS.CACert)
}
if k.TLS.ClientCert != "" {
env = append(env, "KAFKA_TLS_CLIENT_CERT="+k.TLS.ClientCert)
}
if k.TLS.ClientKey != "" {
env = append(env, "KAFKA_TLS_CLIENT_KEY="+k.TLS.ClientKey)
}
if k.TLS.SkipVerify {
env = append(env, "KAFKA_TLS_SKIP_VERIFY=true")
}
}
if k.SASL != nil {
if k.SASL.Mechanism != "" {
env = append(env, "KAFKA_SASL_MECHANISM="+k.SASL.Mechanism)
}
if k.SASL.User != "" {
env = append(env, "KAFKA_SASL_USER="+k.SASL.User)
}
if k.SASL.Password != "" {
env = append(env, "KAFKA_SASL_PASSWORD="+k.SASL.Password)
}
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (7)
pkg/functions/runner.go:324
- The host runner's new TLS/SASL environment branches are untested even though
buildRunnerEnvalready has focused Kafka tests. Add cases covering protocol, TLS fields/skip verification, SASL credentials, and explicitPLAINTEXTprecedence over an inherited protocol.
if k.TLS != nil {
if k.TLS.CACert != "" {
env = append(env, "KAFKA_TLS_CA_CERT="+k.TLS.CACert)
}
if k.TLS.ClientCert != "" {
pkg/functions/function.go:247
- SASL protocols can currently pass validation with no
saslblock, or with an empty user/password, even though the dependent runtime requires both credentials and returns an error at startup otherwise. Require the SASL block and both credential fields whenever a SASL protocol is selected.
if kafka.SASL != nil {
if kafka.SecurityProtocol != "SASL_PLAINTEXT" && kafka.SecurityProtocol != "SASL_SSL" {
errors = append(errors, "run.kafka.sasl requires securityProtocol SASL_PLAINTEXT or SASL_SSL")
}
validMechanisms := map[string]bool{"": true, "PLAIN": true, "SCRAM-SHA-256": true, "SCRAM-SHA-512": true}
pkg/functions/runner.go:319
- An explicit
securityProtocol: PLAINTEXTis not added to the host process environment. BecausebuildRunnerEnvstarts withos.Environ(), an inheritedKAFKA_SECURITY_PROTOCOLsuch asSASL_SSLremains effective and changes the configured function's protocol. Emit every explicitly configured protocol; only omit the empty default.
This issue also appears on line 320 of the same file.
if k.SecurityProtocol != "" && k.SecurityProtocol != "PLAINTEXT" {
env = append(env, "KAFKA_SECURITY_PROTOCOL="+k.SecurityProtocol)
}
pkg/functions/function.go:241
- A configuration with only one of
clientCertorclientKeypassesFunction.Validate, but the Kafka runtime rejects that pair before connecting. Validate that these mutual-TLS fields are either both set or both empty so deployment fails early with a configuration error.
This issue also appears on line 243 of the same file.
if kafka.TLS != nil {
if kafka.SecurityProtocol != "SSL" && kafka.SecurityProtocol != "SASL_SSL" {
errors = append(errors, "run.kafka.tls requires securityProtocol SSL or SASL_SSL")
}
}
pkg/k8s/deployer.go:753
- An explicit
securityProtocol: PLAINTEXTis omitted here, so a conflictingKAFKA_SECURITY_PROTOCOLfromrun.envsor the image remains effective. This also makes the dedicated Kafka field overriderun.envsfor secure protocols but not forPLAINTEXT; emit every non-empty configured protocol.
if kafka.SecurityProtocol != "" && kafka.SecurityProtocol != "PLAINTEXT" {
envVars = append(envVars, corev1.EnvVar{Name: "KAFKA_SECURITY_PROTOCOL", Value: kafka.SecurityProtocol})
}
pkg/docker/runner.go:311
- An explicit
securityProtocol: PLAINTEXTis omitted here, so a conflictingKAFKA_SECURITY_PROTOCOLfromrun.envsor the image remains effective. This also makes dedicated Kafka configuration overriderun.envsfor secure protocols but not forPLAINTEXT; emit every non-empty configured protocol.
if k.SecurityProtocol != "" && k.SecurityProtocol != "PLAINTEXT" {
c.Env = append(c.Env, "KAFKA_SECURITY_PROTOCOL="+k.SecurityProtocol)
}
pkg/k8s/deployer_test.go:598
- This loop only checks an env var when it happens to be present, so the test still passes if either
KAFKA_SASL_USERorKAFKA_SASL_PASSWORDis omitted. Look up both names explicitly and fail when either is missing before checking itsSecretKeyRef.
for _, ev := range got {
if ev.Name == "KAFKA_SASL_USER" {
if ev.ValueFrom == nil || ev.ValueFrom.SecretKeyRef == nil {
t.Fatal("KAFKA_SASL_USER should have ValueFrom with SecretKeyRef")
}
Summary
KafkaConfigwithsecurityProtocol,tls, andsaslfields in func.yamlKAFKA_SECURITY_PROTOCOL,KAFKA_TLS_*,KAFKA_SASL_*env vars{{ secret:name:key }}syntax supported forsasl.userandsasl.passwordDepends on knative-extensions/func-go#186
func.yaml example (SASL_SSL)
Verification instructions (Kind + Strimzi)
Prerequisites
1. Build the func CLI
Both repos have un-merged branches. Build the CLI from the
kafka-tls-saslbranch:2. Patch the scaffolding to use the func-go fork
The func-go dependency lives in the scaffolding's
go.mod(embedded in the CLI), not the function'sgo.mod. Add areplacedirective, re-tidy, regenerate the embedded filesystem, and rebuild:3. Create a Kind cluster with Knative
4. Install Strimzi with a TLS+SASL listener
5. Create a KafkaUser and topic
6. Create the function
7. Configure func.yaml
Copy secrets to the function namespace:
Edit
func.yaml:8. Deploy and verify
FUNC_REGISTRY=ttl.sh/my-kafka-tls-test /tmp/func-local deploy --build --verbose kubectl wait pods -l serving.knative.dev/service=my-kafka-tls-func \ --for=condition=Ready --timeout=120sCheck env vars on the pod:
9. Tail logs and send a test message
Expected log output:
Cleanup
kubectl delete ksvc my-kafka-tls-func kubectl delete secret my-cluster-cluster-ca-cert my-kafka-user -n default kubectl delete kafkauser my-kafka-user -n kafka kubectl delete kafkatopic test-topic -n kafka kubectl delete kafka my-cluster -n kafka kubectl delete -f 'https://strimzi.io/install/latest?namespace=kafka' -n kafka kubectl delete namespace kafka kind delete cluster --name kafka-tls-test rm -rf /tmp/my-kafka-tls-func /tmp/func-local