Repository navigation
fix(app.yaml): leave routing.priority out so the platform derives one - #4
Closed
Alexanderamiri wants to merge 1 commit into
Closed
Alexanderamiri wants to merge 1 commit into
Alexanderamiri wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
Superseded by #5, which also drops routing.priority. |
Alexanderamiri
added a commit
to javaBin/platform
that referenced
this pull request
Oct 31, 2026
…me, validate compute (#181) ## Problem - **ALB priority collisions (finding 1).** app-template ships `routing.priority: 100`, which cupcake (frosting) already holds. jz-watch run 40079243698 failed at apply with `CreateRule PriorityInUse: Priority '100' is currently in use`. `routing.login` rules take the numbers directly below `routing.priority`, so nearby picks collide too. The docs said to "coordinate via the registry", but the registry has nothing for priorities, and nothing checked them before apply. - **`name` default was inconsistent (part of finding 13).** Only the generator fell back to the repo name. detect.yml emitted `app_name=""`, the CLI rejects a missing name, and a repo name like `2027.javazone.no` fails `NAME_RE` anyway. - **Compute values were not validated (part of finding 14).** An invalid Fargate cpu/memory pair, port or desired_count only failed at apply. ## Change - `scripts/generate-modules.py` - `routing.priority` is optional. When it is left out, `derived_priority(team, name)` takes sha256 of `team/name` and maps it to one of 4454 slots of 11 in 1000-50000. The host rule sits at the top of its slot and up to 10 login rules sit below it, inside the same slot. The value is printed as a `::notice`. `domain:` no longer needs a priority. - An explicit `routing.priority` must be 1-999, so it can never land in the derived range. Explicit values are passed through unchanged. - `name` is required, with a clear message. The message for a dotted name gives an example fix (`2027.javazone.no` becomes e.g. `javazone-2027`). The `APP_SERVICE` repo-name fallback is removed. - `compute.cpu` and `compute.memory` must be whole numbers and a valid Fargate task size; the error lists the memory values allowed for that cpu. `port` must be 1-65535 and `desired_count` 0 or more. - New `scripts/check-alb-priorities.py`, run in tf-plan after the plan when it has changes, using the team's read-only plan credentials (`ReadOnlyAccess` covers `elbv2:DescribeRules`). It takes every `aws_lb_listener_rule` the plan creates or moves and compares its priority with the live rules on that listener. - Priorities the plan frees in the same run (delete-then-create, or the app's own rule moving) count as free. - On a conflict it fails with `::error`, naming the host that holds the priority and suggesting the lowest free number that leaves 10 free below it. - If the plan or the live rules cannot be read, it only warns, because apply still catches a conflict. - `detect.yml` fails when `app.yaml` has no `name`. `expand-terraform.yml` no longer passes `APP_SERVICE`. - Docs: `docs/app-yaml-reference.md` (name, compute table, routing.priority, login priorities, domain), `docs/reusable-workflows.md` and `CLAUDE.md`. ## No rule churn These are the live rules on `javabin-platform-alb`:443: | Priority | Host | |---|---| | 10 | password | | 100 | cupcake | | 101 | cupcake-backend | | 150 | watch | | 200 | 2027 | | 900 | test | Every app.yaml in the org (cupcake, frosting, jz-watch, 2027.javazone.no, platform-test-app, app-template) sets `name` and an explicit priority below 1000. Their generated priorities are unchanged; I checked by generating each app.yaml from GitHub with this branch. ## Verification - `python3 -m unittest discover -s scripts/tests`: 153 tests OK, including new tests for name, priority range, derived priority (stability, range, room for 10 login rules, `domain:`), compute validation, and `test_check_alb_priorities.py`. - `python3 -m unittest discover -s terraform/lambda-src/tests`: 219 tests OK. - Ran the check against the live listener rules (read-only `describe-rules`): - priority 100 is reported as held by `cupcake.javazone.no`, with suggestion 21; - the derived priority for cupcake/cupcake-backend (2297) passes. - `actionlint` on the changed workflows: the only finding is the existing SC2129 style note in detect.yml. ## Follow-ups (other repos) - javaBin/app-template#4 drops `priority: 100` from the template `app.yaml` (merge after this PR). - javaBin/javabin-cli#9 removes the priority prompt from `javabin init` (no `priority` written, so new apps take the derived one). - `docs/getting-started.md` in javaBin/docs still shows `priority: 120`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The template's
app.yamlsetsrouting.priority: 100, whichcupcake.javazone.noalready holds on the shared ALB listener. Every new app made from the template collides: jz-watch run 40079243698 failed at apply withCreateRule PriorityInUse: Priority '100' is currently in use. The comment also says 1-50000, but explicit priorities are now limited to 1-999.Change
app.yaml:priorityis commented out, with a note that the platform derives a unique one and that explicit values (1-999) are only for keeping an existing rule or moving off a collision.CLAUDE.md: theroutingline says the same.Depends on javaBin/platform#181, which adds the derived priority (hash of team/name into 1000-50000 with room for login rules) and the tf-plan conflict check. Merge that first.
Verification
Loaded the new
app.yamlwith the generator from platform#181:validate()passes, and the derived priority forteam/my-serviceis 6587, which no live rule uses.🤖 Generated with Claude Code