storj / edge

Storj edge services (including multi-tenant, S3-compatible server to interact with the Storj network)
GNU Affero General Public License v3.0
53 stars 18 forks source link

Add Test Case for Object Lock Configuration API: Year and Day Duration Calculation #512

Open NiaStorj opened 1 week ago

NiaStorj commented 1 week ago

Description:

We need to add a test case for the PutObjectLockConfiguration API to verify the calculation of the duration specified in years and days. The API requires users to provide the number of years and days, and this duration is used to lock object versions. Given potential variations such as leap years, leap seconds, and time zone changes, it's critical to ensure that the duration calculations align with S3's standards.

Acceptance Criteria:

NiaStorj commented 1 week ago

cc @amwolff

pwilloughby commented 1 week ago

it's critical to ensure that the duration calculations align with S3's standards.

Do you think that's really true? S3's documentation is vague and the retention periods are coarse. I think that is intentional to prevent having to deal will all the complexities mentioned in this issue. If it's roughly the same as s3 I suspect most customers will be happy with that.

halkyon commented 1 week ago

minio just does whatever Go's t.AddDate does: https://github.com/minio/minio/blob/master/internal/bucket/object/lock/lock.go#L282, maybe that's enough: https://pkg.go.dev/time#Time.AddDate

AddDate returns the time corresponding to adding the given number of years, months, and days to t. For example, AddDate(-1, 2, 3) applied to January 1, 2011 returns March 4, 2010.

Note that dates are fundamentally coupled to timezones, and calendrical periods like days don't have fixed durations. AddDate uses the Location of the Time value to determine these durations. That means that the same AddDate arguments can produce a different shift in absolute time depending on the base Time value and its Location. For example, AddDate(0, 0, 1) applied to 12:00 on March 27 always returns 12:00 on March 28. At some locations and in some years this is a 24 hour shift. In others it's a 23 hour shift due to daylight savings time transitions.

AddDate normalizes its result in the same way that Date does, so, for example, adding one month to October 31 yields December 1, the normalized form for November 31.

amwolff commented 1 week ago

it's critical to ensure that the duration calculations align with S3's standards.

Do you think that's really true? S3's documentation is vague and the retention periods are coarse. I think that is intentional to prevent having to deal will all the complexities mentioned in this issue. If it's roughly the same as s3 I suspect most customers will be happy with that.

I feel like one extra day (the lack of it) might be a big deal in some cases. I don't know. But as Sean mentioned, if we use standard library's time handling, we'll be safe.

jewharton commented 6 days ago

Based on my testing, S3 does not define a year as strictly 365 days. For leap years, it becomes 366 days.

On 2024-10-01, I set a default retention configuration with a duration of 4 years, encompassing the next leap day on 2028-02-29. This gave uploaded objects a retention period expiration date of 2028-10-01. I then changed the default retention duration to 1460 days (365 * 4), and the retention period expiration date of objects uploaded thereafter was 2028-09-30. These dates would be the same if a year and 365 days were considered equivalent quantities by S3.