build(server): remove unused GitSSH sidecar - #28204
Alex Villarreal (alexvy86) wants to merge 4 commits into
Conversation
server/gitssh provided a git-over-SSH sidecar container (Alpine + git + OpenSSH) alongside GitRest, sharing its persistence volume. No application code opens an SSH/git-protocol connection to it: GitRest reads/writes document snapshots directly through isomorphic-git and the filesystem, not over SSH. The only real use was a documented manual workflow for cloning snapshots with a native git client to inspect history locally. This removes the package and its dedicated `server-gitssh` build pipeline, unwires the sidecar from the docker-compose files, the Historian Helm chart, and the real-service test pipeline, and updates the routerlicious README to use `docker compose exec`/`cp` against GitRest's mounted volume instead of `git clone ssh://...:3022/...`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (263 lines, 21 files), I've queued these reviewers:
How this works
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved findings include a stale Helm Service port and outdated SSH references in documentation and container configuration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Removes the obsolete GitSSH sidecar and updates related build, deployment, testing, and documentation workflows.
Changes:
- Deletes the GitSSH image, package, and dedicated pipeline.
- Removes GitSSH wiring from Compose, Helm, and test-service configurations.
- Replaces SSH-based snapshot inspection instructions.
File summaries
| File | Summary |
|---|---|
tools/pipelines/test-real-service.yml |
Removes GitSSH pipeline resources and image references. |
tools/pipelines/templates/include-test-real-service.yml |
Removes GitSSH parameters and environment wiring. |
tools/pipelines/templates/build-docker-service.yml |
Retains the obsolete GitSSH skip branch. |
tools/pipelines/server-gitssh.yml |
Deletes the GitSSH build pipeline. |
server/routerlicious/README.md |
Updates snapshot inspection instructions. |
server/routerlicious/docker-compose.yml |
Removes the GitSSH service. |
server/README.md |
Removes GitSSH documentation. |
server/gitssh/README.md |
Deletes the obsolete README. |
server/gitssh/entrypoint.sh |
Deletes the obsolete entrypoint. |
server/gitssh/Dockerfile |
Deletes the obsolete container definition. |
server/gitrest/docker-compose.yml |
Removes the GitSSH service. |
server/docker-compose.yml |
Removes the GitSSH service. |
server/docker-compose.dev.yml |
Removes the development GitSSH service. |
server/charts/historian/values.yaml |
Removes GitSSH image values. |
server/charts/historian/templates/gitrest-deployment.yaml |
Removes the GitSSH sidecar. |
server/charts/historian/templates/_helpers.tpl |
Removes the GitSSH naming helper. |
.claude/skills/trigger-pipelines-for-copilot-pr/SKILL.md |
Removes GitSSH pipeline trigger instructions. |
Review details
Suppressed comments (3)
server/README.md:14
- The sidecar removal leaves stale SSH-based guidance in
server/gitrest/README.md:74(the submodule example writesssh://git@localhost:3022/...) andserver/gitrest/Dockerfile:107-109(it describes sharing a volume with an SSH service). Those references now point to the deleted service; update or remove them in this change.
server/routerlicious/README.md:235 - This replacement workflow assumes the GitRest container has the native
gitCLI, butserver/gitrest/Dockerfileusesnode:22.22.2-bookworm-slimwithout installinggit, and the application only depends onisomorphic-git.docker compose exec gitrest git clone ...will therefore fail in the published image; either add the CLI to that image or make the host-sidedocker compose cpflow the supported option.
docker compose exec gitrest git clone /home/node/documents/fluid/fluid /tmp/fluid
docker compose exec gitrest sh -c "cd /tmp/fluid && git checkout <document id>"
server/routerlicious/README.md:243
- The copied path is the bare repository created by
isomorphic-git.init({ bare: true })(server/gitrest/packages/gitrest-base/src/utils/isomorphicgitManager.ts:412-416). Copying it directly to./fluidand runninggit checkoutthere will fail because a bare repository has no work tree; clone the copied bare repository into a second host directory before checking out the document revision.
docker compose cp gitrest:/home/node/documents/fluid/fluid ./fluid
cd fluid
git checkout <document id>
- Files reviewed: 17/17 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Remove the now-dead ssh Service port from the Historian gitrest-service Helm template. - Update gitrest README/Dockerfile comments that still described the removed ssh sidecar. - Fix the routerlicious README snapshot-inspection instructions: the GitRest image has no git CLI (only isomorphic-git), and the mounted repositories are bare, so 'docker compose exec ... git clone' and 'git checkout' directly on the copied bare repo would both fail. Copy the bare repo out with 'docker compose cp' then 'git clone' it locally to get a working tree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fleet Review — CleanNo issues found across the reviewer fleet for this run. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
[Agent-generated] Updated We are deliberately not adding a release or migration notice. Based on the project context, Git-over-SSH was never part of an explicit consumer contract, and these server images are not meaningfully consumed by external users. Existing GitRest REST operations and repository data are unchanged, so a release-facing migration notice would add noise without providing a useful migration path. |
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Deep ReviewReviewed commit Readiness: 10/10 — READY Ready for sign-off. The GitSSH sidecar is removed coherently across Compose, Helm, CI, automation, and documentation while GitRest's storage volume and REST data path remain intact. Context for Reviewers
For human reviewer
Review history (2 prior reviews)
|
|
Deep Review: The Historian README now accurately describes REST-only access. Your explanation establishes that Git-over-SSH was not a supported consumer contract and that there is no useful consumer migration path, so the release-note request is resolved. |
Description
server/gitsshwas a git-over-SSH sidecar container (Alpine + git + OpenSSH) that shared GitRest's persistence volume. No application code opens an SSH/git-protocol connection to it: GitRest reads/writes document snapshots directly through isomorphic-git and the filesystem, not over SSH. Its only real use was a documented manual workflow for cloning snapshots with a native git client to inspect history locally.There was a prior "Remove everything gitssh" commit (Jan 2025) that never made it into
mainand predates several since-added CI/build changes to the package, so this re-derives the removal against currentmain.This removes the package and its dedicated
server-gitsshbuild pipeline, unwires the sidecar from the docker-compose files (server/docker-compose.yml,server/docker-compose.dev.yml,server/gitrest/docker-compose.yml,server/routerlicious/docker-compose.yml), the Historian Helm chart, and thetest-real-servicepipeline, and updates the routerlicious README to usedocker compose exec/cpagainst GitRest's mounted volume instead ofgit clone ssh://...:3022/....Reviewer Guidance
The review process is outlined in the pull request guidelines.
tools/pipelines/templates/build-docker-service.ymlkeeps itstagName == 'gitssh'skip-branch mechanism (now always-true / dead) rather than restructuring the shared template; flagging as optional follow-up cleanup, not required here.ff_internal's selfhost docker-compose files still referenceserver/gitsshand will need a follow-up there once this merges.