Closed synarete closed 2 years ago
@phlogistonjohn I changed this PR to draft
once @spuiuk pointed out that there isn't any need for an explicit init container in order to change root-dir permissions. There is no need to merge this one into master (we can leave it as reference in case needed in the future).
@synarete, I was just checking with John on this. I though ensure_share_dirs() was called each time we create a new pvc path. but I may have been wrong.
@synarete, I was just checking with John on this. I though ensure_share_dirs() was called each time we create a new pvc path. but I may have been wrong.
OK, in that case, this PR is needed.
@synarete. I guess we can test using the operator patches I have with me as soon as we have a server image with a working sambacc in it. I'll try and get one running for testing tomorrow morning.
@synarete. I guess we can test using the operator patches I have with me as soon as we have a server image with a working sambacc in it. I'll try and get one running for testing tomorrow morning.
@spuiuk I rebased this PR over latest master (with @anoopcs9 latest CI fixes in git commit 83f74f6
) and re-pushed.
@synarete, Sorry for the delay in replying. I went through the code again and I believe that perms_handler() is also called from the following code path.
samba-container update_config will call update_config() -> _update_config() -> perms_handler()
This in turn will set the permissions and we don't need an additional init container.
I haven't tested this out yet since we don't have an image with the latest sambacc. But I have now built one and will try it out as soon as I figure out my workflow to include it with the samba-operator.
@spuiuk Thanks for this update. Who is calling update-config
in current deployment flow?
@synarete You are right. Looks like we will need this init container.
Here are my steps 1) Created quay.io/spuiuk/samba-server:experimental a) build samba-server with my patches to include pyxattr b) tag as quay.io/spuiuk/samba-server:experimental c) push to quay.io 2) added this new samba-server image to config/developer/kustomization.yaml file 3) build and deploy new operator with patch to set correct section in /etc/container-config/config.json 4) create smbshare 5) Testing
[sprabhu@t10 samba-operator]$ kubectl create -f ~/smbshare2.yaml
[sprabhu@t10 samba-operator]$ kubectl get pods -w
NAME READY STATUS RESTARTS AGE
samba-ad-server-569f9c54c4-42kp4 1/1 Running 0 23d
smbclient 1/1 Running 0 23d
smbshare2-8575c9ff6b-bhgwk 0/1 ContainerCreating 0 8s
smbshare2-8575c9ff6b-bhgwk 0/1 Running 0 13s
[sprabhu@t10 samba-operator]$ kubectl exec -it smbshare2-8575c9ff6b-bhgwk -- /bin/bash
From within the smbshare pod
[root@smbshare2-8575c9ff6b-bhgwk /]# dnf install -y attr
..
[root@smbshare2-8575c9ff6b-bhgwk /]# ls /mnt
97d365b6-4340-49be-9b1d-6f1939d43985
[root@smbshare2-8575c9ff6b-bhgwk /]# getfattr /mnt/97d365b6-4340-49be-9b1d-6f1939d43985/
[root@smbshare2-8575c9ff6b-bhgwk /]# samba-container ensure-share-paths
2022-05-19 11:27:34,314: INFO: Ensuring share path: /mnt/97d365b6-4340-49be-9b1d-6f1939d43985
2022-05-19 11:27:34,315: INFO: Updating permissions if needed: /mnt/97d365b6-4340-49be-9b1d-6f1939d43985
2022-05-19 11:27:34,315: INFO: Using initializing posix permissions handler
[root@smbshare2-8575c9ff6b-bhgwk /]# getfattr /mnt/97d365b6-4340-49be-9b1d-6f1939d43985/
getfattr: Removing leading '/' from absolute path names
# file: mnt/97d365b6-4340-49be-9b1d-6f1939d43985/
user.share-perms-status
My /etc/container-config/config.json file created by the operator
{
"samba-container-config": "v0",
"configs": {
"smbshare2": {
"shares": [
"smbshare2"
],
"globals": [
"globals"
],
"instance_name": "smbshare2",
"permissions": {
"method": "initialize-share-perms",
"status_xattr": "user.share-perms-status",
"mode": "0777"
}
}
},
"shares": {
"smbshare2": {
"options": {
"path": "/mnt/97d365b6-4340-49be-9b1d-6f1939d43985",
"read only": "no"
}
}
},
"globals": {
"globals": {
"options": {
"disable spoolss": "yes",
"fileid:algorithm": "fsid",
"load printers": "no",
"printcap name": "/dev/null",
"printing": "bsd",
"smb ports": "445",
"vfs objects": "fileid"
}
}
},
"users": {
"all_entries": [
{
"name": "sambauser",
"password": "samba"
}
]
}
}
Add init container to ensure share's top-level permissions are properly set, via sambacc tool.
Signed-off-by: Shachar Sharon ssharon@redhat.com