knative-extensions / knative-extensions/func-go
Harden Kafka security env-var parsing
- Dominant language
- Go
- Stars
- 7
- Forks
- 12
- PR merge metrics
- No merged PRs in 30d
Description
Follow-up from https://github.com/knative-extensions/func-go/pull/186#issuecomment-5496815228 (review by @matejvasek).
None of these are reachable through the normal func deployment path — the CLI validates enums and only emits env vars alongside a matching `KAFKA_SECURITY_PROTOCOL`. They only matter for direct callers (hand-written manifests, `docker run`, standalone host runner).
### 1. Silent PLAINTEXT fallback
If `KAFKA_SECURITY_PROTOCOL` is unset but `KAFKA_TLS_*` / `KAFKA_SASL_*` are set, `configureSecurity` returns nil and connects in cleartext with no warning. A log warning or error would prevent silently insecure connections for direct callers.
### 2. Case-sensitive enums
`KAFKA_SECURITY_PROTOCOL` and `KAFKA_SASL_MECHANISM` are matched case-sensitively (`SASL_SSL` works but `sasl_ssl` doesn't). Inconsistent with the `EqualFold` used for `KAFKA_TLS_SKIP_VERIFY`. A `strings.ToUpper` before the switch would align them.
### 3. `KAFKA_TLS_SKIP_VERIFY` parsing
Only the exact token `true` is honored; `1` / `yes` / `" true "` silently leave verification on. Fails safe, but silently ignores operator intent.
### 4. `scramClient.Step` / `Done` nil-deref
`c.conv` is nil until `Begin` succeeds; there's no defensive nil check. Sarama's current flow guards this by aborting on a `Begin` error, so it's latent rather than reachable today. A nil guard would keep it safe if Sarama's flow changes.
Contributor guide
Research direction
Start by locating configureSecurity and the scramClient Step, Done, and Begin methods. Review how the listed KAFKA_* variables are parsed, then add coverage for insecure fallback warnings or errors, case-insensitive values, accepted skip-verify input, and nil safety; done means these direct-caller cases are handled without changing the normal deployment path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kafka
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100