Skip to content

Fix bound-CRD sort comparator reading annotation from wrong element #4444

Closed

Description

@Sahan-Chathumina-25

Description

I found a copy-paste bug in the OpenAPI v3 cache's CRD ordering: byClusterAndName.Less in pkg/server/openapiv3/servicecache.go:311 reads the bound-CRD annotation from a[i] twice instead of comparing a[i] against a[j]:

_, aBound := a[i].Annotations[BoundAnnotationKey]
_, bBound := a[i].Annotations[BoundAnnotationKey] // should be a[j]

Because bBound always equals aBound, the aBound && !bBound / !aBound && bBound branches never fire and bound CRDs (those carrying apis.kcp.io/bound-crd) are never sorted ahead of regular CRDs. Ordering then falls through to cluster/name comparison, so the resulting orderedCRDs list — and therefore the deterministic API-configuration key built from it in apiConfigurationKey — depends on CRD names rather than the intended bound-first order. In the worst case two logically different orderings can produce different cache keys for the same configuration, or the merge order of bound vs. local specs in addSpecs is not the intended one.

Reproduce

  1. Add a temporary test in pkg/server/openapiv3 with a bound CRD named zzz.example.com (annotation apis.kcp.io/bound-crd: "true") and a plain CRD named aaa.example.com.
  2. Run go test ./pkg/server/openapiv3/ -run TestReproBoundSort -v -count=1.
  3. Observed result: Less(bound,plain)=false, Less(plain,bound)=true, and after sort.Sort the order is [aaa.example.com zzz.example.com] — the bound CRD is not sorted first, so the test fails with BUG: bound CRD not sorted first.

Expected

Less should read the annotation from a[j] on line 311 (_, bBound := a[j].Annotations[BoundAnnotationKey]), so a bound CRD always sorts before a non-bound one and the one-line fix restores the intended deterministic ordering and cache-key stability.

Checklist

  • Searched open issues for openapi/bound/sort (0 hits) and triaged 4 closed items -- no bound-sort comparator report
  • File:line + repro provided

Distinct from bug: something is spamming bound CRD resolution #4216, which is about noisy log spam from bound-CRD lookups at startup; this report is about the Less comparator in the OpenAPI v3 service cache never ordering bound CRDs first.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions