Skip to content

fix: trim whitespace in comma-separated annotation values#442

Open
AlinsRan wants to merge 1 commit into
masterfrom
ee-2723-annotation-trim
Open

fix: trim whitespace in comma-separated annotation values#442
AlinsRan wants to merge 1 commit into
masterfrom
ee-2723-annotation-trim

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Type of change:

  • Bugfix

What this PR does / why we need it:

Sync from apache/apisix-ingress-controller#2815 (fixes apache/apisix-ingress-controller#2723).

GetStringsAnnotation split comma-separated annotation values with strings.Split(value, ",") and did not trim whitespace. 127.0.0.1, 0.0.0.0 produced ["127.0.0.1", " 0.0.0.0"]; the leading space made the IP invalid and stalled sync for every resource under that ingressClass. This trims each element and drops blanks.

Pre-submission checklist:

  • Test cases added (TestGetStringsAnnotation)
  • Backward compatible

Summary by CodeRabbit

  • Bug Fixes

    • Improved comma-separated annotation parsing by trimming surrounding whitespace.
    • Empty entries are now ignored for cleaner, more reliable results.
  • Tests

    • Added coverage for whitespace handling, empty values, multiple entries, and missing annotations.

GetStringsAnnotation split values on commas without trimming, so values
like "127.0.0.1, 0.0.0.0" produced a leading-space element that broke IP
validation and stalled sync. Trim each element and drop blanks.

Sync from apache/apisix-ingress-controller#2815 (fixes #2723)
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 75fc1722-8e70-4ad1-907e-e8c5ded809dc

📥 Commits

Reviewing files that changed from the base of the PR and between 5f84a00 and d3a52e2.

📒 Files selected for processing (2)
  • internal/adc/translator/annotations/types.go
  • internal/adc/translator/annotations/types_test.go

📝 Walkthrough

Walkthrough

GetStringsAnnotation now trims whitespace from comma-separated annotation values and omits empty elements. Tests cover splitting, whitespace handling, blank values, and nil results for missing or entirely blank annotations.

Changes

Annotation parsing

Layer / File(s) Summary
Normalize annotation string values
internal/adc/translator/annotations/types.go, internal/adc/translator/annotations/types_test.go
GetStringsAnnotation trims each comma-separated item, skips empty items, preserves nil for empty input, and verifies the behavior with unit tests.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning The fix is correct, but coverage is unit-only; there’s no end-to-end test of the ingress reconciliation flow using spaced comma-separated annotations. Add an e2e test that applies an ingress with values like 127.0.0.1, 0.0.0.0 and verifies reconciliation/validation succeeds end-to-end.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: trimming whitespace in comma-separated annotation values.
Linked Issues check ✅ Passed The PR trims split annotation values and drops blanks, matching the linked issue's required parsing fix and test coverage.
Out of Scope Changes check ✅ Passed The changes stay focused on annotation parsing and its tests, with no obvious unrelated additions.
Security Check ✅ Passed Only annotation parsing changed; no logging, auth, storage, TLS, or permission code was touched, so no security issues found.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ee-2723-annotation-trim

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-07-22T00:55:06Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests succeeded.
- core:
    failedTests:
    - HTTPRouteInvalidBackendRefUnknownKind
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 1
      Passed: 31
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests failed with 1 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    result: partial
    skippedTests:
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 10
      Skipped: 1
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 1 test skips.

@github-actions

Copy link
Copy Markdown
Contributor

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-07-22T00:55:12Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests succeeded.
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 0
      Passed: 32
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 1 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 10
      Skipped: 1
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 1 test skips.

@AlinsRan AlinsRan self-assigned this Jul 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-07-22T01:12:43Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    failedTests:
    - GatewayModifyListeners
    result: failure
    statistics:
      Failed: 1
      Passed: 11
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests failed with 1 test failures.
- core:
    failedTests:
    - GatewayModifyListeners
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 1
      Passed: 31
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests failed with 1 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    failedTests:
    - GatewayModifyListeners
    - TLSRouteSimpleSameNamespace
    result: failure
    statistics:
      Failed: 2
      Passed: 9
      Skipped: 0
  name: GATEWAY-TLS
  summary: Core tests failed with 2 test failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Ingress annotations: comma-separated values are not trimmed, causing invalid IP parsing when spaces are present

1 participant