Might be better handled in future by new proposed approach with DKIM files generated having a complimentary rspamd.k or rspamd.yaml config beside the keypair files (although the proposed file layout covers a fair amount of that information itself):

• [TODO]: Consider revising DKIM generation #3630
• misc: final Rspamd adjustments for v13 #3599 (comment)
• misc: final Rspamd adjustments for v13 #3599 (comment)
• https://github.com/orgs/kcl-lang/discussions/804#discussioncomment-7430551

Example fetches the separate configs and produces output YAML (dynamic on-demand generation for rspamd config instead of manually editing/updating the domain section):

$ docker run --rm --pull=always -v "/tmp/config:/config:ro" kcllang/kcl:main bash -c \
  'find /config/dkim/keys -type f -name "rspamd.k" | xargs kcl /config/main.k'

main: Pulling from kcllang/kcl
Digest: sha256:094150067c1c1079e320dc1f6a1c2ed68dcf92d09d1f428e8dd760c96f619ab6
Status: Image is up to date for kcllang/kcl:main

domain:
  example.com:
    selectors:
    - path: /config/dkim/keys/example.com/mail-ec/private.pem
      selector: mail-ec
    - path: /config/dkim/keys/example.com/mail-rsa/private.pem
      selector: mail-rsa
  example.org:
    selectors:
    - path: /config/dkim/keys/example.org/mail-rsa/private.pem
      selector: mail-rsa

Alternative example fetches the file paths of the separate configs to generate a YAML list as config:

# Find the `rspamd.k` files and output a YAML list (filepath prefixed with `- `) as input into `yq`:
# NOTE: Just pipe to `yq` with nothing else after to get a pure YAML list from the stdin input
$ find /config/dkim/keys -type f -name "rspamd.k" -exec printf '- %s\n' {} + \
  | yq '{ "kcl_cli_configs": { "files": . } }'

kcl_cli_configs:
  files:
    - /config/dkim/keys/example.com/mail-ec/rspamd.k
    - /config/dkim/keys/example.com/mail-rsa/rspamd.k
    - /config/dkim/keys/example.org/mail-rsa/rspamd.k

Easy to generate and iterate over in bash similar to what you're doing above if needed (the .k / KCL files can be ignored and stick to plain YAML, unless you want config features like immutability).

Just as a note for me: will be deferred to a future PR where DKIM is reworked.

Probably a better way to handle that check than running cat like this? @casperklein would probably have good input here.

Presumably using stat to check the expected ownership/permissions is what you expect is best (and more informative to maintainers?).

If #3630 was implemented (at least the proposed filesystem layout for keypair files), you could have the setup scripts copy those over to an internal location and this whole concern would be a non-issue making this check redundant?

check-for-changes.sh could additionally monitor the config volume should the keypair change, copying over the update and restarting/reloading the opendkim/rspamd service if needed?

Probably a better way to handle that check than running cat like this? @casperklein would probably have good input here.

Presumably using stat to check the expected ownership/permissions is what you expect is best (and more informative to maintainers?).

This could indeed be more elaborate; but as I said, it ought to be simple for now. We could use stat indeed, but then we'd need logic to check all possible cases - remember, we just want the _rspamd user to be able to read these files. Matching against permissions is more complicated here. As you also said yourself:

If #3630 was implemented (at least the proposed filesystem layout for keypair files), you could have the setup scripts copy those over to an internal location and this whole concern would be a non-issue making this check redundant?

I want this piece of code to be easy so that we can throw it away once #3630 is implemented. Until then, I actually think this addition is worthwhile.

check-for-changes.sh could additionally monitor the config volume should the keypair change, copying over the update and restarting/reloading the opendkim/rspamd service if needed?

I like this idea! 🚀

We could use stat indeed, but then we'd need logic to check all possible cases - remember, we just want the _rspamd user to be able to read these files. Matching against permissions is more complicated here.

if [[ $(stat --format '%a %U %G' "${FILE}" 2>/dev/null) != '775 _rspamd docker' ]]; then
  ...
fi

if [[ $(find "${FILE}" -perm -ugo=r 2>/dev/null) ]]; then
  ...
fi

-perm MASK      At least one mask bit (+MASK), all bits (-MASK),
                        or exactly MASK bits are set in file's mode

This should technically be valid:

# Internally _rspamd would only be valid as user/group,
# otherwise require read permission for `other` perms:
if [[ $(find "${FILE}" -user _rspamd -or -group _rspamd -or -perm -o=r 2>/dev/null) ]]; then
  ...
fi

This could indeed be more elaborate; but as I said, it ought to be simple for now.
I want this piece of code to be easy so that we can throw it away once #3630 is implemented.

If that's likely to be addressed before the end of the year, that seems fine 👍

Otherwise the find approach seems simple enough and conveys expectation a bit better?

You proposal works on the (quite awful) Bash logic in which if [[ ${EMPTY} ]]; then evaluates to false and if [[ ${NOT_EMPTY} ]]; then evaluates to true, but I like the concept, so I changed it:

# See https://serverfault.com/a/829314 for an explanation on `-exec false {} +`
# We additionally resolve symbolic links to check the permissions of the actual files
if find "$(realpath -eL "${FILE}")" -user _rspamd -or -group _rspamd -or -perm -o=r -exec false {} +; then
  __rspamd__log 'warn' "Rspamd DKIM private key file '${FILE}' does not appear to have correct permissions/ownership for Rspamd to use it"
else
  __rspamd__log 'trace' "DKIM file '${FILE}' permissions and ownership appear correct"
fi

PS: Looking at both approaches, I don't know which of the two is worse 😆