Skip to content

fix(deps): refresh tmate image and charm packaging tools - #58

Merged
cbartz merged 16 commits into
mainfrom
fix/ssdlc-26-10-dependencies
Oct 7, 2026
Merged

cbartz merged 16 commits into
mainfrom
fix/ssdlc-26-10-dependencies

Conversation

@cbartz

@cbartz cbartz commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Applicable spec: ISD-6336 (SSDLC 26.10, SEC0025)

Overview

Refresh the rock to 1.1 and align the service image tag; require patched setuptools/wheel in the charm. Part of ISD-6336.

Rationale

The deployed image has stale Ubuntu packages and Pebble's Go dependencies. Local Trivy: image High/Critical 41 → 0, with all targeted image CVEs that have fixes gone. The 25 remaining Low/Medium findings have no scanner-listed fixes. Unmodified fresh rebuild → versioned rebuild: High/Critical 0 → 0.

Packed the Ubuntu 22.04 charm before/after and scanned its extracted rootfs: High/Critical 3 → 0. Setuptools is now 84.0.0 and wheel is 0.48.0 (vendored wheel 0.46.3), clearing CVE-2022-40897, CVE-2024-6345, CVE-2025-47273 and CVE-2026-24049. Six existing Low/Medium pip findings remain; no new High/Critical findings. JSON/table scans saved locally. Secscan re-verification after publish.

Image: ghcr.io/canonical/tmate-ssh-server:1.1 is published (digest sha256:c8d9bb93639d9342e25bf5591a947eaa6b0d12cfa3a233422738b199199eb52a), pushed from the same rock that was scanned above; integration tests pass against it. The charm workflow does not publish this hard-coded image, so the publication steps are now documented in CONTRIBUTING.md.

Juju Events Changes

install, update-status and the new upgrade-charm handler all call one idempotent tmate.ensure_daemon_running(address) (replacing start_daemon). It renders the service with the installed container name and keeps a running workload whose service is unchanged, so charm-only upgrades no longer interrupt sessions. Otherwise it pulls the image if not cached (a pull failure aborts with the old workload intact), force-removes the previous container, rewrites/reloads the service and restarts it, preserving SSH keys. The explicit removal is needed because tmate ignores SIGTERM as container PID 1, so a plain service restart leaves the old container holding the port. New containers use --rm to clean themselves up on exit. A restart interrupts active sessions.

Module Changes

Rock version, image tag (now the IMAGE constant in src/tmate.py), Python packaging dependency floors, publication instructions in CONTRIBUTING.md, ensure_daemon_running and upgrade handler with tests, a test_upgrade integration test and changelog. Tox lint/unit/static pass: 60 unit tests, 100% source coverage; rock and charm pack pass.

Library Changes

None.

Checklist

AI-generated message.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Add and test an upgrade path that reloads and restarts existing services to apply the refreshed image.

Review effort: Lite
Findings: None

What changed in this PR

Refreshes the tmate image to 1.1 and updates charm packaging dependencies to patched versions.

Changes:

  • Updates the rock and service image tag.
  • Raises setuptools and wheel version floors.
  • Documents image publication and updates the changelog.
File Summary
tmate-ssh-server_rock/​rockcraft.yaml Updates rock version.
templates/​tmate-ssh-server.service.j2 References image tag 1.1; existing installations may retain 0.1.1 without an upgrade path.
requirements.txt Adds packaging dependency floors.
docs/​reference/​charm-architecture.md Documents image publication instructions.
docs/​changelog.md Records the refresh.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cbartz

cbartz commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the upgrade-path concern in the review body: upgrade-charm now rewrites the unit, reloads systemd and restarts the workload with the refreshed image, preserving SSH keys. Added success, defer, failure and reload-before-restart tests. Tox lint/unit/static pass (53 tests, 100% coverage); rebuilt charm Trivy scan remains 0 High/Critical. The upgrade documentation notes that active sessions are interrupted.

AI-generated message.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Upgrade handling leaves stopped containers behind, risking disk exhaustion on repeated upgrades.

Review effort: Lite
Findings: 1 Medium severity

Open (1)

Comment thread src/tmate.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Upgrade cleanup can remove unrelated stopped containers via host-wide pruning.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread src/charm.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Final human confirmation is warranted for image publication and upgrade restart behavior.

Review effort: Lite
Findings: None

Resolved since last review (1)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified in the review.

Review effort: Lite
Findings: None

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The upgrade restart can leave the legacy container running and prevent the replacement from binding port 10022.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread src/tmate.py Outdated
@cbartz
cbartz marked this pull request as ready for review October 6, 2026 13:56
@cbartz
cbartz requested a review from a team as a code owner October 6, 2026 13:56
@cbartz
cbartz marked this pull request as draft October 6, 2026 14:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Failed upgrades can skip workload recovery on retry, and integration assertions do not verify the expected patched image.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)

Comment thread src/tmate.py Outdated
Comment thread tests/integration/test_upgrade.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Live workload replacement has unresolved availability and regression-coverage concerns, and the published image’s security claims require human validation.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Pin a pre-change charm revision to test image replacement

tests/​integration/​test_upgrade.py:33

After this change reaches latest/edge, the deployed baseline can already have the same image and service template as the charm under test. Both refreshes can then take the unchanged-service path, leaving legacy-container removal and image replacement untested. Pin a known pre-change charm revision and assert before refreshing that its image differs from the expected replacement.

Comment thread src/tmate.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues remain, and the upgrade safeguards are covered by focused unit and integration tests.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The upgrade safeguards and regression coverage address the prior concerns, with no unresolved blocking findings.

Review effort: Balanced
Findings: None

@cbartz
cbartz marked this pull request as ready for review October 7, 2026 06:59

@yanksyoon yanksyoon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks

Comment thread src/charm.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Live-container replacement and the published image’s security scan evidence warrant final human validation.

Review effort: Balanced
Findings: None

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Unused entries in .trivyignore

Image: ghcr.io/canonical/tmate-ssh-server:a0edf8f27d99ca10ec2b3cafa43ffe32feaea597-_1.1_amd64

The following CVEs are in .trivyignore but not ignored by Trivy anymore:

@cbartz
cbartz merged commit b1a9b41 into main Oct 7, 2026
28 of 30 checks passed
@cbartz
cbartz deleted the fix/ssdlc-26-10-dependencies branch October 7, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants