Skip to content

Fix missing icon for minions reporting Linux - #993

Merged
erwindon merged 2 commits into
erwindon:masterfrom
nataliafit:fix/linux-os-icon
Oct 9, 2026
Merged

erwindon merged 2 commits into
erwindon:masterfrom
nataliafit:fix/linux-os-icon

Conversation

@nataliafit

@nataliafit nataliafit commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add saltgui/static/images/os-linux.png so minions reporting Linux as their OS display the Tux icon.

When Salt cannot identify a specific Linux distribution and reports os: Linux, SaltGUI already requests os-linux.png. Since this image is missing, it displays the unknown-OS icon instead.

No JavaScript or CSS changes are needed.

Image source and attribution

The 100 × 100 PNG is based on Tux.svg, originally drawn by Larry Ewing with GIMP. The source SVG also credits Simon Budig and Garrett LeSage.

The image's source, authors and usage terms are recorded in docs/OS-ICONS.md, together with the complete original README/Copyright notice. This shared document replaces the per-image license file and is linked from the documentation navigation. Include it when redistributing the icon, including distributions containing only the saltgui directory. These terms apply only to the image; SaltGUI's license is unchanged.

Include the Tux attribution and original copyright notice alongside the image.
@erwindon

erwindon commented Oct 9, 2026

Copy link
Copy Markdown
Owner

@nataliafit

  1. My policy is to have a VM with the related OS edition installed to do a full test. Can you please indicate which Linux distribution this applies to?

  2. I neglected to add copyright messages for all of the previous OS-icons. Instead of introducing one file for each (set of) icons, I would like to reference such information in a separate markdown file in the docs directory. The file would have filename, original-image-url, author-name and license-url. This PR (indirectly) already has all that information. Can you remove file saltgui/static/images/os-linux.png.LICENSE from the PR?

@erwindon erwindon assigned nataliafit and unassigned erwindon Oct 9, 2026
@sonarqubecloud

sonarqubecloud Bot commented Oct 9, 2026

Copy link
Copy Markdown

@nataliafit

Copy link
Copy Markdown
Contributor Author

Hi Erwin, thanks for reviewing!

  1. The affected system is a Synology NAS running DSM, with salt-minion installed through the Nix package manager. It is not a NixOS installation: DSM is the host OS, and Nix is only used to install packages. Salt reports os: Linux on this system, which is the case this PR addresses.

  2. Done in 15fdba7. I removed saltgui/static/images/os-linux.png.LICENSE and moved the attribution into a shared docs/OS-ICONS.md, with the filename, original image URL, authors and usage-terms URL. It also preserves the complete original README/Copyright notice, since the source artwork's redistribution terms ask for that notice to accompany the image. I added the document to the existing documentation navigation and inclusion list, and updated the PR description accordingly.

The PNG itself is unchanged, and the image's terms remain separate from SaltGUI's license.

@erwindon

erwindon commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Synology NAS running DSM

I would never have guessed that! it sounds familiar :-), but I won't use mine for this test.
Reverting to salt salt-normal-4-minion grains.setval os Linux to temporary overrule the os grain of a Debian 13 VM:

afbeelding

ok!

docs/OS-ICONS.md

2 goals reached: this structure prevents too many files, and it is no longer in a web-accessible location.

ok!

@erwindon erwindon assigned erwindon and unassigned nataliafit Oct 9, 2026
@erwindon
erwindon merged commit a7da27f into erwindon:master Oct 9, 2026
8 checks passed
@erwindon

erwindon commented Oct 9, 2026

Copy link
Copy Markdown
Owner

thx!

@nataliafit

Copy link
Copy Markdown
Contributor Author

Thanks for testing the os: Linux case on your Debian VM and for merging, Erwin! Glad the shared attribution document fits what you had in mind. 🙂

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.

2 participants