object: Validate attribute EXPIRATION_EPOCH on put #1188

acid-ant · 2024-06-18T12:11:49Z

acid-ant commented

2024-06-18 12:11:49 +00:00

Close #1109
Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Close #1109 Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>

acid-ant requested review from storage-core-committers 2024-06-18 12:14:41 +00:00

acid-ant requested review from storage-core-developers 2024-06-18 12:14:57 +00:00

fyrchik reviewed 2024-06-18 12:23:55 +00:00

pkg/core/object/fmt.go Outdated

					
				@ -127,0 +129,4 @@

						if !errors.Is(err, errNoExpirationEpoch) {

							return fmt.Errorf("object did not pass expiration check: %w", err)

						}

					} else {

fyrchik commented

2024-06-18 12:23:55 +00:00

else if !unprepared?

Also, checkExpiration now contains a single if, we could move it here and rename checkExpiration to checkIfExpired. And checking whether expiration epoch is less than the current epoch is valid for both types of objects, the only check that is specific is IsLocked (because not-yet-created object cannot be locked).

`else if !unprepared`? Also, `checkExpiration` now contains a single `if`, we could move it here and rename `checkExpiration` to `checkIfExpired`. And checking whether `expiration epoch` is less than the current epoch is valid for both types of objects, the only check that is specific is `IsLocked` (because not-yet-created object cannot be locked).

acid-ant commented

2024-06-18 14:35:43 +00:00

OMG, updated. Thanks.

fyrchik marked this conversation as resolved

dstepanov-yadro reviewed 2024-06-18 12:30:09 +00:00

pkg/core/object/fmt.go Outdated

					
				@ -124,15 +124,24 @@ func (v *FormatValidator) Validate(ctx context.Context, obj *objectSDK.Object, u

						return fmt.Errorf("invalid attributes: %w", err)

					}

					exp, err := expirationEpochAttribute(obj)

dstepanov-yadro commented

2024-06-18 12:30:09 +00:00

Also it could happen, that user has defined both of attributes: objectV2.SysAttributeExpEpoch and objectV2.SysAttributeExpEpochNeoFS. Maybe it has to be an error too?

Also it could happen, that user has defined both of attributes: `objectV2.SysAttributeExpEpoch` and `objectV2.SysAttributeExpEpochNeoFS`. Maybe it has to be an error too?

fyrchik commented

2024-06-18 12:48:31 +00:00

It definitely should be, but we will eventually remove NeoFS attribute, and currently we have deterministic behaviour, so I would leave it as is.

dstepanov-yadro marked this conversation as resolved

acid-ant force-pushed bugfix/1109-validate-exp-epoch from e0546ff767 to 45f8630deb

2024-06-18 14:35:11 +00:00

Compare

fyrchik approved these changes 2024-06-19 11:43:18 +00:00

dstepanov-yadro approved these changes 2024-06-19 12:45:37 +00:00

fyrchik merged commit 40b68bcb6c into master

2024-06-19 17:19:32 +00:00

Sign in to join this conversation.