fix/Child objects ACL #283

carpawell · 2024-01-12T02:25:26Z

No description provided.

object/types.proto

roman-khimov · 2024-01-18T14:42:56Z

Judging from nspcc-dev/neofs-sdk-go#543 we're missing at least offset data for #264. And I'm worried about #263 as well.

carpawell · 2024-01-22T08:17:29Z

Judging from nspcc-dev/neofs-sdk-go#543 we're missing at least offset data for #264. And I'm worried about #263 as well.

Yes, I separate them to make every change smaller but merge them with a few PRs. Do you think we need to change them at once?

roman-khimov · 2024-01-22T09:59:45Z

Let's make a complete spec for the new format first, then implement it step by step fixing along the way if needed.

carpawell · 2024-01-23T18:22:51Z

A few intermediate questions:

Should the link object be a separate object type? There are no requirements (at least for now) for it. Making a separate type for it certainly makes it harder to adopt in the node and may increase the number of unexpected bugs. Although, I just feel like it should be a separate object type.
Is it OK to have a "broken" parent header in the initial object? It just cannot have any correct payload-dependent fields.
Do we need some structure for the initial object too? Should it have a payload or parent object is ok to be in the header like it was before?

My answers:

Yes (but be ready it takes much more time to implement)
No
Not sure. Want to say "yes" since it is clearer to me but it would be too many different messages and types IMO (especially with 2.).

roman-khimov · 2024-01-23T18:58:08Z

A separate type likely will make it easier to adopt, you'll be able to separate the old one from the new easily.
It can have a lot of data in many cases, what's broken about it? Hashes?
It has a special type, then it depends on what else we need. Payload should OK.

carpawell · 2024-01-24T07:38:46Z

A separate type likely will make it easier to adopt

If we make it from scratch -- yes. But currently a lot of places think that link object is a regular object. I can not even imagine how many. Moreover, backward compatibility is a question here: there are places that are not ready to get anything except REGULAR, TOMB, LOCK. But I do support you.

It can have a lot of data in many cases, what's broken about it? Hashes?

No ID, no payload length (in a general case), no hashes. That is not a header I would say. Previously, "parent" was a ready term in NeoFS. If you saw "parent" somewhere, you know what it was and were able to get anything you want about it. But not finished header solves our problem for now.

It has a special type, then it depends on what else we need. Payload should OK.

So you think INIT object should have a payload? Or not? Currently it does not have it and parent is placed in the header's split.parent field. If it is a separate message in its payload, we may think about storing parent header not like a header but like a new struct without "payload" fields.

cthulhu-rider · 2024-01-24T07:44:46Z

link/types.proto

+// IDs. IDs MUST be ordered according to the original payload split, meaning the
+// first payload part holder MUST be placed at the first place in the corresponding
+// link object.
+message Link {


really deserves a separate package? to me its overhead. object fits well, or at least in refs

depends on the final solution about a separate object type. if it is a separate object type, i do not see any difference with tombstone, storagegroup and lock pkgs. but if you ask me, should it be like that, my answer is no, but we have what we have, do not want this object type to be different in any way

imo the previously used approach only complicates the structuring and, as a consequence, the knowledge of the protocol concepts. All these entities are inextricably linked with objects - they are about objects, they are in the payload of objects. Сurrent packetization is extremely redundant to me

we have what we have

obviously, but we are not burdened with creating a new package (). Keep the existing ones, and this message can be left in object to comply with the system architecture hierarchy

being a newbie, i'll see acl, container, object and other large sections of the system. And then link - specific detail elevated to the top level of the system with a non-descript name

to comply with the system architecture hierarchy

so do i. type of the objects are placed where they always have been

the issue was created #284

👍 for issue, link is still under the question

link/types.proto

object/types.proto

link/types.proto

cthulhu-rider · 2024-01-24T10:37:09Z

separate type link

the concept is very simple: if payload format is strict, then separate type is required. Separate type also grants efficient SEARCH power (if needed). Ofc some servers could deny service of unsupported types, so LINK objects must be essentially optional within the current protocol version. It’s nice that link objects in the current scheme are optional, this will allow both of its variations to be present in the same network

Do we need some structure for the initial object too?

it cannot do without logical features expressed in structural differences. For example, there should be no incompatible collisions with the current split schema. About the structure itself, i think it should be as efficient as possible to solve exact problems regardless of the server implementation

carpawell · 2024-01-24T15:17:17Z

link/types.proto

+// ID. It is NOT required for the original object assembling. It MUST have ALL
+// the "child objects" IDs. Child objects MUST be ordered according to the
+// original payload split, meaning the first payload part holder MUST be placed
+// at the first place in the corresponding link object. Sizes MUST NOT be omitted


if we wanna use it, we should be able to rely on it so there is MUST word. but then, we need to validate it and that is N head requests per link object PUT (N is num of objects in the chain)

N head requests per link object

the problem is more specific - when assembling a split object, the list of descriptors of child objects can diverge with the stored children (not only the sizes btw, IDs too). The proposed validation is correct, but since we cannot guarantee it at the protocol level (this is a server implementation detail), the assembler - one who operates with split chain - must be prepared for the divergence in any case

protocol-level solution is to additionally store such metadata in the child object headers themselves (mentioned here). Then the link object will remain a pure stand-alone helper

but since we cannot guarantee it at the protocol level (this is a server implementation detail)

what do you mean? nodes may not accept such link objects

must be prepared for the divergence in any case

there is no solution for #264 then. if you cannot trust the link object, you cannot be sure you are answering with a correct range

protocol-level solution is to additionally store such metadata in the child object headers themselves

should also be validated somehow. a node can get Nth object with some offset in it. what does it do? just believes it is true? searches for the previous ones and calculates their sizes?

@roman-khimov

what do you mean? nodes may not accept such link objects

nodes may do whatever they want. They may accept linking objects w/o any validation

there is no solution for #264 then

i dont deny current solution, im just highligting that linking object can be unavailable or diverge with the split chain and any network node must be ready for this. In these cases, extra metadata in child objects may be a painkiller in some cases

what does it do?

its own implementation detail, each node can only control what to do itself and what it receives from other nodes

or maybe i misunderstood ur original comment. I dont see any problem with current structure and requirements. If u didnt mean that N head requests per link object PUT is a problem, then everything is ok to me

nodes may do whatever they want

what does it do?

its own implementation detail, each node can only control what to do itself and what it receives from other nodes

i meant we need to find a balance to implement our "perfect" node that may exist in the world of "worst" nodes. if we may not rely on link object, i think the whole work is useless. but if we want to rely on it, we should validate it somehow

extra metadata in child objects may be a painkiller in some cases

here i want to say that ensuring child object data is ok is the same to ensuring similar info about the link object

If u didnt mean that N head requests per link object PUT is a problem, then everything is ok to me

yes. this and the consequences

Validation should be implemented. Then I'd be optimistic wrt the link object contents. If it's not correct --- either you get an error (trying to access inexisting object) or reply with a wrong result, but it's not node's fault, garbage in -- garbage out.

Validation should be implemented.

Yes, sure. Just not sure about some link object replication: imagine you have a 1к parts object and receive a link to it for the first time: you need to do 1k head objects before you finish with this object. And you may be an attacker that sends 1k of such 1k-parts objects. 64mb data for a HEAD chaos in the NeoFS network.

object/types.proto

cthulhu-rider · 2024-01-25T07:24:32Z

link/types.proto

+// ID. It is NOT required for the original object assembling. It MUST have ALL
+// the "child objects" IDs. Child objects MUST be ordered according to the
+// original payload split, meaning the first payload part holder MUST be placed
+// at the first place in the corresponding link object. Sizes MUST NOT be omitted


N head requests per link object

the problem is more specific - when assembling a split object, the list of descriptors of child objects can diverge with the stored children (not only the sizes btw, IDs too). The proposed validation is correct, but since we cannot guarantee it at the protocol level (this is a server implementation detail), the assembler - one who operates with split chain - must be prepared for the divergence in any case

protocol-level solution is to additionally store such metadata in the child object headers themselves (mentioned here). Then the link object will remain a pure stand-alone helper

link/types.proto

object/types.proto

cthulhu-rider · 2024-01-25T07:51:13Z

link/types.proto

+// IDs. IDs MUST be ordered according to the original payload split, meaning the
+// first payload part holder MUST be placed at the first place in the corresponding
+// link object.
+message Link {


imo the previously used approach only complicates the structuring and, as a consequence, the knowledge of the protocol concepts. All these entities are inextricably linked with objects - they are about objects, they are in the payload of objects. Сurrent packetization is extremely redundant to me

we have what we have

obviously, but we are not burdened with creating a new package (). Keep the existing ones, and this message can be left in object to comply with the system architecture hierarchy

being a newbie, i'll see acl, container, object and other large sections of the system. And then link - specific detail elevated to the top level of the system with a non-descript name

cthulhu-rider

format is ok, left some discussions about docs

roman-khimov

Can we have a behavior spec in https://github.com/nspcc-dev/neofs-spec/? I guess most of mechanics stays the same, except for INIT.

I'm also not sure how split_id/init interact, do we need split_id if init is present?

object/types.proto

roman-khimov · 2024-01-29T20:21:18Z

link/types.proto

+// ID. It is NOT required for the original object assembling. It MUST have ALL
+// the "child objects" IDs. Child objects MUST be ordered according to the
+// original payload split, meaning the first payload part holder MUST be placed
+// at the first place in the corresponding link object. Sizes MUST NOT be omitted


Validation should be implemented. Then I'd be optimistic wrt the link object contents. If it's not correct --- either you get an error (trying to access inexisting object) or reply with a wrong result, but it's not node's fault, garbage in -- garbage out.

carpawell · 2024-01-30T15:34:53Z

Can we have a behavior spec in https://github.com/nspcc-dev/neofs-spec/?

You mean before this PR merge? Let's have at least approve+draft to ensure we agree about this PR. I can't continue before we are done here, it has already changed 3 times.

I'm also not sure how split_id/init interact, do we need split_id if init is present?

I'm not sure init object changes something here: if the link and the init objects are lost and there is no SplitID, there is no ability to assembly the original object (except you find somehow the latest part and compare its parent header). Also, you cannot update any indexes related to the original object if you meet some object part before the init or the link object (possible situation, why not?). cc @cthulhu-rider

roman-khimov · 2024-01-30T16:12:07Z

To me, init can completely replace split_id. But if it's better to keep split_id for compatibility/recovery cases --- ok. The problem with missing spec is that these objects/fields are just data, while it's important how exactly we treat this data in various circumstances (init/split_id for example). At the same time, data-wise this seems to be complete, split_id can't be removed anyway.

…#283 Signed-off-by: Pavel Karpy <[email protected]>

…#283 Keep both versions of the split objects and mark them as v1 and v2. Signed-off-by: Pavel Karpy <[email protected]>

It describes future protocol version's link object payload. Child objects list will be moved from the header to the payload. This is done due to the header size restrictions. Closes #263. Signed-off-by: Pavel Karpy <[email protected]>

carpawell · 2024-02-05T15:43:38Z

no INIT object, the first part is used as a parent object's header holder
LINK object is a separate type
SplitID is deprecated, the first part is used instead (so it is placed in every part now)

cthulhu-rider · 2024-02-05T16:50:54Z

@carpawell closes #264?

It allows faster seeking through a split object without fetching the whole chain. Closes #264. Signed-off-by: Pavel Karpy <[email protected]>

This commit makes it easier to differ link objects from the other types. Object split hierarchy rework increases the link object's structure and makes it more strictly formatted, so now it plays a more important role in the split chains (and the split rules became more complex too). Signed-off-by: Pavel Karpy <[email protected]>

There is no need to generate some UUID with non-specified rules if the first object part allows the same identification routines but uses hashes widely accepted in the protocol. Signed-off-by: Pavel Karpy <[email protected]>

carpawell · 2024-02-05T17:02:56Z

closes #264?

Yes, added it to one of the commits.

…#283 Keep both versions of the split objects and mark them as v1 and v2. Signed-off-by: Pavel Karpy <[email protected]>

carpawell self-assigned this Jan 12, 2024

carpawell changed the title ~~fix/Child-objects-acl~~ fix/Child objects ACL Jan 12, 2024

roman-khimov reviewed Jan 18, 2024

View reviewed changes

object/types.proto Outdated Show resolved Hide resolved

carpawell force-pushed the fix/child-objects-acl branch from 4b26e9e to ae6f915 Compare January 23, 2024 18:01

carpawell requested a review from roman-khimov January 23, 2024 18:22

carpawell force-pushed the fix/child-objects-acl branch from ae6f915 to da4aa5f Compare January 23, 2024 18:24

cthulhu-rider reviewed Jan 24, 2024

View reviewed changes

carpawell mentioned this pull request Jan 24, 2024

Move "objects" to object dir #284

Open

carpawell force-pushed the fix/child-objects-acl branch from da4aa5f to 4dc7ec0 Compare January 24, 2024 15:13

carpawell commented Jan 24, 2024

View reviewed changes

object/types.proto Outdated Show resolved Hide resolved

carpawell marked this pull request as ready for review January 24, 2024 15:19

cthulhu-rider reviewed Jan 25, 2024

View reviewed changes

carpawell requested a review from cthulhu-rider January 25, 2024 15:23

carpawell force-pushed the fix/child-objects-acl branch from 4dc7ec0 to 7b98b6a Compare January 25, 2024 15:23

cthulhu-rider previously approved these changes Jan 25, 2024

View reviewed changes

roman-khimov reviewed Jan 29, 2024

View reviewed changes

roman-khimov previously approved these changes Jan 30, 2024

View reviewed changes

carpawell dismissed stale reviews from roman-khimov and cthulhu-rider via e5b2df3 January 31, 2024 07:17

carpawell force-pushed the fix/child-objects-acl branch from 7b98b6a to e5b2df3 Compare January 31, 2024 07:17

carpawell force-pushed the fix/child-objects-acl branch from e5b2df3 to bbeae5c Compare January 31, 2024 13:42

carpawell added a commit to nspcc-dev/neofs-spec that referenced this pull request Jan 31, 2024

arch: Adapt object split information according to nspcc-dev/neofs-api…

b47ab7d

…#283 Signed-off-by: Pavel Karpy <[email protected]>

carpawell added a commit to nspcc-dev/neofs-spec that referenced this pull request Feb 1, 2024

arch: Adapt object split information according to nspcc-dev/neofs-api…

4ccc6e2

…#283 Signed-off-by: Pavel Karpy <[email protected]>

carpawell mentioned this pull request Feb 1, 2024

Upd/split object updates nspcc-dev/neofs-spec#97

Merged

carpawell force-pushed the fix/child-objects-acl branch from bbeae5c to 9df9a1b Compare February 2, 2024 09:53

link: Create link object payload message

fa2080f

It describes future protocol version's link object payload. Child objects list will be moved from the header to the payload. This is done due to the header size restrictions. Closes #263. Signed-off-by: Pavel Karpy <[email protected]>

carpawell force-pushed the fix/child-objects-acl branch from 9df9a1b to fcbf8f8 Compare February 5, 2024 15:24

carpawell requested review from roman-khimov and cthulhu-rider February 5, 2024 15:43

roman-khimov approved these changes Feb 5, 2024

View reviewed changes

cthulhu-rider approved these changes Feb 5, 2024

View reviewed changes

carpawell added 3 commits February 5, 2024 20:01

link: Add children sizes information

39d130d

It allows faster seeking through a split object without fetching the whole chain. Closes #264. Signed-off-by: Pavel Karpy <[email protected]>

object: Use the first object part as a split ID

a825a7e

There is no need to generate some UUID with non-specified rules if the first object part allows the same identification routines but uses hashes widely accepted in the protocol. Signed-off-by: Pavel Karpy <[email protected]>

carpawell force-pushed the fix/child-objects-acl branch from fcbf8f8 to a825a7e Compare February 5, 2024 17:02

cthulhu-rider merged commit 533950f into master Feb 5, 2024
3 checks passed

cthulhu-rider deleted the fix/child-objects-acl branch February 5, 2024 17:08

cthulhu-rider mentioned this pull request Feb 15, 2024

fix/Child objects ACL nspcc-dev/neofs-sdk-go#543

Merged

carpawell mentioned this pull request Mar 21, 2024

Storage group (for complex object) test cannot live without children in the link's payload (split V2) nspcc-dev/neofs-testcases#755

Closed

carpawell mentioned this pull request Apr 23, 2024

Test attribute-based eACL for big objects nspcc-dev/neofs-testcases#786

Closed

carpawell mentioned this pull request May 3, 2024

GC object parts test nspcc-dev/neofs-testcases#794

Closed

roman-khimov mentioned this pull request Sep 3, 2024

Reconsider using eacl filters by object name nspcc-dev/neofs-s3-gw#642

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix/Child objects ACL #283

fix/Child objects ACL #283

carpawell commented Jan 12, 2024

roman-khimov commented Jan 18, 2024 •

edited

Loading

carpawell commented Jan 22, 2024

roman-khimov commented Jan 22, 2024

carpawell commented Jan 23, 2024

roman-khimov commented Jan 23, 2024

carpawell commented Jan 24, 2024 •

edited

Loading

cthulhu-rider Jan 24, 2024

carpawell Jan 24, 2024 •

edited

Loading

cthulhu-rider Jan 25, 2024

carpawell Jan 25, 2024

cthulhu-rider Jan 25, 2024

cthulhu-rider commented Jan 24, 2024 •

edited

Loading

carpawell Jan 24, 2024

cthulhu-rider Jan 25, 2024

carpawell Jan 25, 2024

cthulhu-rider Jan 25, 2024

carpawell Jan 26, 2024

roman-khimov Jan 29, 2024

carpawell Jan 30, 2024

cthulhu-rider Jan 25, 2024

cthulhu-rider Jan 25, 2024

cthulhu-rider left a comment

roman-khimov left a comment

roman-khimov Jan 29, 2024

carpawell commented Jan 30, 2024

roman-khimov commented Jan 30, 2024

carpawell commented Feb 5, 2024

cthulhu-rider commented Feb 5, 2024

carpawell commented Feb 5, 2024

fix/Child objects ACL #283

fix/Child objects ACL #283

Conversation

carpawell commented Jan 12, 2024

roman-khimov commented Jan 18, 2024 • edited Loading

carpawell commented Jan 22, 2024

roman-khimov commented Jan 22, 2024

carpawell commented Jan 23, 2024

roman-khimov commented Jan 23, 2024

carpawell commented Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

carpawell Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cthulhu-rider commented Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cthulhu-rider left a comment

Choose a reason for hiding this comment

roman-khimov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell commented Jan 30, 2024

roman-khimov commented Jan 30, 2024

carpawell commented Feb 5, 2024

cthulhu-rider commented Feb 5, 2024

carpawell commented Feb 5, 2024

roman-khimov commented Jan 18, 2024 •

edited

Loading

carpawell commented Jan 24, 2024 •

edited

Loading

carpawell Jan 24, 2024 •

edited

Loading

cthulhu-rider commented Jan 24, 2024 •

edited

Loading