Repository navigation
feat: v0.8.9 UDP ports, partial PUT, CI OIDC providers, Access details - #1402
Merged
Merged
Conversation
The token after # never reaches the server or its logs. Re-print it with setup-token.
DNS and certificate for a new domain can add about a minute.
It names the branch and the push command; --allow-unpushed overrides.
Exits like deploy and prints the failure reason; --no-wait returns early.
Pinned image with a /healthz check; no login, so restrict access.
Quick start folds into the first deploy card as import and Git links.
UDP publishing ships first; capability and sysctl grants wait on a threat model.
Pinned image, generated credentials and a real health probe.
Pinned image, generated credentials and a real health probe.
Shows the version gap, release notes, an update action and a per-version dismiss.
Single container with data, config and cache volumes.
The update dot now lives on the version line; the logo badge shows health only.
Pinned images, generated password and encryption key, health probe.
Headscale runs env-only; Authelia hashes a generated admin password at boot.
Adds a short not-secure note that links to the Enable HTTPS card.
Compose ports with /udp or protocol: udp now parse and are kept as host-published ports beside the routed tcp port.
UDP ports bind with the service's bind address, recreate on change, and are refused for services placed on a remote node.
Apps list their published_ports; a compose deploy fails with a coded error on a taken udp port, a missing host port or a remote node.
…lia fallback Open Notebook works behind one public port. Authelia boots without a domain.
Status rows, verified list and a catalog section with security notes.
…rted Adds the UDP ports page, fixes the compose subset table and links it in the nav.
The API client carries host-published TCP and UDP ports.
The address pill already opens the app, so the duplicate Open app button is gone.
Adds the published_ports wire shape with a host-port/protocol formatter.
The card collapses to a line and a button; the inline link is gone.
JSON output carries published_ports.
Only set behind a TLS-terminating proxy, where plain-http routes exist.
Each port gets a protocol badge beside the host port; apps without extras look unchanged.
Mirrors the server rule and prefers its outcome when present.
Deploy list, cancel and failed responses now carry outcome, outcome_reason and the crash-loop threshold; the CLI prints the reason.
An env-only save no longer drops command, domains, labels or limits. Send an empty value or null to clear a field.
Generated logins stay reachable from the overview and the header menu; the wizard grid deploy shows the same result dialog.
Stack trusts take the same provider, issuer and protected-ref rules as app trusts.
apps github-actions token and trust keep working and are marked deprecated.
The form shows each provider's fields, the issuer, and the protected-refs warning.
Stages and the first-app card prefer the server outcome and threshold, with the local rule as fallback.
…ate app Apps now record the template they were deployed from and return it as template_id.
Clearing now sends explicit null or empty; decodeAppUpdate joins the ref guard allowlist.
Brings in the v0.8.8 merge commit so the v0.8.9 PR diff is only v0.8.9 work.
|
Important Review skippedToo many files! This PR contains 371 files, which is 271 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files.
 |
github-actions
Bot
added
area/ci
Contributor
glinr-botA protected condition applies, so a person decides.
No action taken. A person reviews this one. |
thegdsks
marked this pull request as ready for review
October 28, 2026 20:19
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+70
to
+84
| have := map[store.PublishedPort]bool{} | ||
| for _, p := range ports { | ||
| have[store.PublishedPort{ContainerPort: p.ContainerPort, HostPort: p.HostPort, Protocol: bindingProtocol(p)}] = true | ||
| } | ||
| want := map[store.PublishedPort]bool{} | ||
| for _, p := range desired.PublishedPorts { | ||
| key := store.PublishedPort{ContainerPort: p.ContainerPort, HostPort: p.HostPort, Protocol: p.EffectiveProtocol()} | ||
| want[key] = true | ||
| if !have[key] { | ||
| return true | ||
| } | ||
| } | ||
| // Only non-tcp leftovers count: tcp bindings also come from the main port and app streams. | ||
| for k := range have { | ||
| if k.Protocol != store.PortProtocolTCP && !want[k] { |
There was a problem hiding this comment.
For a UDP-only app, changing bind_address from public to private leaves the public socket open. The main-port check skips Port == 0, and publishedPortsDrifted compares port numbers without checking addresses. Removing a TCP entry from published_ports also leaves its socket open because this check ignores TCP leftovers.
Compare the full expected bindings, including addresses, and exclude only known main-port and stream bindings.
How this was verified: The container name stays unchanged on these edits, and neither port check requests a replacement for these stale bindings.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/reconcile/application/port_drift.go
Line: 70-84
Comment:
**Old public ports stay open**
For a UDP-only app, changing `bind_address` from public to private leaves the public socket open. The main-port check skips `Port == 0`, and `publishedPortsDrifted` compares port numbers without checking addresses. Removing a TCP entry from `published_ports` also leaves its socket open because this check ignores TCP leftovers.
Compare the full expected bindings, including addresses, and exclude only known main-port and stream bindings.
**How this was verified:** The container name stays unchanged on these edits, and neither port check requests a replacement for these stale bindings.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| return refusePort(http.StatusBadRequest, codePublishedPortRemoteNode, | ||
| fmt.Sprintf("host port %s can only be published on the control plane's own node, service %q is placed on node %q", p, svc.Name, node)) | ||
| } | ||
| want[portKey(p)] = svc.Name |
There was a problem hiding this comment.
A fresh Compose deploy with two services declaring 5353:53/udp passes this check. Each service passes its own duplicate check, then the second assignment overwrites the first service in want. The later database scan skips both services, so conflicting settings are saved and one service cannot bind its socket.
Reject a key already claimed by another service in next before saving the batch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/api/app_published_ports.go
Line: 257
Comment:
**Duplicate port claims pass**
A fresh Compose deploy with two services declaring `5353:53/udp` passes this check. Each service passes its own duplicate check, then the second assignment overwrites the first service in `want`. The later database scan skips both services, so conflicting settings are saved and one service cannot bind its socket.
Reject a key already claimed by another service in `next` before saving the batch.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+1017
to
+1021
| if shmDrifted(state, desired) { | ||
| if err := c.removeContainer(ctx, state.ID); err != nil { | ||
| return replicaOutcome{reason: reasonShmDriftRecreate}, fmt.Errorf("remove %q to apply its changed shm_size: %w", target, err) | ||
| } | ||
| state = nil |
There was a problem hiding this comment.
A healthy service using recreate bypasses ensureReplicaRunning, so it never reaches the new shmDrifted check. Redeploying the same Compose image with only a changed shm_size also leaves the container name unchanged: composeConfigChanged does not include shared memory. The app therefore keeps its old /dev/shm size after a successful save.
Check this difference before the recreate fast return, or make a shared-memory edit trigger a restart.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/reconcile/application/controller.go
Line: 1017-1021
Comment:
**Shared-memory edits never apply**
A healthy service using `recreate` bypasses `ensureReplicaRunning`, so it never reaches the new `shmDrifted` check. Redeploying the same Compose image with only a changed `shm_size` also leaves the container name unchanged: `composeConfigChanged` does not include shared memory. The app therefore keeps its old `/dev/shm` size after a successful save.
Check this difference before the `recreate` fast return, or make a shared-memory edit trigger a restart.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+22
to
+26
| ports: | ||
| - "21115:21115" | ||
| - "21116:21116" | ||
| - "21116:21116/udp" | ||
| - "21117:21117" |
There was a problem hiding this comment.
These TCP host ports do not survive Compose translation. splitPorts keeps only the first TCP container port as the routed port and adds only UDP entries to PublishedPorts. This template produces a random host binding for TCP 21115 plus UDP 21116, but no TCP 21116 or 21117.
Setting a public bind and opening the documented firewall ports cannot make the relay reachable. Add a supported path for all required direct TCP bindings before offering this template.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/catalog/templates_catalog_batch_rustdesk.go
Line: 22-26
Comment:
**RustDesk TCP ports are missing**
These TCP host ports do not survive Compose translation. `splitPorts` keeps only the first TCP container port as the routed port and adds only UDP entries to `PublishedPorts`. This template produces a random host binding for TCP 21115 plus UDP 21116, but no TCP 21116 or 21117.
Setting a public bind and opening the documented firewall ports cannot make the relay reachable. Add a supported path for all required direct TCP bindings before offering this template.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+42
to
+49
| keep("host_port", func() { req.HostPort = existing.HostPort }) | ||
| keep("published_ports", func() { req.PublishedPorts = existing.PublishedPorts }) | ||
| keep("bind_address", func() { req.BindAddress = existing.BindAddress }) | ||
| keep("domains", func() { req.Domains = existing.Domains }) | ||
| keep("env", func() { req.Env = existing.Env }) | ||
| keep("resources", func() { req.Resources = existing.Resources }) | ||
| keep("health", func() { req.Health = existing.Health }) | ||
| keep("hooks", func() { req.Hooks = existing.Hooks }) |
There was a problem hiding this comment.
UpdateApp cannot send clears for fields still marked omitempty. Getting an app, setting HostPort = nil or PublishedPorts = [], and passing it back omits those keys. The new merge then restores the old values despite returning success. Empty domains and nil resource, health, or hook settings have the same problem.
Give updates an explicit way to send empty values, as this PR already does for Env.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/api/apps_update_preserve.go
Line: 42-49
Comment:
**Go updates cannot clear settings**
`UpdateApp` cannot send clears for fields still marked `omitempty`. Getting an app, setting `HostPort = nil` or `PublishedPorts = []`, and passing it back omits those keys. The new merge then restores the old values despite returning success. Empty domains and nil resource, health, or hook settings have the same problem.
Give updates an explicit way to send empty values, as this PR already does for `Env`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+118
to
+122
| const { control, register, handleSubmit, watch, setError, formState } = | ||
| useForm<PortFormInput, unknown, PortFormOutput>({ | ||
| resolver: zodResolver(portSchema), | ||
| values: toFieldValues(app), | ||
| resetOptions: { keepDirtyValues: true }, |
There was a problem hiding this comment.
UDP-only apps have port: 0, but portSchema requires a positive HTTP port before onSubmit runs. Changing a published UDP port or switching its bind to public therefore fails with a port error unless the user invents an HTTP port.
Allow zero for apps without a routed port, matching the API.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/PortEditor.tsx
Line: 118-122
Comment:
**UDP-only settings cannot save**
UDP-only apps have `port: 0`, but `portSchema` requires a positive HTTP port before `onSubmit` runs. Changing a published UDP port or switching its bind to public therefore fails with a port error unless the user invents an HTTP port.
Allow zero for apps without a routed port, matching the API.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| }: TemplateDescriptionProps) { | ||
| const { t } = useTranslation('templates') | ||
| const text = description?.trim() | ||
| if (!text || text === slogan?.trim()) return null |
There was a problem hiding this comment.
Manifest.Template sets Description and Slogan to the same text, so this condition hides notes for every manifest-backed template. That avoids repetition where the slogan is already shown, but TemplateAccessCard does not show it elsewhere. Access details therefore loses notes such as Authelia's user-file location and cookie-domain setup.
Allow the access card to show the description even when it equals the slogan.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/templates/TemplateDescription.tsx
Line: 16
Comment:
**Access details hide setup notes**
`Manifest.Template` sets `Description` and `Slogan` to the same text, so this condition hides notes for every manifest-backed template. That avoids repetition where the slogan is already shown, but `TemplateAccessCard` does not show it elsewhere. Access details therefore loses notes such as Authelia's user-file location and cookie-domain setup.
Allow the access card to show the description even when it equals the slogan.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
UpdateApp still sends an empty env so the last key can be cleared.
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.
Summary
Everything for v0.8.9, stacked on the v0.8.8 merge.
outcomeandoutcome_reason, the dashboard uses them.PUT /appskeeps omitted fields; an explicit empty value clears them.template_id(migration 3041).apps ci token|trust|workflow, dashboard trust form.Verification
Gate on the pre-merge tip: build, vet,
go test ./internal/api, the short Go batch, migration and CI-plan scripts, 4 vitest shards, tsc, logos all pass. An independent review of thePUT /appsmerge found no authz hole.Not verified on a real host
Authelia start script, UDP with DOCKER-USER, the
PUT /appsmerge, migration 3040 on existing stack trusts, GitLab and CircleCI against the real providers.Draft first so the cheap checks run; flip to ready for the full suite.
This PR is not ready to merge because the previous public-port exposure and deployment failures remain.
Findings
Fix with agent prompt
Summary
This PR adds UDP published ports, partial app updates, more CI identity providers, and template access details.
Reviews (2) · Last reviewed commit: "fix(mcp): app tool output no longer fail..." · Reviewed by Greptile