Skip to content

fix: record the class a claim resolved to, and stop guessing its family - #129

Open
scotwells wants to merge 1 commit into
mainfrom
fix/128-default-claim-class
Open

scotwells wants to merge 1 commit into
mainfrom
fix/128-default-claim-class

Conversation

@scotwells

Copy link
Copy Markdown
Contributor

Closes #128.

Problem

A claim may name a family instead of a class, and NSO files every primary interface address that way — NetworkInterfaceClaim.spec.ipFamilies defaults to {IPv6}, so it is the default path, not an edge case. Three things follow from that:

The resolved class is never recorded. It reaches the IPAllocation but not the claim, so the object does not say what it allocated under. The default for a family can be repointed, so the answer is not recoverable afterwards — and kubectl get ipclaims shows <none> for a claim that is very much bound to a class.

spec.ipFamily is silently ignored when a class is named. ResolveClass returns LoadClass(className) without looking at it, so a claim asking for IPv4 from an IPv6 class allocates IPv6 and reports no error.

Prefix-length validation assumes IPv4. It reads the claim's own family and defaults to 32, so a claim naming an IPv6 class and asking for prefixLength: 64 is rejected with "must be between 1 and 32" — exactly the shape the class model encourages.

Change

spec.className is defaulted from the resolved class, in Create, before the claim is persisted — the same way a PersistentVolumeClaim is defaulted to the cluster's StorageClass. It is already immutable, so what is written is what the claim allocated under, permanently. The conventional defaulting hook cannot do this: PrepareForCreate has no transaction and cannot find the default class.

A family that disagrees with the resolved class is refused, under a new FamilyMismatch reason. Agreement stays legal — the fabric-identity controller sets both fields on all 18 of its claims in production, consistently.

Validation now applies only the bound that holds for every family (1–128). EffectivePrefixLength already applies the family-specific one against the resolved class, so the check was redundant as well as wrong.

Left out

spec.ipFamily stays. It is the input the defaulting needs, and removing it would break primary addressing for every workload. No API types change and no migration: existing claims keep whatever they were created with, and new ones name their class.

A claim may name a family instead of a class, and NSO files every primary
interface address that way. The resolved class was written to the IPAllocation
but never to the claim, so the object never recorded what it allocated under —
and the default for a family can be repointed, making the answer unrecoverable.
The claim is persisted after resolution, so spec.className is now defaulted
from the resolved class the same way a PersistentVolumeClaim is defaulted to
the cluster's StorageClass. It is already immutable, so it cannot drift.

spec.ipFamily selects a default class; it was also silently ignored whenever a
claim named one, so a claim could ask for IPv4, get IPv6, and see no error. A
family that disagrees with the class is now refused. Agreement stays legal: the
fabric-identity controller sets both fields on every claim it files.

Prefix-length validation assumed IPv4 when the claim stated no family, so a /64
from an IPv6 class was rejected with "must be between 1 and 32" — the common
shape under the class model. Validation runs before the class is resolved and
cannot read it, so it now applies only the bound that holds for every family;
EffectivePrefixLength already applies the family-specific one against the class.
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.

IPClaim should not carry spec.ipFamily; the class owns the family

1 participant