README.md updates

[Erriez]

Behaviour

Thanks for creating this project. I'd like to share user feedback and some proposals for documentation improvement.

Issue 1

The --load option in README.md is missing before the created image samba:local can be used with docker-compose up. Additionally, the default tag can be overruled as I'd like to push to my local registry:

git clone https://github.com/crazy-max/docker-samba.git
cd docker-samba

# Build image and output to docker (default)
-docker buildx bake
+docker buildx bake --load
+
+# To override the default tag samba:local, set environment variable
+DEFAULT_TAG=<username>samba:<tag> docker buildx bake --load

Issue 2

Copy the content of folder examples/compose in /var/samba/ on your host for example.

This is confusing, because any directory on the host can be used. Proposal to remove the sentence, or update to copy examples/compose to any location on the host.

Issue 3

Volumes of the shared directories are missing. It is recommended to save the data on the host and not inside the container. For example, the path setting in config.yml is the path inside the container, not the host. (Maybe confusing for beginners) Proposal to update the README.md and docker-compose.yml to:

version: "3.5"

services:
  samba:
    image: registry.mydomain.com/erriez/docker-samba-cm:latest
    container_name: samba
    restart: always
    network_mode: host
+    volumes:
+      - $PWD/data:/data
+      - $PWD/samba/foo:/samba/foo
+      - $PWD/samba/public:/samba/public
+      - $PWD/samba/share:/samba/share

Issue 4

Missing documentation password_file: /run/secrets/baz_password in config.yml. Proposal to add link to the official documentation how to create a secure password file: https://www.samba.org/samba/docs/current/man-html/smbpasswd.8.html

Is it possible to create a secure password with this container? A suggestion to share the password file via the volume data/.

Issue 5

User baz is currently not used in docker-compose.yml. It would be nice to create an example share with read/write permission for two users, for example in config.yml:

+  - name: foo-baz
+    path: /samba/foo-baz
+    browsable: yes
+    readonly: no
+    guestok: no
+    validusers: foo,baz
+    writelist: foo,baz
+    veto: no

Issue 6

Missing explanation veto in config.yml. What is the purpose of this setting?

Issue 7

The network is exposed to the host in docker-compose.yml with network_mode: host. However, port 445 for SMB3 is sufficient as NetBIOS ports / multicast are no longer used. Proposal to change to:

version: "3.5"

services:
  samba:
    image: registry.mydomain.com/erriez/docker-samba-cm:latest
    container_name: samba
    restart: always
-    network_mode: host
+    ports:
+      - 445:445

Configuration

• Docker version (type docker --version) : Not relevant.
• Docker compose version if applicable (type docker-compose --version) : Not relevant.
• Platform (Debian 9, Ubuntu 18.04, ...) : Ubuntu 20.04 server ARM64 (Samba Server), Windows 10 client, Ubuntu 20.04 client
• System info (type uname -a) : Not relevant.
• Include all necessary configuration files : docker-compose.yml, .env, ...

[crazy-max]

@Erriez

Issue 1

The image will be already loaded to Docker for the default image-local target, no need to add the --load flag:

$ docker buildx bake --print
{
  "target": {
    "image-local": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "tags": [
        "samba:local"
      ],
      "output": [
        "type=docker"
      ]
    }
  }
}

Issue 2

Yeah that's just an example, feel free to open a PR with:

update to copy examples/compose to any location on the host.

Issue 3

Good point, the example should be updated.

Issue 4

password_file is a secret that will be used by smbpasswd in the entrypoint script and not a generated SMB sessions file.

Issue 5

Agree we could do that, feel free to open a PR.

Issue 6

veto setting is a list of predefined files and directories that are neither visible nor accessible.

More info https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html#VETOFILES

Issue 7

Indeed this is enough. I guess I used that while testing avahi support. Should be fixed.

[Erriez]

Issue 1

Answered: You're right, it builds samba:local and I overlooked this. Great!

Issue 4

Answered: It loads a password_file configured in /data/config.yml with username and password on separate lines. I assume it is in plain text as the smbpasswd --help shows -s use stdin for password prompt. Explanation of this file format is missing in the README.md.

Issue 6

Ok, now I understand how it works: https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html#VETOFILES

A PR can improve remaining issues.