Repository navigation
fix: preserve Settings SSH policy and prevent gateway resize crashes - #11
Merged
Merged
Conversation
Signal terminal-size queue shutdown separately from its data channel so late or concurrent window requests cannot panic the gateway. Release blocked operations, reject future resizes, and make Stop idempotent without locking across a send. Add regression coverage through real execution completion and OnWindow in connection, persistent, and session modes. Reproduced the original send-on-closed-channel panic; 50 race-enabled repetitions, vet, and gateway build pass. Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
Return the capture helper error last and explain the internal-package test annotation, following existing regression-test conventions. Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
Port regression-tested single-start, signal-policy, connection readiness rollback, and mode-aware PID framing fixes from dev.box. Keep the resize shutdown fix and validate all cases with the race detector. Publish same-repository PR images even when this repository is itself a fork; continue excluding external PR publication, with matching attestation and summary conditions. Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
Both PR and push CI failed TestSmallUpload, TestLargeUpload, and TestS3 because the public minio/minio image is no longer available. Official Quay images and binary downloads are also unavailable. Build the official RELEASE.2025-10-15T17-29-55Z source commit as a local Docker fixture and reuse the existing S3 helper for upload tests. Keep integration assertions enabled, use dynamically allocated ports, and clean up temporary storage via testing.T. Validation: all three original failures reproduced locally; pinned source builds; targeted vet and golangci-lint pass; Settings and resize regressions pass with race detection (20 repetitions). Local Docker integration verification is blocked by a full Docker VM disk; validate the complete suite in CI.
CI built the pinned source successfully, but Docker returned no port mapping because the replacement image lacked the original image's EXPOSE metadata. Declare port 9000 so the existing test helper can allocate and discover its host port.
Revert the source-built MinIO fixture and upload-helper refactor. The updated deploy branch already carries upstream's minimal switch to pgsty/minio, so Settings does not need a separate test-infrastructure implementation.
meffmadd
added a commit
that referenced
this pull request
Oct 16, 2026
Bring in PR #11 (Settings SSH policy and gateway resize crash fixes) so the error-messages branch stays current with deploy: session signals now honor the dedicated signal policy, session channel handlers start only once, and the kubernetes resize shutdown and PID/rollback regressions are covered. No interaction with the authorization-denial banner work. Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
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
Preserve the gateway guarantees needed to test the Settings POC with this fork image, and fix the resize/exit crash observed during manual testing.
A late
window-changepreviously causedpanic: send on closed channel, crashed the entire gateway, broke the port-forward, and stranded connection-mode Settings pods. The resize regression exercises real execution completion andOnWindowin all three Kubernetes modes.The other four fixes were already required by the deployed dev.box gateway. They are now ported into this PR with their existing regressions so replacing that gateway with the fork image does not reintroduce startup deadlock or weaken Settings policy/lifecycle behavior. The user explicitly approved this expanded scope.
CI publication now compares the PR head repository with
github.repository, rather than testing whether the repository itself is a fork. Same-repository PRs can publishpr-11; external PRs remain excluded. Push, attestation, and summary conditions match. The earlier successful image build usedpush: falseand did not publish its tag.Target:
TU-Wien-dataLAB/ContainerSSH→deploy, not upstream. No chart/webhook/application code, orphan GC, or automatic cluster deployment is included.Evidence
OnWindowpanics and repeated-Stop panic. Each of the four added regressions also failed before its production fix: repeated starts accepted, signal policy routed through Shell.Mode, no readiness rollback, and four-byte PID blocked before initial resize.After: all five regressions pass with race detection:
golangci-lint2.14.0 (diff againstorigin/deploy) pass with zero issues.deploybase by upstream PR #713, which switches topgsty/minio. Merged updateddeployinto this branch and reverted the temporary source-built fixture. S3 helpers, upload tests, and their READMEs now matchdeployexactly; they are not part of this PR's diff.5f6c644: pull-request tests and push tests pass. Lint, both snapshot builds, and image publication also pass; the push snapshot succeeded on rerun after an external release-query HTTP 403. Targeted vet/lint and all Settings/resize regressions pass locally with race detection for 20 repetitions. Local Docker integration reruns remain blocked by a full Docker VM disk.ghcr.io/tu-wien-datalab/containerssh:pr-11@sha256:a2e40e8265787a86b5b49579dd4fc7fa7515641b244121d7e5d3e2104047914c. This image has not been rolled out or live-tested; the live evidence below refers to the earlier pinned digest.ghcr.io/tu-wien-datalab/containerssh:pr-11@sha256:529a4f5618be89a56e3714fe9f77cf886a6674131e0cec5f5f1b28841d92beb9. Gateway-only upgrade preserved all other values/Secret references. Full SSH policy/lifecycle probe, 112 quit/resize packets across four fresh connections, and real OpenSSH checks passed; gateway UID remained stable with zero restarts, new Settings pods were removed after disconnect, and the ordinary box UID was unchanged. Two pre-existing crash leftovers were deliberately not deleted. Live evidence.Merge Danger
Door: two-way
No configuration/API migration or persistent-data change. Reverting the commits restores prior behavior, including the bugs. A resize already in flight may still reach an active reader while shutdown races it; calls starting after shutdown are rejected. Readiness rollback bounds the DELETE request, not finalizer resolution or physical disappearance.
Blast Radius: Gateway
Single-start enforcement affects SSH session channels across backends; signal policy follows its own configuration. Kubernetes PID/resize behavior changes across execution modes, while new readiness rollback is connection-mode only. CI can publish trusted same-repository PR images. Live image selection and rollout are separate operations; existing orphaned Settings pods still require separately approved, UID-preconditioned cleanup.