Merge pull request #115 from NethServer/bug-7537-1

Refs NethServer/dev#7537

Author: DavidePrincipi

README.md

@@ -256,16 +256,27 @@ container scans for files and directories whose filesystem change time
 exceeds the configured retention period and removes them. If a retention
 value of `0` is set, the automatic cleanup procedure is disabled.
 
-When the recycle bin is enabled, the Samba share parameter
-`recycle:repository = .recycle` is set in the share registry
-configuration. Review the current share configuration with:
+When the recycle bin is enabled on a share, its the Samba parameter
+`recycle:repository = .recycle/%U` is set in the share registry
+configuration.
+
+Review the current share configuration with:
 
     podman exec samba-dc net conf list
 
-The default repository value `.recycle` assigned to enable the recycle bin
-can be adjusted by executing a command like this:
+The default base path for the recycle bin is `.recycle`. It is created by
+the add-share and alter-share actions if it does not exist, to properly
+set its permissions. It can be changed with the following command:
+
+       python3 -magent -c 'agent.set_env("RECYCLE_REPOSITORY", ".myrecycle")'
+
+The path is relative to the share root directory. This change does not
+affect shares that have already been configured.
+
+The full recycle bin path is private for each user. For this reason, the
+`/%U` suffix is always automatically appended when generating the
+corresponding Samba configuration parameter.
 
-    python3 -magent -c 'agent.set_env("RECYCLE_REPOSITORY", ".myrecycle")'
 
 ## Known log messages
 
@@ -292,3 +303,24 @@ Migration is implemented in the `import-module` action.
 - `user.SAMBA_PAI` attribute is not copied to NS8 shares. It contains
   the ACL *protected/don't inherit* flag. See [map acl
   inherit](https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html).
+
+## Testing
+
+Test the module using the `test-module.sh` script:
+
+    ./test-module.sh <NODE_ADDR> ghcr.io/nethserver/samba:bug-7537
+
+Additional arguments are forwarded to the `robot` command (see [Robot
+Framework](https://robotframework.org/)).
+
+For instance, to speed up testing on a local machine:
+
+1. Skip the instance removal
+
+       ./test-module.sh 10.5.4.1 ghcr.io/nethserver/samba:bug-7537 --exclude remove
+
+2. Continue to use the Samba instance, skipping the installation steps.
+   The `--variable` option is required to find the existing Samba
+   instance.
+
+       ./test-module.sh 10.5.4.1 ghcr.io/nethserver/samba:bug-7537 --exclude createORremove --variable MID1:samba1

imageroot/actions/list-shares/50list_shares

@@ -70,7 +70,7 @@ list_shares = {
     "shares": []
 }
 
-ocfg = configparser.ConfigParser(delimiters=("="))
+ocfg = configparser.ConfigParser(delimiters=("="), interpolation=None)
 with subprocess.Popen(podman_exec + ["net", "conf", "list"], stdout=subprocess.PIPE, text=True) as hconf:
     try:
         ocfg.read_file(hconf.stdout, 'samba-registry-conf')

imageroot/pypkg/samba.py

@@ -227,12 +227,13 @@ def configure_samba_audit(sharename, enable_audit=True, log_failed_events=False)
 
 def configure_recycle(sharename, enable_recycle=True, recycle_retention=0, recycle_versions=False):
     if enable_recycle:
-        agent.run_helper("podman", "exec", "samba-dc", "net", "conf", "setparm", sharename, "recycle:repository", os.getenv("RECYCLE_REPOSITORY", ".recycle"))
+        agent.run_helper("podman", "exec", "samba-dc", "net", "conf", "setparm", sharename, "recycle:repository", os.getenv("RECYCLE_REPOSITORY", ".recycle") + "/%U")
         if recycle_versions:
             agent.run_helper("podman", "exec", "samba-dc", "net", "conf", "delparm", sharename, "recycle:versions")
         else:
             agent.run_helper("podman", "exec", "samba-dc", "net", "conf", "setparm", sharename, "recycle:versions", "no")
         agent.run_helper("podman", "exec", "samba-dc", "recycle", "set_retention", sharename, str(recycle_retention))
+        agent.run_helper("podman", "exec", "samba-dc", "recycle", "init_repository", sharename)
     else:
         agent.run_helper("podman", "exec", "samba-dc", "net", "conf", "delparm", sharename, "recycle:repository")
         agent.run_helper("podman", "exec", "samba-dc", "net", "conf", "delparm", sharename, "recycle:versions")

samba-dc/usr/local/sbin/recycle

@@ -10,6 +10,7 @@ import json
 import sys
 import time
 import os
+import shutil
 from pathlib import Path
 import argparse
 import configparser
@@ -70,7 +71,27 @@ def get_recycle_repository(share_name):
             stderr=subprocess.DEVNULL).strip()
     except subprocess.CalledProcessError as e:
         repodir = '' # recycle is disabled
-    return repodir
+    return repodir.removesuffix("/%U")
+
+def init_recycle_directory(share_name):
+    repository = get_recycle_repository(share_name)
+    if not repository:
+        return # recycle bin is disabled
+    share_path = RECYCLE_BASE / share_name
+    recycle_path = share_path / repository
+    if recycle_path.exists():
+        return
+    print(f"Creating recycle repository: {recycle_path}", file=sys.stderr)
+    cur = share_path
+    share_meta = share_path.stat()
+    # Create the recycle bin repository and its parent path components
+    # like mkdir -p, copying on them the share root permissions and
+    # extended attributes:
+    for part in recycle_path.relative_to(share_path).parts:
+        cur = cur / part
+        cur.mkdir(exist_ok=True)
+        os.chown(cur, share_meta.st_uid, share_meta.st_gid)
+        shutil.copystat(share_path, cur)
 
 def run_daemon():
     while True:
@@ -101,6 +122,8 @@ def main():
     set_parser.add_argument("num_days", type=int, help="Retention in days (0 means keep forever)")
     del_parser = subparsers.add_parser("del_retention", help="Delete retention for a share")
     del_parser.add_argument("share_name", help="Share name")
+    init_parser = subparsers.add_parser("init_repository", help="Initialize the repository directory for a share")
+    init_parser.add_argument("share_name", help="Share name")
     subparsers.add_parser("run_daemon", help="Run the daily cleanup loop continuosly")
     subparsers.add_parser("purge_shares", help="Run cleanup procedure immediately")
     args = parser.parse_args()
@@ -110,6 +133,8 @@ def main():
         set_retention(args.share_name, args.num_days)
     elif args.command == "del_retention":
         del_retention(args.share_name)
+    elif args.command == "init_repository":
+        init_recycle_directory(args.share_name)
     elif args.command == "run_daemon":
         run_daemon()
     elif args.command == "purge_shares":

tests/10__configure_domain.robot

@@ -13,19 +13,19 @@ Retrieve defaults
     ${response} =    Run task    module/${MID1}/get-defaults    {"provision":"new-domain"}
     Set Global Variable    ${IPADDR}    ${response['ipaddress_list'][0]['ipaddress']}
     Should Not Be Empty    ${IPADDR}
+    Set Global Variable    ${DOMAIN}    ${MID1}.test
+    Set Global Variable    ${DOMSUFFIX}    dc=${MID1},dc=test
 
 Configure the domain
     [Tags]    create    instance
-    Set Global Variable    ${DOMAIN}    ${MID1}.test
-    Set Global Variable    ${DOMSUFFIX}    dc=${MID1},dc=test
     ${out}    ${err}     ${rc} =    Execute Command
     ...    api-cli run module/${MID1}/configure-module --data '{"provision":"new-domain","realm":"${DOMAIN}","nbdomain":"${nbdomain}","hostname":"${dc1_hostname}","ipaddress":"${IPADDR}","adminuser":"${admuser}","adminpass":"${admpass}"}'
     ...    return_rc=True  return_stdout=True  return_stderr=True
     Should Be Equal As Integers    ${rc}  0
+
+The server is reachable after restart
     ${val} =     Get server url    ${MID1}
     Set Global Variable    ${SURL}    ${val}
     Wait Until Keyword Succeeds    10s    1s    RootDSE is correct    ${SURL}
-
-The server is reachable after restart
     Execute Command    systemctl restart ${MID1}
     Wait Until Keyword Succeeds    10s    1s    RootDSE is correct    ${SURL}