Skip to content

fix(lint): accept configs the platform accepts, and reject web build egress - #242

Merged
pjcdawkins merged 4 commits into
mainfrom
fix/lint-schema-platform-parity
Oct 16, 2026
Merged

pjcdawkins merged 4 commits into
mainfrom
fix/lint-schema-platform-parity

Conversation

@pjcdawkins

Copy link
Copy Markdown
Contributor

Fixes cases where upsun lint and editors using the schema reject configs that the platform accepts with the Upsun settings, reported on upsun/meta#220. Each was checked against the platform's validation, by validating one config per schema property set to null, and one per object with a .-prefixed key added.

  • null: explicit null on optional fields is treated as unset. It is now allowed on the 90+ properties where the platform allows it (e.g. web.size, location passthru and root, operation timeout, egress port, route id/primary, HSTS options, task egress). Some differ by site: mounts: null is accepted on web and workers (no override) but not on the application, so null is added where the property is used rather than in the shared definition.
  • .-prefixed keys (YAML anchors, notes) are ignored in every mapping with fixed keys, and in the applications, services and routes sections. They are now allowed there via patternProperties, except in stack, which the platform checks itself, and dropped when merging and decoding. They are still rejected where the platform reads them as names (e.g. a cron or mount named .note) and directly under tasks. A schema test checks that every closed object allows them.
  • request_buffering.max_request_size accepts an integer (bytes).
  • dynamic_compression is accepted on locations and rules.
  • required: [applications] is dropped. Files in .upsun/ are merged and editors validate each file on its own, so routes.yaml beside config.yaml was rejected. No Go check replaces it: a project without applications (e.g. only redirect routes) is valid, and routes to missing applications are already reported.
  • container_profile accepts any name, as profiles are defined per project. The four standard profiles are listed in examples, so editors still suggest them.

Also:

  • web.egress.build is rejected, as on workers: build-phase egress is only read from the application. worker_egress is renamed container_egress and used for both. This replaces the warning added in fix(lint): accept per-container overrides and fix invalid schema defaults #234. The platform accepts this override and ignores it, so this is stricter than the platform.
  • Each $ref with sibling keywords is moved into allOf (or anyOf with null), as draft-07 ignores keywords beside $ref, so the "Overrides the application's value for this container." descriptions were never shown. A schema test checks this.
  • gojsonschema's "Must validate at least one schema (anyOf)" error is omitted when the errors of the closest branch are reported for the same field, which the null alternatives would otherwise add to many errors.

These will be synced to resources/validation/upsun.json in upsun/meta.

🤖 Generated with Claude Code

@upsun-dispatch upsun-dispatch Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Changes suggested — 🟡 1 warning · 🔵 1 minor point

🔍 Full review · 8 files reviewed

Verification
  • DecodeConfig drops .-prefixed entries only from top-level sections other than tasks, so .note under tasks still reaches the Go checks and fails, as the new test expects.
  • mergeConfigFiles skips .-prefixed section names before the duplicate check. Two files can each define .base without a duplicate-key error, and no sources entry is created for it.
  • CheckContent returns early on schema errors, so crons: {.note: x} never reaches DecodeConfig's struct decode, where a string would fail to decode into the cron struct.
  • With the {type: null} alternatives, an invalid value still reports the error from the real branch (e.g. egress.runtime: Expected: object), and the anyOf error is suppressed. I checked this by running it on sample configs.

This PR adds TestLint_PlatformLenience, new cases in TestLint_ContainerFeatures, TestMergeConfigFiles_DotPrefixedNames and the schema-wide TestSchemaKeywords. They run in CI via make test-go in .github/workflows/ci.yml. No test covers the error output for an invalid container_profile override, which is where the extra allOf message shows up.

Review details
  • Commit: a2a2071
  • Model: claude-opus-5-5

Review 1 of 10 for this pull request · View the full run

Comment thread internal/lint/yaml.go Outdated
Comment thread internal/lint/yaml.go Outdated
pjcdawkins added a commit to upsun/meta that referenced this pull request Oct 15, 2026
Copies the CLI's schema, which now accepts configs the platform accepts:

- Explicit null on optional fields, where the platform treats it as unset.
- Keys starting with "." in mappings with fixed keys and in the
  applications, services and routes sections (YAML anchors and notes).
- An integer `request_buffering.max_request_size`, and
  `dynamic_compression` on locations and rules.
- Files without `applications`, as files in .upsun are merged.
- Any container profile name, as profiles are per project.

The web `egress.build` rejection and the `allOf` wrapping of `$ref`s are
now in the CLI's copy too.

The runtime and service version `$ref`s, the `composable:`/`docker:`
pattern and the stack `maxItems: 0` are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pjcdawkins and others added 3 commits October 15, 2026 16:53
…egress

Checked against the platform's validation with the Upsun
settings, by validating one config per schema property set to null, and one
per object with a "."-prefixed key added.

- Explicit null on optional fields is treated as unset: allow it on the 90+
  properties where the platform does. Some differ by site (e.g. `mounts: null`
  is accepted on web and workers, as no override, but not on the
  application), so null is added where the property is used.
- Keys starting with "." are ignored in mappings with fixed keys, and in the
  applications, services and routes sections. Allow them there, except in the
  stack, and drop such entries when merging and decoding. They are still
  rejected where the platform reads them as names (e.g. a cron or mount
  named ".note") and in tasks.
- Accept an integer `request_buffering.max_request_size`, and
  `dynamic_compression` on locations and rules.
- Drop `required: [applications]`: files in .upsun are merged and editors
  validate each one, and a project without applications (e.g. only redirect
  routes) is valid.
- Allow any container profile name: profiles are per project. The known ones
  are listed as examples.
- Reject `egress.build` on web, as on workers (`worker_egress` is renamed
  `container_egress`): build-phase egress is only read from the application.
  This replaces the warning.
- Move each `$ref` with sibling keywords into `allOf` (or `anyOf` with null),
  as draft-07 ignores keywords beside `$ref`, so the override descriptions
  were never shown.
- Omit gojsonschema's "Must validate at least one schema (anyOf)" error when
  the errors of the closest branch are also reported.

A schema test checks that no keyword is beside a `$ref` and that closed
objects allow "."-prefixed keys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The allOf wrappers added for $ref descriptions made gojsonschema add
"Must validate all the schemas (allOf)" beside the real error, e.g. for an
invalid container_profile override. Also correct the comment on the order of
gojsonschema's errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pjcdawkins
pjcdawkins force-pushed the fix/lint-schema-platform-parity branch from 5c8f2e0 to 1b29a1f Compare October 15, 2026 15:53
@pjcdawkins
pjcdawkins enabled auto-merge (squash) October 16, 2026 11:15
@pjcdawkins
pjcdawkins merged commit 8d81ccd into main Oct 16, 2026
7 checks passed
@pjcdawkins
pjcdawkins deleted the fix/lint-schema-platform-parity branch October 16, 2026 11:23
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.

1 participant