fix(webhook): resolve matcher order entirely in Terraform

[vegardx]

Reworked per @edersonbrilhante's suggestion: the ordering now happens once, in webhook.tf.

What changed

Ordering was split across two places. webhook.tf sorted by priority, then the dispatcher re-sorted to put exact and bidirectional matchers first. Both are folded into the Terraform sort key, and the dispatcher consumes the list as-is:

format("%d-%010d-%s",
  (v.matcherConfig.bidirectionalLabelMatch || v.matcherConfig.exactMatch) ? 0 : 1,
  v.matcherConfig.priority, k)

This also removes an in-place sort of the config held on the ConfigLoader singleton. That object outlives a single invocation, so one event's dispatch could reorder what the next event saw on a warm container. Idempotent in practice, but it made order depend on invocation history.

On the padding width

The pad has to be at least as wide as the value, or lexicographic sorting stops matching numeric sorting. With %03d, priority 1000 sorted ahead of the 999 default, because '1' < '9'.

A range validation was supposed to prevent that, but it dereferences a map as though it were an object:

condition = try(var.runner_matcher_config.matcherConfig.priority, 999) >= 0 && ...

runner_matcher_config is a map(object(...)), so .matcherConfig looks up a key of that name, never finds one, and try() falls back to 999. The condition has always evaluated 999 >= 0 && 999 < 1000. It has never rejected anything since it was added in #3591.

Widening the pad makes the range a non-issue, so this drops the validation rather than repairing it. Ordering is unchanged for any config within the previously documented 0-999 range — verified, in-range values sort identically under both widths.

Incidentally, the original design in #3590 specified %05d; the implementation shipped %03d.

Tests

Ordering coverage moves to modules/webhook/tests/matcher_order.tftest.hcl, since the dispatcher no longer decides it. Two cases: strict-before-loose-then-priority, and a priority of 1000 sorting after the 999 default. Both fail if the strict component is dropped from the key or the pad is narrowed back to %03d.

The three dispatcher tests that fed an unsorted config now use the order Terraform actually emits.

Closes #5133.

[Copilot]

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

[edersonbrilhante]

Would make more sense to move the whole sorting to webhook.tf?

Something like this

  runner_matcher_config = {
    for k, v in var.runner_matcher_config :
    format("%d-%03d-%s", (v.matcherConfig.bidirectionalLabelMatch || v.matcherConfig.exactMatch) ? 0 : 1, v.matcherConfig.priority, k) => merge(v, { key = k })
  }

The config is saved in ssm and loaded by lambda.
Tf code is sorting the priority, then ts code sorting the bidirectionalLabelMatch/exactMatch. Maybe I am missing some context, but for me makes more sense to combine everything in tf.

@Brend-Smits @guicaulada What you folks think?

[vegardx]

Agreed, your key gives the same order. Array.sort is stable, so the two stage sort already yields strict then priority; yours does it in one place. I'll update this PR to do that.

One addition while editing that line: I'd pad wider than %03d. The priority validation has never worked (var.runner_matcher_config.matcherConfig looks up a map key that doesn't exist, so try() always falls back to 999), so out of range values get through, and a 4 digit value can sort ahead of a 3 digit one. 1000 lands before 999. Widening the pad makes that correct and lets us delete the dead validation rather than repair it. Ordering is unchanged for anything in 0-999.

[edersonbrilhante]

Why 010d?

[vegardx]

No good reason, honestly. 10 was just "comfortably wider than any priority anyone would realistically set" and I never went back to justify it. That slipped by me.

The only thing the width actually needs to guarantee is that string sorting matches numeric sorting, so it has to be at least as wide as the largest allowed priority. Happy to tie it to a real bound (there's an old validation on priority that was meant to cap it), or just narrow it if you'd prefer.