feat: Limit public IPv4 claims per project

config/components/README.md

@@ -14,6 +14,7 @@ and order-insensitive within an overlay (with a few documented exceptions).
 | `observability`          | `ServiceMonitor` + `GrafanaDashboard` resources                          |
 | `k6-performance-tests`   | k6 SA/RBAC + `TestRun` resources for the perf suite                      |
 | `service-catalog`        | `Service` + `ServiceConfiguration` registering IPAM (incl. IPClaim quota) on the Milo control plane |
+| `service-catalog-public-ipv4` | `service-catalog` plus a per-project public IPv4 IPClaim limit; needs a Milo and service catalog that honour metric cost conditions |
 
 Order matters when:
 - `cert-manager-ca` must precede the deployment so its CSI volume can mount.

config/components/service-catalog-public-ipv4/kustomization.yaml

@@ -0,0 +1,47 @@
+# The service catalog registration plus a per-project public IPv4 claim limit.
+#
+# The limit charges only IPv4 claims from a public class, through a metric cost
+# condition. A Milo older than milo-os/milo#940 drops that condition and charges
+# every IPClaim, so an environment selects this path in place of
+# ../service-catalog only once its Milo and service catalog carry conditions.
+apiVersion: kustomize.config.k8s.io/v1alpha1
+kind: Component
+
+components:
+  - ../service-catalog
+
+patches:
+  - target:
+      group: services.miloapis.com
+      version: v1alpha1
+      kind: ServiceConfiguration
+      name: ipam-miloapis-com
+    patch: |-
+      - op: add
+        path: /spec/metrics/-
+        value:
+          name: ipam.miloapis.com/public-ipv4claims
+          displayName: Public IPv4 Claims
+          description: >-
+            Number of public IPv4 IPClaim objects a project holds. Each counts
+            once against this metric in addition to the IP Claims metric.
+          kind: Gauge
+          unit: "1"
+      - op: add
+        path: /spec/quota/limits/-
+        value:
+          name: public-ipv4claims-per-project
+          metric: ipam.miloapis.com/public-ipv4claims
+          consumerType:
+            apiGroup: resourcemanager.miloapis.com
+            kind: Project
+          unit: "1/{project}"
+          defaultLimit: 2
+          maxLimit: 256
+      - op: add
+        path: /spec/quota/metricRules/0/metricCosts/ipam.miloapis.com~1public-ipv4claims
+        value: 1
+      - op: add
+        path: /spec/quota/metricRules/0/metricCostConditions
+        value:
+          ipam.miloapis.com/public-ipv4claims: "has(trigger.spec.ipFamily) && trigger.spec.ipFamily == 'IPv4' && has(trigger.spec.addressScope) && trigger.spec.addressScope == 'Public'"

docs/api.md

@@ -22,6 +22,7 @@ IPv6 are never mixed.
 
 ```go
 IPFamily        = IPv4 | IPv6
+AddressScope    = Public | Private
 Strategy        = FirstFit | BestFit | LeastUtilized
 ReclaimPolicy   = Delete | Retain
 ClaimPhase      = Pending | Bound | Releasing | Error
@@ -69,6 +70,7 @@ The claim's CREATE response body contains the allocated CIDR.
 | Field | Type | Required | Notes |
 |-------|------|----------|-------|
 | `spec.ipFamily` | IPFamily | yes | Address family to allocate. |
+| `spec.addressScope` | AddressScope | no | Copied from the claim's class on create; if set, must agree with the class. Immutable. |
 | `spec.prefixLength` | int (0–128) | yes | Requested sub-prefix size; must be valid for the family and within the pool's min/max. |
 | `spec.poolRef` | NamespacedRef | one of | Pin to a specific pool (optionally in another project via `projectRef`). |
 | `spec.poolSelector` | PoolSelector | one of | Pick a pool by labels (optionally scoped by `projectRef`). |

internal/registry/ipam/ipamvalidation/addressscope.go

@@ -0,0 +1,16 @@
+package ipamvalidation
+
+import (
+	"k8s.io/apimachinery/pkg/util/validation/field"
+
+	"go.miloapis.com/ipam/pkg/apis/ipam"
+)
+
+func AddressScope(s ipam.AddressScope, path *field.Path) field.ErrorList {
+	switch s {
+	case "", ipam.AddressScopePublic, ipam.AddressScopePrivate:
+		return nil
+	}
+	return field.ErrorList{field.NotSupported(path, s,
+		[]string{string(ipam.AddressScopePublic), string(ipam.AddressScopePrivate)})}
+}

internal/registry/ipam/ipclaim/storage.go

@@ -218,35 +218,11 @@ func (r *AllocatingREST) Create(ctx context.Context, obj runtime.Object, createV
 		metrics.RecordAllocationFailure("ipclaim", "internal", ipFamily, project, org)
 		return nil, err
 	}
-	if createValidation != nil {
-		if err := createValidation(ctx, claim.DeepCopyObject()); err != nil {
-			metrics.RecordAllocationFailure("ipclaim", "internal", ipFamily, project, org)
-			return nil, err
-		}
-	}
-
-	// Before any capacity is reserved: a namespace that is gone, or going,
-	// never releases what is bound into it.
-	if err := r.checkNamespaceLiveness(ctx, project, claim.Namespace); err != nil {
-		metrics.RecordAllocationFailure("ipclaim", "namespace_not_live", ipFamily, project, org)
-		failSpan(tracing.ReasonNamespaceNotLive)
-		return nil, err
-	}
-
 	if claim.Spec.ClassName == "" && claim.Spec.IPFamily == "" {
 		metrics.RecordAllocationFailure("ipclaim", "internal", ipFamily, project, org)
 		return nil, apierrors.NewBadRequest("spec.className or spec.ipFamily is required")
 	}
 
-	// Overwrite any client-supplied ownerRef. The requestheader CA guarantees
-	// the Extra headers carrying the tenant identity, so that identity is the
-	// source of truth for who this claim is for.
-	claim.Spec.OwnerRef = &ipam.ObjectRef{
-		APIGroup: id.APIGroup,
-		Kind:     id.Kind,
-		Name:     id.Name,
-	}
-
 	tx, err := r.db.Begin(ctx)
 	if err != nil {
 		metrics.RecordAllocationFailure("ipclaim", "tx_error", ipFamily, project, org)
@@ -259,8 +235,8 @@ func (r *AllocatingREST) Create(ctx context.Context, obj runtime.Object, createV
 	// then finds the pool that project offers. Cross-project sharing is the
 	// reference, so there is no separate cross-project path here.
 	class, err := allocator.ResolveClass(ctx, tx, claim.Spec.ClassName, v1alpha1.IPFamily(claim.Spec.IPFamily))
+	_ = tx.Rollback(ctx)
 	if err != nil {
-		_ = tx.Rollback(ctx)
 		metrics.RecordAllocationFailure("ipclaim", "class_not_found", ipFamily, project, org)
 		failSpan(tracing.ReasonPoolNotFound)
 		switch {
@@ -281,25 +257,58 @@ func (r *AllocatingREST) Create(ctx context.Context, obj runtime.Object, createV
 	// about the class a claim names. Silently ignoring a disagreement let a
 	// claim ask for IPv4 from an IPv6 class and get IPv6 back with no error.
 	if claim.Spec.IPFamily != "" && string(claim.Spec.IPFamily) != string(class.Spec.IPFamily) {
-		_ = tx.Rollback(ctx)
 		metrics.RecordAllocationFailure("ipclaim", "invalid", ipFamily, project, org)
 		failSpan(tracing.ReasonPoolNotFound)
 		return nil, ipamerrors.New(ipamerrors.ReasonFamilyMismatch, fmt.Sprintf(
 			"spec.ipFamily is %s but class %q hands out %s",
 			claim.Spec.IPFamily, class.LocalName, class.Spec.IPFamily))
 	}
+	if claim.Spec.AddressScope != "" && string(claim.Spec.AddressScope) != string(class.Spec.AddressScope) {
+		metrics.RecordAllocationFailure("ipclaim", "invalid", ipFamily, project, org)
+		failSpan(tracing.ReasonPoolNotFound)
+		return nil, ipamerrors.New(ipamerrors.ReasonAddressScopeMismatch, fmt.Sprintf(
+			"spec.addressScope is %s but class %q has addressScope %q",
+			claim.Spec.AddressScope, class.Name, class.Spec.AddressScope))
+	}
 
 	// Default the class the same way a PersistentVolumeClaim is defaulted to
 	// the cluster's StorageClass: the claim is persisted below, so the stored
 	// object and the create response both name the class this resolved to
 	// rather than leaving the choice implicit. spec.className is immutable, so
 	// what is written here is what the claim allocated under, for good.
 	claim.Spec.ClassName = class.LocalName
+	claim.Spec.IPFamily = ipam.IPFamily(class.Spec.IPFamily)
+	claim.Spec.AddressScope = ipam.AddressScope(class.Spec.AddressScope)
+	ipFamily = string(claim.Spec.IPFamily)
+	span.SetAttributes(attribute.String(tracing.AttrClaimIPFamily, ipFamily))
+
+	if createValidation != nil {
+		if err := createValidation(ctx, claim.DeepCopyObject()); err != nil {
+			metrics.RecordAllocationFailure("ipclaim", "internal", ipFamily, project, org)
+			return nil, err
+		}
+	}
+
+	// Before any capacity is reserved: a namespace that is gone, or going,
+	// never releases what is bound into it.
+	if err := r.checkNamespaceLiveness(ctx, project, claim.Namespace); err != nil {
+		metrics.RecordAllocationFailure("ipclaim", "namespace_not_live", ipFamily, project, org)
+		failSpan(tracing.ReasonNamespaceNotLive)
+		return nil, err
+	}
+
+	// Overwrite any client-supplied ownerRef. The requestheader CA guarantees
+	// the Extra headers carrying the tenant identity, so that identity is the
+	// source of truth for who this claim is for.
+	claim.Spec.OwnerRef = &ipam.ObjectRef{
+		APIGroup: id.APIGroup,
+		Kind:     id.Kind,
+		Name:     id.Name,
+	}
 
 	if claim.Spec.Target == ipam.TargetScopeRange {
-		// The chain is provisioned outside this transaction, level by level,
-		// exactly as it is for a block claim — so this one is released first.
-		_ = tx.Rollback(ctx)
+		// The chain is provisioned outside any transaction, level by level,
+		// exactly as it is for a block claim.
 		obj, outcome, err := r.createScopeRange(ctx, span, claim, class, id, dryRun)
 		result = outcome
 		if err != nil {
@@ -312,7 +321,6 @@ func (r *AllocatingREST) Create(ctx context.Context, obj runtime.Object, createV
 
 	prefixLen, err := allocator.EffectivePrefixLength(class.IPClass, claim.Spec.PrefixLength)
 	if err != nil {
-		_ = tx.Rollback(ctx)
 		metrics.RecordAllocationFailure("ipclaim", "internal", ipFamily, project, org)
 		return nil, ipamerrors.New(ipamerrors.ReasonPrefixLengthRejected, err.Error())
 	}
@@ -326,7 +334,6 @@ func (r *AllocatingREST) Create(ctx context.Context, obj runtime.Object, createV
 	// nothing to explain it.
 	uniqueScope, scopeDigest, err := scope.ProjectAddressSpaceDigest(id.Name, claim.Spec.Scope, class.Spec.UniqueWithin, "uniqueWithin")
 	if err != nil {
-		_ = tx.Rollback(ctx)
 		metrics.RecordAllocationFailure("ipclaim", "internal", ipFamily, project, org)
 		var missing *scope.MissingRoleError
 		if errors.As(err, &missing) {
@@ -338,9 +345,8 @@ func (r *AllocatingREST) Create(ctx context.Context, obj runtime.Object, createV
 	// Resolving the pool provisions any missing level of the class's chain, and
 	// each level commits on its own — so it runs BEFORE the allocation
 	// transaction opens, not inside it. A pool is durable infrastructure that
-	// outlives the claim that caused it; holding this transaction across the
+	// outlives the claim that caused it; holding a transaction across the
 	// chain would make a herd of first claims serialise behind the slowest.
-	_ = tx.Rollback(ctx)
 	poolKey, err := allocator.ResolvePool(ctx, r.db, class, claim.Spec.Scope)
 	if err != nil {
 		metrics.RecordAllocationFailure("ipclaim", "pool_not_found", ipFamily, project, org)

internal/registry/ipam/ipclaim/storage_postgres_test.go

@@ -429,3 +429,141 @@ func TestCreateDefaultsTheReferenceNameIntoSpec(t *testing.T) {
 		t.Errorf("stored spec.className = %q, want shared", got)
 	}
 }
+
+func TestCreateFillsTheClassAndFamilyBeforeAdmission(t *testing.T) {
+	r, _ := newPostgresREST(t)
+	claim := newClassClaim()
+	claim.Spec.IPFamily = ""
+
+	var admitted ipam.IPClaimSpec
+	validate := func(_ context.Context, obj runtime.Object) error {
+		admitted = obj.(*ipam.IPClaim).Spec
+		return nil
+	}
+
+	obj, err := r.Create(claimCtx(testProject), claim, validate, &metav1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	if admitted.IPFamily != ipam.IPv4 || admitted.ClassName != "standard" {
+		t.Errorf("admission saw className %q ipFamily %q, want standard and IPv4", admitted.ClassName, admitted.IPFamily)
+	}
+	if got := obj.(*ipam.IPClaim).Spec.IPFamily; got != ipam.IPv4 {
+		t.Errorf("stored ipFamily = %q, want %q", got, ipam.IPv4)
+	}
+}
+
+func TestCreateRefusesAFamilyMismatchBeforeAdmission(t *testing.T) {
+	r, db := newPostgresREST(t)
+	claim := newClassClaim()
+	claim.Spec.IPFamily = ipam.IPv6
+
+	admitted := false
+	validate := func(context.Context, runtime.Object) error {
+		admitted = true
+		return nil
+	}
+
+	_, err := r.Create(claimCtx(testProject), claim, validate, &metav1.CreateOptions{})
+	if !apierrors.IsBadRequest(err) {
+		t.Fatalf("Create returned %v, want a bad request", err)
+	}
+	if admitted {
+		t.Error("a mismatched claim reached admission")
+	}
+	if n := countRows(t, db, `SELECT count(*) FROM ipam_objects WHERE kind = 'IPClaim'`); n != 0 {
+		t.Errorf("refused claim left %d rows, want 0", n)
+	}
+}
+
+func markStandardClassScope(t *testing.T, db *pgxpool.Pool, s ipamv1alpha1.AddressScope) {
+	t.Helper()
+	class := &ipamv1alpha1.IPClass{
+		TypeMeta:   metav1.TypeMeta{APIVersion: "ipam.miloapis.com/v1alpha1", Kind: "IPClass"},
+		ObjectMeta: metav1.ObjectMeta{Name: "standard"},
+		Spec: ipamv1alpha1.IPClassSpec{
+			IPFamily: ipamv1alpha1.IPv4, AddressScope: s, DefaultPrefixLength: 24,
+		},
+	}
+	data, err := json.Marshal(class)
+	if err != nil {
+		t.Fatalf("marshal class: %v", err)
+	}
+	if _, err := db.Exec(context.Background(),
+		`UPDATE ipam_objects SET data = $2 WHERE key = $1`,
+		tenant.Identity{Name: testProject}.ResourceKey("ipclasses", "standard"), data); err != nil {
+		t.Fatalf("update class: %v", err)
+	}
+}
+
+func TestCreateCopiesTheAddressScopeFromTheClassBeforeAdmission(t *testing.T) {
+	r, db := newPostgresREST(t)
+	markStandardClassScope(t, db, ipamv1alpha1.AddressScopePublic)
+	claim := newClassClaim()
+	claim.Spec.IPFamily = ""
+
+	var admitted ipam.AddressScope
+	validate := func(_ context.Context, obj runtime.Object) error {
+		admitted = obj.(*ipam.IPClaim).Spec.AddressScope
+		return nil
+	}
+
+	obj, err := r.Create(claimCtx(testProject), claim, validate, &metav1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	if admitted != ipam.AddressScopePublic {
+		t.Errorf("admission saw addressScope %q, want %q", admitted, ipam.AddressScopePublic)
+	}
+	if got := obj.(*ipam.IPClaim).Spec.AddressScope; got != ipam.AddressScopePublic {
+		t.Errorf("stored addressScope = %q, want %q", got, ipam.AddressScopePublic)
+	}
+}
+
+func TestCreateRefusesAnAddressScopeThatContradictsTheClass(t *testing.T) {
+	r, db := newPostgresREST(t)
+	markStandardClassScope(t, db, ipamv1alpha1.AddressScopePrivate)
+	claim := newClassClaim()
+	claim.Spec.AddressScope = ipam.AddressScopePublic
+
+	admitted := false
+	validate := func(context.Context, runtime.Object) error {
+		admitted = true
+		return nil
+	}
+
+	_, err := r.Create(claimCtx(testProject), claim, validate, &metav1.CreateOptions{})
+	if !apierrors.IsBadRequest(err) {
+		t.Fatalf("Create returned %v, want a bad request", err)
+	}
+	if !strings.Contains(err.Error(), "Public") || !strings.Contains(err.Error(), "standard") {
+		t.Errorf("error %q should name both the scope asked for and the class", err)
+	}
+	if admitted {
+		t.Error("a contradicted claim reached admission")
+	}
+	if n := countRows(t, db, `SELECT count(*) FROM ipam_objects WHERE kind = 'IPClaim'`); n != 0 {
+		t.Errorf("refused claim left %d rows, want 0", n)
+	}
+}
+
+func TestCreateLeavesTheAddressScopeEmptyWhenTheClassStatesNone(t *testing.T) {
+	r, _ := newPostgresREST(t)
+
+	var admitted ipam.AddressScope = "unset"
+	validate := func(_ context.Context, obj runtime.Object) error {
+		admitted = obj.(*ipam.IPClaim).Spec.AddressScope
+		return nil
+	}
+
+	obj, err := r.Create(claimCtx(testProject), newClassClaim(), validate, &metav1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	if admitted != "" {
+		t.Errorf("admission saw addressScope %q, want empty", admitted)
+	}
+	if got := obj.(*ipam.IPClaim).Spec.AddressScope; got != "" {
+		t.Errorf("stored addressScope = %q, want empty", got)
+	}
+}

internal/registry/ipam/ipclaim/strategy.go

@@ -15,6 +15,7 @@ import (
 	"sigs.k8s.io/structured-merge-diff/v6/fieldpath"
 
 	"go.miloapis.com/ipam/internal/fieldindex"
+	"go.miloapis.com/ipam/internal/registry/ipam/ipamvalidation"
 	"go.miloapis.com/ipam/internal/scope"
 	"go.miloapis.com/ipam/pkg/apis/ipam"
 )
@@ -83,6 +84,9 @@ func (ipClaimStrategy) ValidateUpdate(_ context.Context, obj, old runtime.Object
 	if n.Spec.IPFamily != o.Spec.IPFamily {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "ipFamily"), "ipFamily is immutable"))
 	}
+	if n.Spec.AddressScope != o.Spec.AddressScope {
+		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "addressScope"), "addressScope is immutable"))
+	}
 	if !equality.Semantic.DeepEqual(n.Spec.PrefixLength, o.Spec.PrefixLength) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "prefixLength"), "prefixLength is immutable"))
 	}
@@ -121,6 +125,7 @@ func validateIPClaim(c *ipam.IPClaim) field.ErrorList {
 		allErrs = append(allErrs, field.NotSupported(specPath.Child("ipFamily"), c.Spec.IPFamily,
 			[]string{string(ipam.IPv4), string(ipam.IPv6)}))
 	}
+	allErrs = append(allErrs, ipamvalidation.AddressScope(c.Spec.AddressScope, specPath.Child("addressScope"))...)
 	// Only the bound that holds for every family. The family-specific one comes
 	// from the class, in EffectivePrefixLength: this runs before the class is
 	// resolved and cannot read it, and assuming IPv4 here rejected a /64 from