Backport and fix of space membership expired event - #766
Conversation
maki5
changed the title
…ce_membership_expired_event
rhafer
left a comment
rhafer
left a comment
There was a problem hiding this comment.
Thanks for looking into this. This changes to the share manager look mostly fine to me, just change it to use the helper for formatting the SpaceId.
I don't think we need to changes for the storage provider (see my comments).
| if s.ResourceId.SpaceId == s.ResourceId.OpaqueId { | ||
| if err := events.Publish(ctx, m.eventStream, events.SpaceMembershipExpired{ | ||
| SpaceOwner: s.GetOwner(), | ||
| SpaceID: &provider.StorageSpaceId{OpaqueId: s.ResourceId.StorageId + "$" + s.ResourceId.SpaceId}, |
There was a problem hiding this comment.
| SpaceID: &provider.StorageSpaceId{OpaqueId: s.ResourceId.StorageId + "$" + s.ResourceId.SpaceId}, | |
| SpaceID: &provider.StorageSpaceId{OpaqueId: storagespace.FormatStorageID(s.ResourceId.StorageId, s.ResourceId.SpaceId)}, |
| if s.ResourceId.SpaceId == s.ResourceId.OpaqueId { | ||
| if err := events.Publish(ctx, m.eventStream, events.SpaceMembershipExpired{ | ||
| SpaceOwner: s.GetOwner(), | ||
| SpaceID: &provider.StorageSpaceId{OpaqueId: s.ResourceId.StorageId + "$" + s.ResourceId.SpaceId}, |
| if s.ResourceId.SpaceId == s.ResourceId.OpaqueId { | ||
| if err := events.Publish(ctx, m.eventStream, events.SpaceMembershipExpired{ | ||
| SpaceOwner: s.GetOwner(), | ||
| SpaceID: &provider.StorageSpaceId{OpaqueId: s.ResourceId.StorageId + "$" + s.ResourceId.SpaceId}, |
| if s.ResourceId.SpaceId == s.ResourceId.OpaqueId { | ||
| if err := events.Publish(ctx, m.eventStream, events.SpaceMembershipExpired{ | ||
| SpaceOwner: s.GetOwner(), | ||
| SpaceID: &provider.StorageSpaceId{OpaqueId: s.ResourceId.StorageId + "$" + s.ResourceId.SpaceId}, |
| if s.ResourceId.SpaceId == s.ResourceId.OpaqueId { | ||
| if err := events.Publish(ctx, m.eventStream, events.SpaceMembershipExpired{ | ||
| SpaceOwner: s.GetOwner(), | ||
| SpaceID: &provider.StorageSpaceId{OpaqueId: s.ResourceId.StorageId + "$" + s.ResourceId.SpaceId}, |
There was a problem hiding this comment.
If we really need these changes, they should go into pkg/storage/pkg/decomposedfs/spaces.go. The implementation in pkg/storage/utils is there mostly for historic reference to the "old" ocis driver.
(This is causing confusion very often, probably we should finally remove it 🤔 )
But do we really need those changes here at all? We're already sending the event in the share manager, sending it here again mean we'd emit the event twice per expired space membership.
It would be nice though to have an event listener that consumes the SpaceMembershipExpired Events and removes the associated grants from the storage provider.
There was a problem hiding this comment.
agree, basically I was not able to reproduce this method to be fired on my local tests, seems like this backport is dead code right now and removing it makes sense
3 participants
this change is needed for
opencloud-eu/opencloud#3257