Skip to content

Repair OpenAPI definition names for the identity group - #765

Open
yahyafakhroji wants to merge 1 commit into
mainfrom
fix/openapi-identity-definition-names
Open

yahyafakhroji wants to merge 1 commit into
mainfrom
fix/openapi-identity-definition-names

Conversation

@yahyafakhroji

Copy link
Copy Markdown
Contributor

What

Convert slash-bearing OpenAPI definition names before handing them to the generic
namer, on both the v2 and v3 configs. Adds guard tests.

Why

The identity types are generated by a pinned openapi-gen (v0.23.0, Taskfile.yaml:346)
that predates the Kubernetes 1.35 convention of emitting REST-friendly model names, so
it emits raw Go import paths as definition keys. Through 1.34 the serving layer applied
that conversion itself; 1.35 moved it into the generator and turned
DefinitionNamer.GetDefinitionName into a pass-through, so the import path now reaches
the published spec verbatim.

Its slashes are JSON-Pointer escaped to ~1 inside every $ref while the definitions
key keeps the literal /, leaving all 9 intra-group references dangling. That poisons
the whole /openapi/v2 document, so any client doing client-side schema validation
fails on resources with nothing to do with identity — this surfaced as
network-services-operator's Federated E2E failing to apply an IPAM fixture:

error validating "test/e2e/fixtures/ipam/project-alpha/classes.yaml":
  SchemaError(go.miloapis.com/milo/pkg/apis/identity/v1alpha1.Passkey.status):
  unknown model in reference: "go.miloapis.com~1milo~1pkg~1apis~1identity~1v1alpha1.PasskeyStatus"

Passkey is only the alphabetically first casualty — all 13 identity definitions are
affected, ServiceAccountKey, Session and UserIdentity included. It has hit NSO
main twice (32896926505,
32959526255);
it only looks flaky because kubectl does not always take the client-side validation path.

Two details worth review attention:

  • The conversion runs before the namer rather than after, which lets the namer's
    lookup hit and restores the x-kubernetes-group-version-kind extension the identity
    kinds were silently losing.
  • Only slash-bearing names are converted. ToRESTFriendlyName is not idempotent and
    would otherwise mangle io.k8s.api.core.v1.Pod into Pod.v1.core.api.k8s.io.

This is a stopgap so the shared e2e stops breaking. The real fix is regenerating with a
1.35-era openapi-gen (now at k8s.io/kube-openapi/cmd/openapi-gen, needs
+k8s:openapi-model-package and --output-model-name-file). I trial-ran it: it produces
byte-identical names, so that follow-up is a no-op on the wire and the override can be
deleted in the same change.

Test plan

  • go build ./...
  • go test ./cmd/milo/apiserver/
  • Guard tests verified to fail without the fix — all 13 identity definitions reported
  • Cut a Milo release and bump the milo-kustomize pin in NSO (config/dependencies/milo/,
    currently v0.32.5) to confirm Federated E2E goes green

https://claude.ai/code/session_01GJXyxySyTN3vmkeFs1HMoL

The identity types are generated by a pinned openapi-gen (v0.23.0) that
predates the Kubernetes 1.35 convention of emitting REST-friendly model
names. Through 1.34 the serving layer applied that conversion itself;
1.35 moved it into the generator and turned GetDefinitionName into a
pass-through, so the raw Go import path now reaches the published spec.

Its slashes are JSON-Pointer escaped to "~1" inside every $ref while the
definitions key keeps the literal "/", leaving all 9 intra-group
references dangling. That poisons the whole /openapi/v2 document, so any
client doing client-side schema validation fails on resources that have
nothing to do with identity:

  SchemaError(go.miloapis.com/milo/pkg/apis/identity/v1alpha1.Passkey.status):
  unknown model in reference: "go.miloapis.com~1milo~1...~1PasskeyStatus"

Passkey is only the alphabetically first casualty; ServiceAccountKey,
Session and UserIdentity are equally affected.

Convert slash-bearing names before handing them to the generic namer.
Doing it first rather than last also lets the namer's lookup hit, which
restores the x-kubernetes-group-version-kind extension the identity
kinds were silently losing. Only slash-bearing names are converted:
ToRESTFriendlyName is not idempotent and would otherwise mangle
"io.k8s.api.core.v1.Pod" into "Pod.v1.core.api.k8s.io".

This is a stopgap. The real fix is regenerating with a 1.35-era
openapi-gen, which emits the same names, making that follow-up a
no-op on the wire.

Claude-Session: https://claude.ai/code/session_01GJXyxySyTN3vmkeFs1HMoL
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.

1 participant