Skip to content

Conversation

@igorpecovnik
Copy link
Member

@igorpecovnik igorpecovnik commented Nov 8, 2025

Description

Change default folder, branding

Testing Procedure

Manual tests.

Checklist

  • Changes have been tested and verified

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

Walkthrough

The OpenSSH server module script was modified to change the default mount point from OPENSSHSERVER_BASE/storage to ${SOFTWARE_FOLDER}/swag/config/www. After container start, the script now reads PRETTY_NAME from /etc/os-release inside the openssh-server container to extract a container_version and writes that into the MOTD. The rsync installation and MOTD population are combined into a multiline shell block. Docker image removal was removed from the "remove" command and instead is conditionally performed in the "purge/cleanup" command; other container and storage cleanup steps are unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review the new mount path usage and any assumptions about ${SOFTWARE_FOLDER} and swag layout
  • Validate the /etc/os-release PRETTY_NAME extraction and string handling (quoting, empty values)
  • Inspect the multiline rsync/install/MOTD block for correct shell quoting, error propagation, and idempotence
  • Check the altered lifecycle commands: ensure image removal was correctly moved to purge and that conditional checks for image existence are robust
  • Verify no unintended side effects on container restart or backup paths due to the mount path change

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main behavioral change: keeping the Docker image on remove command but deleting it on purge command.
Description check ✅ Passed The description mentions changing default folder and branding, which aligns with the mount point path change and container version branding updates in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sshserver

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between fc5bd90 and 97a1287.

📒 Files selected for processing (1)
  • tools/modules/system/module_sshserver.sh (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/modules/system/module_sshserver.sh

Comment @coderabbitai help to get the list of available commands and usage tips.

@igorpecovnik igorpecovnik added the Ready to merge Reviewed, tested and ready for merge label Nov 8, 2025
@github-actions github-actions bot added 11 Milestone: Fourth quarter release size/small PR with less then 50 lines and removed Ready to merge Reviewed, tested and ready for merge labels Nov 8, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 47ab306 and fc5bd90.

📒 Files selected for processing (1)
  • tools/modules/system/module_sshserver.sh (2 hunks)
🔇 Additional comments (2)
tools/modules/system/module_sshserver.sh (2)

36-36: Default mount point updated as intended.

The default folder has been changed to ${SOFTWARE_FOLDER}/swag/config/www aligning with the PR objective for folder and branding updates.


75-82: Cleanup logic refactored per PR objective: image removal moved to purge only.

The removal flow now correctly:

  • Remove command (lines 75-77): Removes container only, leaves image intact (for quick reinstall).
  • Purge command (lines 78-82): Removes container first, then the image if it exists.

This aligns with the stated PR objective of "keep image on remove, remove on purge."

@igorpecovnik igorpecovnik merged commit c4d8b52 into main Nov 8, 2025
12 checks passed
@igorpecovnik igorpecovnik deleted the sshserver branch November 8, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

11 Milestone: Fourth quarter release size/small PR with less then 50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants