Repository navigation
seaweedfs: follow-ups for the Object Lock actions hook from #4893 #4894
Description
The Object Lock actions hook from #4893 has these behaviour and test-coverage gaps.
- If an identity is deleted between the hook's dry run and its apply,
s3.configuredoes not find it and creates a new identity with no keys and only the Object Lock actions. It grants nothing to anyone, but it leaves a stray record in IAM. A separate existence check can narrow that window but cannot remove the race. - A hook pod that never starts (image pull failure, exhausted quota) can fail the Helm install or upgrade action without running the script. The script reports handled read and grant failures without returning an error, but that cannot help when the pod never starts.
- The Warning Event written on a failed grant expires with the cluster event TTL, one hour by default, and after that only the Job log is left. The Job also stays in the namespace after the app is removed, because Helm does not delete hook resources on uninstall.
ttlSecondsAfterFinishedof a few days would keep the log readable and still clean it up. - An identity name with a newline followed by a YAML document marker breaks the Event manifest. The marker is
---or...followed by a space or the end of the line, at the start of a line in the message.kubectl createfails and the script logs it, so only the Event is lost. A plain newline folds into a space and the Event is created. - No bats case covers an empty identity list printed as indented JSON, which is what a fresh install returns. The code handles it.
- No bats case covers SeaweedFS 4.45 failing a read with empty output and exit code 0. The code treats it as a failed read.
- helm-unittest checks the hook weight and delete policy only on the ConfigMap and the Job, not on the ServiceAccount, Role and RoleBinding.
Reacted by dosubot
Activity
lexfrei commentedon Oct 13, 2026
More from the review of #4879, all about test coverage of the backported s3 HTTPRoute:
s3.portfromfiler.s3.port, because both default to 8333. Swapping the two port branches, or dropping the standalone one, still passes. A different port for one of them in the test closes both.allInOnebranches of both routes have no tests. Cozystack does not use allInOne, so this one is optional.lexfrei commentedon Nov 12, 2026
One more limit of the hook, found while reviewing #4915: it does not tell left-over identities from live ones. A
ba-<uid>identity whose BucketAccess is already gone still has Read and Write, so the hook adds the Object Lock actions to it as well. Its keys keep working until the file is removed, now with the extra actions.#4915 now runs its cleanup before this hook, so with
cosi.orphanIdentitiesresolving todeletethose files are gone before any grant. Inreportmode, whichautoselects whenfiler.grpcHostis set, they stay and get the actions on every install and upgrade. Skippingba-*identities that no BucketAccess names would close it, using the same check #4915 already does.