samba-in-kubernetes / samba-operator

An operator for a Samba as a service on PVCs in kubernetes
Apache License 2.0
113 stars 24 forks source link

Custom SMB configuration options #281

Open taxilian opened 1 year ago

taxilian commented 1 year ago

Is there a way to add custom SMB config options? For example I am trying to create a share that I can use for provisioning servers, but the management tools for the servers can only use the oldest SMB1 (NT1) protocol, so I need to override the "min version" in the config file.

Is there a place that I'm just not finding for adding things like that?

phlogistonjohn commented 1 year ago

Unfortunately, there is no option for custom configuration today. The CRD design doc for shares proposed a key for custom configuration but it has not been implemented. While it's not in the design we could probably add global custom configs in the SmbCommonConfig.

Thanks for sharing your use-case too. It's unfortunate that your tool doesn't support the more secure SMB2+ protocols.

I can't promise any kind of timelines but I do try to prioritize things based on feedback - so keep an eye on this issue and we may try to add these features. :-)

taxilian commented 1 year ago

No worries -- it turns out the client is even worse than suspected, when I use it with a loadbalancer IP it won't connect at all. By comparing the difference with tcpdump it appears that it won't even attempt to connect to something that doesn't respond to a ping -- and when I tried connecting it to the pod directly it worked but only very poorly, which leads me to suspect it's not designed to work with anything not on the same local network (which my kube cluster sorta technically isn't since it requires BGP routing via the ToR router to get to).

... so, I had to give up on putting it in my cluster, sadly.

Thank you for the response!

taxilian commented 1 year ago

Oh -- as a workaround for anyone in the future looking for this, though, you can edit the configmap after creating the resource and it will work. That will probably get recreated if you change the resource, but it does at least let you put in custom config options.

phlogistonjohn commented 1 year ago

It's too bad about that ping thing. Too often people write things like that are "helpfully" unhelpful.

And because I feel obligated to mention it: you can still use our sibling project samba-containers Samba server images on standalone nodes. If it's easy enough in your env. to install samba from packages, no worries, but I do like to promote the container images when I can :-)

taxilian commented 1 year ago

Yeah, that's a point -- probably should have done that. Maybe I could even figure out how to set up docker to use cephfs still =]

I may look into it, just ran out of time. That would at least give me an easily repeatable option. I appreciate the suggestion!

tiferrei commented 7 months ago

I don't wish specifically to comment on an old(er) issue but I just wanted to report my use case that would be greatly improved by having custom global configs.

I mostly connect to my shares on a Mac, which uses the built-in SMB stack. It is known for being horrible however I guess it is just the one that I have access to. An issue I've been facing a lot recently is that whenever I try to copy a file into the share I get a "file in use error" that seems to be resolved by using some special vfs_fruit configs that make the experience easier on Mac.

Naturally this sort of stuff would also allow me to have Time Machine in the cluster, as suggested in #316. However I must highlight that these sort of attributes seem to be needed even to make simple things, like moving a file into the share, bearable and reliable on Macs.

I can confirm that for example, on a vanilla baremetal Samba install (that I use at the moment for time machine), I employ these extra configs and the experience is miles better. Somehow, even the transfer speeds are a lot faster.

phlogistonjohn commented 7 months ago

I appreciate the feedback. Personally, due to business priorities I don't have much time to work on the operator directly at the moment as I have to focus more of my time on the Ceph project. That said, having generic custom configs makes it so I/we don't have to stop and add other features that folks might want if they can add it via a custom config, so this would be the highest priority feature for me if I was available to hack on it.

Let me make one request to anyone in the community that feels up to coding in Go and is interested to have this feature to give coding it up it a try and submit a PR! I will try to coach you as needed if you so desire it. :-) Otherwise, when things quiet down a little bit on the ceph front I'll try to find the time to get this one done myself.

barrettMCW commented 7 months ago

Hey all! I'm in a pretty similar situation as tiferrei. custom configs seems easy enough to implement and would be nice for getting my feet wet in Go.

I've added the property to the crd and structs in my fork. Looking at the design docs seems like there is some uncertainty on how we want to define these keyvals in the configmap. To me it seems like we only need to provide override access to globals.globals.options?

so i wonder if we do a configmap with an options.json file containing a simple dictionary of key vals?

next is how do we get it into the container? The only thing that I can think of is just having the operator update the real config map with those properties.

How does all this sound? Any pointers or preferred mechanisms of adding this functionality?

Thanks!

phlogistonjohn commented 7 months ago

Hi @barrettMCW, thanks a ton for looking into this! A few thoughts:

Hey all! I'm in a pretty similar situation as tiferrei. custom configs seems easy enough to implement and would be nice for getting my feet wet in Go.

:+1:

I've added the property to the crd and structs in my fork. Looking at the design docs seems like there is some uncertainty on how we want to define these keyvals in the configmap. To me it seems like we only need to provide override access to globals.globals.options?

It depends on what we're trying to override. If we're trying to override only global options (options marked G in smb.conf) that'd be fine and if you want to apply a share level option (options marked (S) in smb.conf) for all shares hosted by a particular smbd. If we want to override share settings on a per-share basis without impacting any other shares we'd need the custom option to only go in that share's section.

All that said, I have no problem if you only want to do global options because it solves your use case and deferring per-share options. Both do not have to be done at the same time.

so i wonder if we do a configmap with an options.json file containing a simple dictionary of key vals?

That would be acceptable. I think what might be slightly more user-friendly would be a new sub-section of the SmbCommonConfig CR. That subsection would then take a value of a map of string-to-string values (map[string]string in Go speak). I think this would be easier to document and have users reason about.

next is how do we get it into the container? The only thing that I can think of is just having the operator update the real config map with those properties.

I think putting it in the CR solves this issue. If you go with the configmap approach the operator could also be coded to look for a configmap with a predefined name (or names) and read it via the k8s Go apis.

How does all this sound? Any pointers or preferred mechanisms of adding this functionality?

I hope my feedback was useful. I personally think that extending the CR would be the simplest to code up because you don't have to add much new stuff and I think the use would be simpler to document and explain. I'm happy to discuss futher if desired! Again, thanks for taking the time to work on this topic.

barrettMCW commented 7 months ago

Thanks @phlogistonjohn That all sounds great! Started the smbcommonconfig crd modifications.

Did some digging to try and understand the json a bit better: https://github.com/samba-in-kubernetes/sambacc/blob/master/docs/configuration.md

Here's where I'm at: add new property

SmbCommonConfig: 
  smbConf: 
    options: 
      key:val

options mapping defined in SmbCommonConfig gets placed into the configmap's config.json: globals.{commonconfigname}.options This should handle global and share configs within a given smbd instance.

Time permitting I plan on adding the same to SmbShare for share specific overrides. should just be adding this new map:

SmbShare:
  smbConf:
     options:
       key:val

to shares.{share}.options

All of this seems good to me and within my capabilities, I'll be working on it today with hopefully a pr in the next 24hrs or so. Thanks for your help and cooperation!

phlogistonjohn commented 7 months ago

Sounds like a good plan to me! I look forward to seeing the patches - but don't feel the need to rush to meet your goal. :-)