Skip to content

Commit f53b59d

Browse files
jkoe-cfjkoe-cf
committed
R2-2612: updating lock date/days to retention date/days and explicitly making retention required instead of indefinite being implied
1 parent 0e37ea9 commit f53b59d

File tree

2 files changed

+113
-37
lines changed

2 files changed

+113
-37
lines changed

‎packages/wrangler/src/__tests__/r2.test.ts‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2517,7 +2517,7 @@ describe("r2", () => {
25172517
result: true,
25182518
});
25192519
await runWrangler(
2520-
`r2 bucket lock add ${bucketName} --id "rule-no-prefix" --lock-days 1`
2520+
`r2 bucket lock add ${bucketName} --id "rule-no-prefix" --retention-days 1`
25212521
);
25222522
expect(std.out).toMatchInlineSnapshot(`
25232523
"Adding lock rule 'rule-no-prefix' to bucket 'my-bucket'...
@@ -2532,7 +2532,7 @@ describe("r2", () => {
25322532

25332533
mockConfirm({
25342534
text:
2535-
`Are you sure you want to add lock rule 'rule-not-indefinite' to bucket '${bucketName}' without an expiration? ` +
2535+
`Are you sure you want to add lock rule 'rule-not-indefinite' to bucket '${bucketName}' without retention? ` +
25362536
`The lock rule will apply to all matching objects indefinitely.`,
25372537
result: false,
25382538
});
@@ -2561,7 +2561,7 @@ describe("r2", () => {
25612561
]);
25622562
// age
25632563
await runWrangler(
2564-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --lock-days 1`
2564+
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days 1`
25652565
);
25662566
expect(std.out).toMatchInlineSnapshot(`
25672567
"Adding lock rule 'rule-age' to bucket 'my-bucket'...
@@ -2576,7 +2576,7 @@ describe("r2", () => {
25762576
// age
25772577
await expect(() =>
25782578
runWrangler(
2579-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --lock-days one`
2579+
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days one`
25802580
)
25812581
).rejects.toThrowErrorMatchingInlineSnapshot(
25822582
`[Error: Days must be a number.]`
@@ -2600,7 +2600,7 @@ describe("r2", () => {
26002600
// age
26012601
await expect(() =>
26022602
runWrangler(
2603-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --lock-days -10`
2603+
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days -10`
26042604
)
26052605
).rejects.toThrowErrorMatchingInlineSnapshot(
26062606
`[Error: Days must be a positive number: -10]`
@@ -2623,22 +2623,22 @@ describe("r2", () => {
26232623
]);
26242624
// date
26252625
await runWrangler(
2626-
`r2 bucket lock add ${bucketName} --id rule-date --prefix prefix-date --lock-date 2025-01-30`
2626+
`r2 bucket lock add ${bucketName} --id rule-date --prefix prefix-date --retention-date 2025-01-30`
26272627
);
26282628
expect(std.out).toMatchInlineSnapshot(`
26292629
"Adding lock rule 'rule-date' to bucket 'my-bucket'...
26302630
✨ Added lock rule 'rule-date' to bucket 'my-bucket'."
26312631
`);
26322632
});
2633-
it("it should fail to add an invalid date lock rule using command-line arguments", async () => {
2633+
it("it should fail to add an invalid date lock rule using command-line arguments if retention is not", async () => {
26342634
setIsTTY(true);
26352635
const bucketName = "my-bucket";
26362636

26372637
mockBucketLockGetExistingRules(bucketName, []);
26382638
// date
26392639
await expect(() =>
26402640
runWrangler(
2641-
`r2 bucket lock add ${bucketName} --id "rule-date" --prefix "prefix-date" --lock-date "January 30, 2025"`
2641+
`r2 bucket lock add ${bucketName} --id "rule-date" --prefix "prefix-date" --retention-date "January 30, 2025"`
26422642
)
26432643
).rejects.toThrowErrorMatchingInlineSnapshot(
26442644
`[Error: Date must be a valid date in the YYYY-MM-DD format: January 30, 2025]`
@@ -2659,9 +2659,32 @@ describe("r2", () => {
26592659
},
26602660
]);
26612661

2662+
await runWrangler(
2663+
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite --retention-indefinite`
2664+
);
2665+
expect(std.out).toMatchInlineSnapshot(`
2666+
"Adding lock rule 'rule-indefinite' to bucket 'my-bucket'...
2667+
✨ Added lock rule 'rule-indefinite' to bucket 'my-bucket'."
2668+
`);
2669+
});
2670+
it("it should add an indefinite lock rule using command-line arguments and prompt if not initially specified", async () => {
2671+
setIsTTY(false);
2672+
const bucketName = "my-bucket";
2673+
2674+
mockBucketLockPutNew(bucketName, [
2675+
{
2676+
id: "rule-indefinite",
2677+
enabled: true,
2678+
prefix: "prefix-indefinite",
2679+
condition: {
2680+
type: "Indefinite",
2681+
},
2682+
},
2683+
]);
2684+
26622685
mockConfirm({
26632686
text:
2664-
`Are you sure you want to add lock rule 'rule-indefinite' to bucket '${bucketName}' without an expiration? ` +
2687+
`Are you sure you want to add lock rule 'rule-indefinite' to bucket '${bucketName}' without retention? ` +
26652688
`The lock rule will apply to all matching objects indefinitely.`,
26662689
result: true,
26672690
});
@@ -2670,12 +2693,35 @@ describe("r2", () => {
26702693
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite`
26712694
);
26722695
expect(std.out).toMatchInlineSnapshot(`
2673-
"? Are you sure you want to add lock rule 'rule-indefinite' to bucket 'my-bucket' without an expiration? The lock rule will apply to all matching objects indefinitely.
2696+
"? Are you sure you want to add lock rule 'rule-indefinite' to bucket 'my-bucket' without retention? The lock rule will apply to all matching objects indefinitely.
26742697
🤖 Using fallback value in non-interactive context: yes
26752698
Adding lock rule 'rule-indefinite' to bucket 'my-bucket'...
26762699
✨ Added lock rule 'rule-indefinite' to bucket 'my-bucket'."
26772700
`);
26782701
});
2702+
it("it should fail to add a lock rule if retenion is indefinite but false", async () => {
2703+
setIsTTY(true);
2704+
const bucketName = "my-bucket";
2705+
2706+
mockBucketLockPutNew(bucketName, [
2707+
{
2708+
id: "rule-indefinite",
2709+
enabled: true,
2710+
prefix: "prefix-indefinite",
2711+
condition: {
2712+
type: "Indefinite",
2713+
},
2714+
},
2715+
]);
2716+
2717+
await expect(() =>
2718+
runWrangler(
2719+
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite --retention-indefinite false`
2720+
)
2721+
).rejects.toThrowErrorMatchingInlineSnapshot(
2722+
`[Error: Retention must be specified.]`
2723+
);
2724+
});
26792725
it("it should fail a lock rule without any command-line arguments", async () => {
26802726
setIsTTY(false);
26812727
const bucketName = "my-bucket";

‎packages/wrangler/src/r2/lock.ts‎

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,20 @@ export const r2BucketLockAddCommand = createCommand({
8484
type: "string",
8585
requiresArg: true,
8686
},
87-
"lock-days": {
88-
describe: "Number of days after which objects expire",
87+
"retention-days": {
88+
describe: "Number of days which objects will be retained for",
8989
type: "number",
90-
conflicts: "lock-date",
90+
conflicts: ["retention-date", "retention-indefinite"],
9191
},
92-
"lock-date": {
93-
describe: "Date after which objects expire (YYYY-MM-DD)",
92+
"retention-date": {
93+
describe: "Date after which objects will be retained until (YYYY-MM-DD)",
9494
type: "string",
95-
conflicts: "lock-days",
95+
conflicts: ["retention-days", "retention-indefinite"],
96+
},
97+
"retention-indefinite": {
98+
describe: "Retain objects indefinitely",
99+
type: "boolean",
100+
conflicts: ["retention-date", "retention-days"],
96101
},
97102
jurisdiction: {
98103
describe: "The jurisdiction where the bucket exists",
@@ -108,7 +113,16 @@ export const r2BucketLockAddCommand = createCommand({
108113
},
109114
},
110115
async handler(
111-
{ bucket, lockDays, lockDate, jurisdiction, force, id, prefix },
116+
{
117+
bucket,
118+
retentionDays,
119+
retentionDate,
120+
retentionIndefinite,
121+
jurisdiction,
122+
force,
123+
id,
124+
prefix,
125+
},
112126
{ config }
113127
) {
114128
const accountId = await requireAuth(config);
@@ -138,7 +152,8 @@ export const r2BucketLockAddCommand = createCommand({
138152
if (prefix === "") {
139153
const confirmedAdd = await confirm(
140154
`Are you sure you want to add lock rule '${id}' to bucket '${bucket}' without a prefix? ` +
141-
`The lock rule will apply to all objects in your bucket.`
155+
`The lock rule will apply to all objects in your bucket.`,
156+
{ defaultValue: false }
142157
);
143158
if (!confirmedAdd) {
144159
logger.log("Add cancelled.");
@@ -151,57 +166,72 @@ export const r2BucketLockAddCommand = createCommand({
151166
newRule.prefix = prefix;
152167
}
153168

154-
if (lockDays === undefined && lockDate === undefined && !force) {
155-
const confirmIndefinite = await confirm(
156-
`Are you sure you want to add lock rule '${id}' to bucket '${bucket}' without an expiration? ` +
169+
if (
170+
retentionDays === undefined &&
171+
retentionDate === undefined &&
172+
retentionIndefinite === undefined &&
173+
!force
174+
) {
175+
retentionIndefinite = await confirm(
176+
`Are you sure you want to add lock rule '${id}' to bucket '${bucket}' without retention? ` +
157177
`The lock rule will apply to all matching objects indefinitely.`,
158-
{ defaultValue: true }
178+
{ defaultValue: false }
159179
);
160-
if (confirmIndefinite !== true) {
180+
if (retentionIndefinite !== true) {
161181
logger.log("Add cancelled.");
162182
return;
163183
}
164184
}
165185

166-
if (lockDays !== undefined) {
167-
if (!isNaN(lockDays)) {
168-
if (lockDays > 0) {
169-
const conditionDaysValue = Number(lockDays) * 86400; // Convert days to seconds
186+
if (retentionDays !== undefined) {
187+
if (!isNaN(retentionDays)) {
188+
if (retentionDays > 0) {
189+
const conditionDaysValue = Number(retentionDays) * 86400; // Convert days to seconds
170190
newRule.condition = {
171191
type: "Age",
172192
maxAgeSeconds: conditionDaysValue,
173193
};
174194
} else {
175-
throw new UserError(`Days must be a positive number: ${lockDays}`, {
176-
telemetryMessage: "Lock days not a positive number.",
177-
});
195+
throw new UserError(
196+
`Days must be a positive number: ${retentionDays}`,
197+
{
198+
telemetryMessage: "Retention days not a positive number.",
199+
}
200+
);
178201
}
179202
} else {
180203
throw new UserError(`Days must be a number.`, {
181-
telemetryMessage: "Lock days not a positive number.",
204+
telemetryMessage: "Retention days not a positive number.",
182205
});
183206
}
184-
} else if (lockDate !== undefined) {
185-
if (isValidDate(lockDate)) {
186-
const date = new Date(`${lockDate}T00:00:00.000Z`);
207+
} else if (retentionDate !== undefined) {
208+
if (isValidDate(retentionDate)) {
209+
const date = new Date(`${retentionDate}T00:00:00.000Z`);
187210
const conditionDateValue = date.toISOString();
188211
newRule.condition = {
189212
type: "Date",
190213
date: conditionDateValue,
191214
};
192215
} else {
193216
throw new UserError(
194-
`Date must be a valid date in the YYYY-MM-DD format: ${String(lockDate)}`,
217+
`Date must be a valid date in the YYYY-MM-DD format: ${String(retentionDate)}`,
195218
{
196219
telemetryMessage:
197-
"Lock date not a valid date in the YYYY-MM-DD format.",
220+
"Retention date not a valid date in the YYYY-MM-DD format.",
198221
}
199222
);
200223
}
201-
} else {
224+
} else if (
225+
retentionIndefinite !== undefined &&
226+
retentionIndefinite === true
227+
) {
202228
newRule.condition = {
203229
type: "Indefinite",
204230
};
231+
} else {
232+
throw new UserError(`Retention must be specified.`, {
233+
telemetryMessage: "Lock retention not specified.",
234+
});
205235
}
206236
rules.push(newRule);
207237
logger.log(`Adding lock rule '${id}' to bucket '${bucket}'...`);

0 commit comments

Comments
 (0)