[botdeniska]

Contribution Licensing (no CLA, inbound=outbound, DCO required)

This pull request contributes to PGSTY SILO (pgsty/silo). Code contributions
are accepted under AGPL-3.0-or-later, the same license as the server.
This project does not use a CLA or require a separate Apache-2.0 license grant.
By submitting this pull request I represent that I have the right to contribute
the code changes under this repository's
GNU Affero General Public License v3.0 or later
and retain copyright in my original work. Existing copyright and license
notices remain intact; separately licensed material keeps its applicable terms.
Every commit must carry a DCO Signed-off-by trailer
(git commit -s) certifying the
Developer Certificate of Origin — see
CONTRIBUTING.md.

Description

Fixes #271.

9faeb7a recognizes an IAM record name that no drive can store by the error it produces. That works on a single drive. On an erasure set every drive fails with its own raw ENAMETOOLONG path error, the errors never agree, and the object layer reports a quorum failure, which iamConfigNameTooLong rightly does not accept. LDAP logins of users with a long normalized DN then fail with 500, and attaching a policy to such a DN returns 503.

This change applies the same name criterion before the drives are asked, so both topologies behave alike.

One commit:

fix(iam): decide unstorable IAM record names before asking the drives

1. iamConfigNameUnstorable(objPath) (cmd/iam-object-store.go) holds the name part of iamConfigNameTooLong, unchanged: a valid key (checkBucketAndObjectNames) with an ASCII element over 255 bytes. iamConfigNameTooLong(objPath, err) keeps its contract: a length error for such a key.
2. loadIAMConfigBytesWithMetadata returns errConfigNotFound for such a key without reading. Before, it read first and mapped the error afterwards, which covered only the single-drive case.
3. saveIAMConfig returns ObjectNameInvalid{Bucket: .minio.sys, Object: <key>} for such a key without writing: what a single drive reports today, and what iamConfigNameTooLong and the existing tests expect from a failed save. On an erasure set this turns a 503 into a 400.
4. New TestIAMLongNameRecordsOnErasureSet (cmd/iam-long-dn_test.go), run on a single drive and on an erasure set of 16 drives in LDAP mode, with the parent directories present: for a Cyrillic DN of 305 bytes once normalized, the parent revision read and PolicyDBGet find nothing and return the group policy; attach, detach (a tombstone write) and saving a revocation fail as name-too-long; the lookups still work afterwards.

Deletes are not changed: deleting such a key already returns nil on both topologies.

Motivation and Context

In a directory with non-Latin DNs (for example Cyrillic Active Directory), a user whose escaped DN exceeds 255 bytes cannot log in on any erasure set as soon as one user-level policy mapping exists. The existing tests use a single-drive fixture, so they pass. Details and an LDAP script are in the issue.

How to test this PR?

go test ./cmd -count=1 -run '^TestIAMLongNameRecordsOnErasureSet$' -v

On main, with this test file (git checkout <this branch> -- cmd/iam-long-dn_test.go), the single-drive case passes and the erasure-set case fails on every step (lines shortened):

=== RUN   TestIAMLongNameRecordsOnErasureSet/erasure_set
    iam-long-dn_test.go:104: before writes: parent revision: {...}, Storage resources are insufficient for the read operation .minio.sys/config/iam/users/cn=\d0\98...
    iam-long-dn_test.go:104: before writes: policy lookup: Storage resources are insufficient for the read operation .minio.sys/config/iam/policydb/sts-users/cn=\d0\98...
    iam-long-dn_test.go:108: attach to the long DN: got Storage resources are insufficient for the read operation .minio.sys/config/iam/policydb/sts-users/cn=\d0\98...
    iam-long-dn_test.go:111: revocation of the long DN: got Storage resources are insufficient for the read operation .minio.sys/config/iam/users/cn=\d0\98...
    iam-long-dn_test.go:115: detach from the long DN: got Storage resources are insufficient for the read operation .minio.sys/config/iam/policydb/sts-users/cn=\d0\98...
    iam-long-dn_test.go:117: after writes: parent revision: {...}, Storage resources are insufficient for the read operation ...
    iam-long-dn_test.go:117: after writes: policy lookup: Storage resources are insufficient for the read operation ...
--- FAIL: TestIAMLongNameRecordsOnErasureSet (0.11s)
    --- PASS: TestIAMLongNameRecordsOnErasureSet/single_drive (0.01s)
    --- FAIL: TestIAMLongNameRecordsOnErasureSet/erasure_set (0.10s)

Run locally on Go 1.27.3 (darwin/arm64), on main 0cfca29cb:

• make verifiers (golangci-lint: 0 issues), make build
• the existing long-name tests: go test ./cmd -count=1 -run '^(TestIAMConfigNameTooLong|TestLoadIAMRevision.*)$': ok
• IAM, STS, policy and revision tests: go test ./cmd -count=1 -run '^Test(IAM|STS|.*LDAP|.*Policy|.*Revision|.*Revoc).*': ok
• the Docker-backed LDAP suites against quay.io/minio/openldap: _MINIO_LDAP_TEST_SERVER=localhost:13391 go test ./cmd -count=1 -run '^Test.*LDAP.*$': ok

I did not run the whole ./cmd suite.

Manual check with the script from the issue (OpenLDAP, one server with 4 drives): after a user-level mapping exists, the long-DN login goes from 500 to 200, and attaching a policy to the DN from 503 to 400.

Compatibility impact

No API, configuration, storage metadata, metric, route or dependency changes.

• Erasure sets now treat these names as a single drive already does: reads find no record, writes fail with ObjectNameInvalid (400 XMinioInvalidObjectName on the admin API) instead of a quorum error (500/503).
• Which names: exactly those iamConfigNameTooLong already accepted, an ASCII element over 255 bytes in an otherwise valid key. Unicode names and long total paths still go to the drives, as before.
• Cost: a scan of the key string on every IAM record read and write, which used to happen only after a failed read.
• etcd is not affected.
• Rollback brings back the quorum errors on erasure sets. Nothing is stored differently.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Optimization (provides speedup with no functional changes)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• All commits are signed off (git commit -s) per the DCO
• Fixes a regression (If yes, please add commit-id or PR # here): partly; completes 9faeb7a for erasure sets
• Unit tests added/updated
• make verifiers passes
• Relevant package tests and make build pass
• Compatibility and rollback impact documented
• Internal documentation updated (not needed)
• Public documentation update opened in pgsty/silo.pgsty.com, if needed (not needed)