Skip to content

Conversation

@XiYang6666
Copy link

No description provided.

@XiYang6666 XiYang6666 requested a review from crazy-max as a code owner June 21, 2025 05:20
README.md Outdated
Comment on lines 130 to 132
groups:
- group1:2000
- group2:2001
Copy link
Owner

Choose a reason for hiding this comment

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

Would be better as a map:

Suggested change
groups:
- group1:2000
- group2:2001
groups:
group1: 2000
group2: 2001

Copy link
Owner

Choose a reason for hiding this comment

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

Also I think we could deprecate group in favor of the new field.

Copy link
Author

Choose a reason for hiding this comment

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

This is a good idea, I am trying to implement it.

@crazy-max
Copy link
Owner

Can you also add a new user with multiple groups in this file https://github.com/crazy-max/docker-samba/blob/master/test/data/config.yml so it's tested in ci workflow?

@XiYang6666
Copy link
Author

XiYang6666 commented Jun 21, 2025

auth:
  - user: foo
    group: foo
    uid: 1000
    gid: 1000
    password: bar
    groups: [qux, quux]
  - user: yyy
    group: xxx
    uid: 1100
    gid: 1200
    password_file: /tmp/yyy_password

group:
  qux: 2001
  quux: 2002

I think this is a better implementation because each user requires a main group. Additionally, this implementation can also ensure that groups are not created repeatedly.

@spaghetti-coder
Copy link

Nice one! You spared me a MR with a more clumsy implementation
Think will also be good to see the usage in examples here: https://github.com/crazy-max/docker-samba/tree/master/examples
The entries added to readme do nothing useful in terms of samba shares from the same conf file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants