Add top-level scale field to func.yaml

[aliok]

This is Part 1 of 2 splitting #4051 into reviewable pieces. Together with the follow-up Part 2, this PR supersedes #4051.

• Part 1 (this PR): the top-level scale field and its migration.
• Part 2 (follow-up): Kafka consumer-lag scaling (the scale.keda block: scaler types and triggers).

Part 1 contains no KEDA scale-config surface — it's the foundational schema change that Part 2 builds on.

What changes

Autoscaling configuration moves from the nested deploy.options.scale to a top-level scale field:

scale:
  min: 0                 # deployer-agnostic replica lower bound
  max: 10                # deployer-agnostic replica upper bound
  kpa:                   # Knative Pod Autoscaler tuning (consumed by the knative deployer)
    metric: concurrency  # concurrency | rps
    target: 100
    utilization: 70

• scale.min / scale.max — deployer-agnostic replica bounds (0 … 2147483647).
• scale.kpa — metric / target / utilization, consumed only by the knative deployer (and the unset default, which behaves as knative). On the raw and keda deployers it is ignored with a warning at deploy time rather than rejected.

deploy.options now exposes only resources.

Details

• Migration 0.38.0 (migrateScaleToTopLevel) lifts the old flat deploy.options.scale (min/max/metric/target/utilization) to the new top-level scale, folding metric/target/utilization into scale.kpa. It is a deployer-agnostic plain move: the flat fields were only ever read by the knative deployer, so they are lifted for every deployer and simply ignored (with a warning) where unused, rather than being silently dropped for raw/keda.
• Backward compatibility — old func.yaml files are handled by the migration, which reads the pre-migration deploy.options.scale (and the pre-0.34 top-level options.scale) directly from the raw YAML on disk. deploy.options no longer carries a scale field at all, so a stale deploy.options.scale left in a current-spec file surfaces as an unknown key rather than being silently accepted.
• Deployers — the raw, knative, and keda deployers now read f.Scale.
• Schema + docs — regenerated schema/func_yaml-schema.json and updated docs/reference/func_yaml.md.

Breaking changes

• Options.Scale removed from the functions package. func.yaml files are migrated automatically, so end users are unaffected, but Go/library consumers referencing f.Deploy.Options.Scale will no longer compile — read f.Scale instead.

Behavior notes for reviewers

• Defaults when scale is unset differ per deployer (unchanged in spirit, just relocated): raw → 1 replica; keda → min 1 / max 10; knative → Knative's own autoscaling defaults.
• scale is validated against the effective deployer — the intent (f.Deployer) when set, otherwise the last-deployed deployer (f.Deploy.Deployer) — so the deployer-specific rules (e.g. keda's max >= 1) apply consistently across the CLI, the tekton pipeline path, and library callers. replicaBounds re-checks at deploy time as a backstop.
• Switching deployers on an already-deployed function is still refused by the existing deployers.ValidateSwitch gate (func delete first). Part 1 doesn't change that; the analogous scaler/trigger switch gate lands in Part 2.

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aliok
Once this PR has been reviewed and has the lgtm label, please assign matejvasek for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot review overview

🟡 Changes recommended

Legacy KPA settings can be lost during migration, and some invalid KEDA bounds bypass deployer-level validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 · 1

Open (4)

• Preserve legacy top-level scale tuning during migration · New
• Preserve fractional minimums in the generated schema · New
• Reject explicit scale.max values below KEDA's minimum · New
• Remove the inverted KEDA scale validation branch · New

[gauron99]

Few migration simplifications and some leftover code to remove and we should be good to go!

[gauron99]

This keeps a field in the live Function only so migrations can use it. Migrations should read old shapes from their own snippet of the raw YAML, not from the current struct. migrateGitToSource does that for build.git, which is no longer on BuildSpec. migrateScaleToTopLevel already decodes its own oldScale from disk, so it doesn't need this field.

Keeping it has a real cost: a current-spec file that still has deploy.options.scale is accepted silently. The 0.38.0 migration does not run, no deployer reads it, Validate() no longer checks it, and Write() keeps it. The unknown-field warning in Migrate() never fires either, because the key still maps to a field.

Repro: specVersion: 0.38.0 with deploy.options.scale: {min: -5, max: 3} gives f.Scale == nil, Validate() == nil, and the block is written back unchanged.

Anyone copying an older example gets no scaling and no error. Please drop Options.Scale, together with the in-memory fallback in the migration that reads it. Then the strict unmarshal warns on the stale key.

[aliok]

Done in eabeb16d. Dropped Options.Scale entirely and made migrateScaleToTopLevel self-contained: it now reads deploy.options.scale from its own disk snippet (like migrateGitToSource does for build.git), falling back to the pre-0.34 top-level options.scale — which is still on disk when the migration runs, since each migration re-reads the original file and migrateToSpecsStructure no longer stages it in-memory.

Reading that block with the old shape (rather than the current ScaleOptions) also recovers the pre-0.34 flat metric/target/utilization fields, so it incidentally fixes the pre-0.34 half of the Copilot drop finding too.

Also removed the in-memory fallback branch and the now-unnecessary in-memory clear. As you noted, the real win: an at-spec func.yaml with a stale deploy.options.scale now surfaces via the unknown-fields warning instead of being silently accepted. Added TestUnknownFieldsWarnOnStaleScale to lock that in.

[gauron99]

Please drive these through NewFunction(root) on the inline func.yaml instead of calling migrateScaleToTopLevel with a hand-built Function. Hand-built states that can't happen in practice (e.g. empty Deploy.Deployer with a value on disk) are what the write-back at line 536 was written for.

[aliok]

Done in fa7d20aa. All TestMigrateScaleToTopLevel subtests now go through NewFunction(root) on inline func.yaml (via a small newFn helper) instead of hand-building a Function and calling migrateScaleToTopLevel directly. SpecVersion assertions became LastSpecVersion(), and the pre-0.34 lift case uses specVersion: 0.33.0 so it actually flows through migrateToSpecsStructure too.

Dropped the "empty Root" subtest — it was the only hand-built state (no backing file), un-drivable through NewFunction, and it only exercised the write-back that's now gone (see the other thread).

[gauron99]

This is the only consumer of scale.kpa, and the mapping to annotations has no unit test. The only coverage is TestInt_Deploy_WithOptions, which needs a cluster. A small unit test calling setServiceOptions with KPA set, KPA nil, and scale nil, asserting which annotations are added and removed, would cover it.

[aliok]

Done in 75a2e2c4. Added TestSetServiceOptions_KPAAnnotations in pkg/knative/deployer_scale_test.go, a pure unit test (no cluster) covering the three shapes:

• KPA set → min/max plus metric/target/utilization annotations are added with the expected values.
• KPA nil (min/max still set) → the three kpa annotations are removed, min/max kept.
• scale nil → the whole block is skipped, so pre-existing autoscaling annotations are left untouched.

TestInt_Deploy_WithOptions still covers the cluster path.

[gauron99]

This write-back is a no-op on every real path. NewFunction unmarshals the same bytes into f, and deploy.deployer/deploy.expose keep their yaml tags, so the values are already there. hasInitializedFunction calls Migrate with Root == "", so nothing is read from disk at all. expose has nothing to do with scale either.

It looks carried over from #4051, together with the #4054 NOTE around line 448. Please drop the block, the NOTE, and the subtests that only cover it:

• "old deploy.deployer/deploy.expose keys populate the observed-state fields"
• "migration never touches f.Deployer intent"
• "no-op when neither old deployer/expose key is present"
• "empty Root does not touch the in-memory deployer/expose value"

This holds on its own. If the migration becomes a plain move (see the comment on line 503), the rest of the deployer handling here goes as well.

[aliok]

Done in fa7d20aa. Removed the write-back block and the deploy.deployer/deploy.expose disk read that fed it — you're right it was a no-op on every real path: NewFunction unmarshals those keys onto f (via their yaml tags) before migrations run, and hasInitializedFunction migrates with Root == "" so nothing is read from disk at all. Either way the values are already on f. expose was unrelated to scale, and the #4054 NOTE went with it.

One thing kept: the kpa-gating still needs the observed deployer for pre-#3953 files (deployer recorded only under deploy.deployer, so f.Deployer is empty) — otherwise the legacy flat fields would be lifted into a scale.kpa that raw/keda rejects. But per the same reasoning, it now reads f.Deploy.Deployer directly (already populated by the primary unmarshal) instead of re-reading disk, so the redundant read is gone with no behavior change.

[aliok]

Update: the one piece I said I kept here — the deployer-gating reading f.Deploy.Deployer — has now been removed too (919a65e1). Adopting your other comment fully (plain move + scale.kpa ignored-with-warning) means there's no reason to gate the lift on deployer, so the migration no longer inspects the deployer at all. The flat fields are lifted into scale.kpa for every deployer; raw/keda just warn-and-ignore at deploy time.

[gauron99]

The flat metric/target/utilization were only ever read by the knative deployer (setServiceOptions). raw reads only min and keda only min/max; both ignore the rest. So the migration can stay a plain move: lift the whole old block to scale (min/max as is, the three flat fields into scale.kpa) for every deployer, without looking at the deployer at all.

To keep the behavior raw/keda users have today, scale.kpa on raw/keda should be ignored with a warning rather than rejected, the same way warnExposeIgnore in cmd/deploy.go handles expose on knative. That means the scale.kpa requires deployer: knative rule in ValidateScale becomes a warning.

This removes from the migration: validKPADeployer and the intent/observed deployer logic, the deploy.deployer/deploy.expose read from disk, oldScale.KPA (no released func ever wrote deploy.options.scale.kpa), and the subtests about the deployer check. It also fixes the current silent drop of the flat fields from func.yaml for raw/keda.

[aliok]

Done in 919a65e1 — adopted this fully. migrateScaleToTopLevel is now a plain move: min/max as-is and the flat metric/target/utilization into scale.kpa, for every deployer, with no deployer inspection at all. That drops validKPADeployer and the intent/observed logic, the oldScale.KPA field (no released func ever wrote deploy.options.scale.kpa), and it fixes the silent drop of those flat fields for raw/keda.

scale.kpa requires deployer: knative in ValidateScale is gone; scale.kpa on raw/keda is now ignored-with-warning at deploy time via warnScaleKpaIgnore in cmd/deploy.go, mirroring warnExposeIgnore. The kpa values are still validated regardless of deployer, so a bad metric/target/utilization is still caught.

Two follow-on effects, noted on their own threads: the deployer-gating I'd kept is now removed entirely, and the f.Deployer = deployer recovery in the tekton provider is removed — its only purpose was to make that rule hard-fail. The migration subtests that asserted "no scale.kpa for non-knative" are inverted accordingly.