Skip to content

Samba: Normalize stored enabled_shares values to lower case - #4717

Merged
agners merged 2 commits into
home-assistant:masterfrom
kingpanther13:samba-enabled-shares-lowercase
Jul 15, 2026
Merged

Samba: Normalize stored enabled_shares values to lower case#4717
agners merged 2 commits into
home-assistant:masterfrom
kingpanther13:samba-enabled-shares-lowercase

Conversation

@kingpanther13

@kingpanther13 kingpanther13 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Step 1 of the plan agreed in home-assistant/supervisor#7023: prepare enabled_shares for a strict list() schema by normalizing stored values to lower case at startup. Values have always been validated case-insensitively and lower-cased at runtime before rendering smb.conf, so this changes no behavior — it only persists the canonical form.

On startup the app reads its stored options via the Supervisor API and, only when an enabled_shares value is not already lower case, writes them back lower-cased. On API failure it logs a warning and continues to boot, retrying on the next start.

Step 2 (separate, breaking): #4718 switches the schema from match() to list(), which also fixes the empty suggestion list in the configuration UI (home-assistant/frontend#51510).

Summary by CodeRabbit

  • New Features

    • Added automatic normalization of stored share names to lowercase during startup.
    • If updating stored configuration fails, a warning is logged and the update will be retried on the next startup.
  • Documentation

    • Updated release notes with a new 12.8.1 entry describing the startup-time normalization behavior.
  • Chores

    • Updated the Samba add-on version to 12.8.1.

Values have always been validated case-insensitively and lower-cased at
runtime before rendering smb.conf. Persisting the canonical lower-case
form prepares for a future release that restricts the schema to the
exact share names.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5be7ac25-3a94-4172-8fdb-1633e8b06327

📥 Commits

Reviewing files that changed from the base of the PR and between 30517a7 and 00b8551.

📒 Files selected for processing (2)
  • samba/CHANGELOG.md
  • samba/config.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • samba/config.yaml

📝 Walkthrough

Walkthrough

The Samba add-on now normalizes stored enabled_shares names to lowercase at startup, persists the migrated options through the supervisor API, documents the behavior, and updates the version to 12.8.1.

Changes

Enabled shares normalization

Layer / File(s) Summary
Startup enabled shares migration
samba/rootfs/etc/s6-overlay/s6-rc.d/init-smbd/run
Startup reads stored options, detects non-lowercase enabled_shares values, posts lowercase replacements, and logs failures for retry on the next start.
12.8.1 release metadata
samba/CHANGELOG.md, samba/config.yaml
The changelog documents startup normalization and the add-on version is updated to 12.8.1.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant init_smbd as init-smbd run
  participant SupervisorAPI
  participant StoredOptions
  init_smbd->>SupervisorAPI: Fetch stored options
  SupervisorAPI->>StoredOptions: Read enabled_shares
  StoredOptions-->>SupervisorAPI: Return share names
  SupervisorAPI-->>init_smbd: Return options
  init_smbd->>init_smbd: Lowercase non-normalized names
  init_smbd->>SupervisorAPI: POST migration payload
  SupervisorAPI->>StoredOptions: Persist normalized options
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: lowercasing stored enabled_shares values at startup.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@agners

agners commented Jul 15, 2026

Copy link
Copy Markdown
Member

This writes back all options, including the newly introduced netbios in PR #4705. In this case this is a side effect we prefer, since we want netbios to be enabled in settings for existing installations. This will allow us to flip the default to false, later, disabling it by default for new users without affecting existing installations.

Just wanted to point this out for this PR. I'd like to test this PR again before merging, probably later today or tomorrow.

@agners agners 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!

@agners
agners merged commit 4afd6cb into home-assistant:master Jul 15, 2026
8 checks passed
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