Skip to content

fix(decomposedfs): skip a space whose name attribute is unset#710

Closed
michaelstingl wants to merge 1 commit into
opencloud-eu:mainfrom
michaelstingl:fix/1878-listspaces-graceful-missing-name
Closed

fix(decomposedfs): skip a space whose name attribute is unset#710
michaelstingl wants to merge 1 commit into
opencloud-eu:mainfrom
michaelstingl:fix/1878-listspaces-graceful-missing-name

Conversation

@michaelstingl

@michaelstingl michaelstingl commented Jun 27, 2026

Copy link
Copy Markdown

Description

ListStorageSpaces fails when it reads a space node that has no user.oc.name attribute. The read returns node.Xattr ... user.oc.name: no data available. This state appears after the owning user is deleted while a grant still points at the node, for example after inviting an already-deleted user to a space.

Two paths in pkg/storage/pkg/decomposedfs/spaces.go handle that read error differently:

  • direct read (an id filter): returns the error and fails the whole call with gRPC code 15 ("error listing spaces").
  • worker loop: already skips the node, but logs it at error level on every listing.

The fix treats such a node as non-existent, like the adjacent !n.Exists branch: it matches metadata.IsAttrUnset and skips it. The direct read returns an empty result, the worker loop continues, and the skip is logged at debug. Other read errors are unchanged.

Related Issue

Part of opencloud-eu/opencloud#1878

This fixes the storage-users error reported there. It does not clean up the orphaned index entries, and the search service hits the same node at a separate site in the opencloud repository, so the issue is not fully resolved.

Motivation and Context

On the list-all path the call still returns and the failure shows up only as error-log lines, repeated on every listing that reads such a node. The id-filter path also fails the call, but that resolves a share for a now-gone user. The issue reports the log noise. The fix removes it and keeps one unreadable node from failing a listing.

How Has This Been Tested?

  • reproduction: OpenCloud 7.1.0 (posix), acceptance scenario apiSharingNgShareInvitation/shareInvitations.feature ("send share invitation to deleted user"). The server logs the code-15 error during the run; the scenario itself passes.
  • unit test in spaces_test.go: remove a space node's name attribute, then check that ListStorageSpaces returns empty for an id filter and still returns the healthy spaces for a list-all. Red on main, green with this change.
  • with test(decomposedfs): purge project spaces as a delete-all-spaces user #709 applied on top, the full decomposedfs package passes in a golang:1.25 container on an ext4 volume (102 specs); gofmt, go vet and golangci-lint v2.10.1 are clean.

Note

On its own, CI shows three failing decomposedfs purge tests until #709 merges: they broke when #695 merged on top of #672 (a semantic conflict), predate this change, and are fixed by #709. The change here is verified green on top of #709 (above), and this branch will be rebased onto green main after #709 merges.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation added (n/a: reva builds the changelog from the PR title and Type:* label via ready-release-go, so no fragment is needed)

🤖 drafted with Claude Code, reviewed before submitting.

ListStorageSpaces failed when it resolved a node whose name attribute is
gone, which happens when the owning user is deleted (for example by
inviting an already-deleted user to a space). The direct-read path
returned "node.Xattr ... user.oc.name: no data available" and failed the
whole listing with gRPC code 15, and the worker-loop path logged it at
error level on every listing. The result is server error-log spam on
every space listing that touches such a node, while the listing's API
response is unaffected.

Treat a node with an unset name attribute as non-existent, the same way
the existing !n.Exists branch and the worker-loop sibling already handle
a missing node: match metadata.IsAttrUnset and skip the node (the
direct-read path returns an empty result, the worker loop continues),
logged at debug. Any other read error keeps its existing behaviour.

Related: opencloud-eu/opencloud#1878
@michaelstingl

Copy link
Copy Markdown
Author

Closing in favor of #713, which fixes the same root cause one level down.

Both branches here key on metadata.IsAttrUnset(err) from node.ReadNode. #713 swallows that at the source (matching the owner lookup right above it), so these two branches can never fire once it lands, and it covers every other ReadNode caller too, not just the two ListStorageSpaces sites. StorageSpaceFromNode already tolerates a missing name downstream, so no separate skip is needed. If a nameless orphan should be hidden from listings, that is a small name filter on top of #713.

🤖 drafted with Claude Code, reviewed before submitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant