Skip to content

Commit 99db3e4

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 0c68157 commit 99db3e4

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
@@ -2500,7 +2500,7 @@ describe("r2", () => {
25002500
result: true,
25012501
});
25022502
await runWrangler(
2503-
`r2 bucket lock add ${bucketName} --id "rule-no-prefix" --lock-days 1`
2503+
`r2 bucket lock add ${bucketName} --id "rule-no-prefix" --retention-days 1`
25042504
);
25052505
expect(std.out).toMatchInlineSnapshot(`
25062506
"Adding lock rule 'rule-no-prefix' to bucket 'my-bucket'...
@@ -2515,7 +2515,7 @@ describe("r2", () => {
25152515

25162516
mockConfirm({
25172517
text:
2518-
`Are you sure you want to add lock rule 'rule-not-indefinite' to bucket '${bucketName}' without an expiration? ` +
2518+
`Are you sure you want to add lock rule 'rule-not-indefinite' to bucket '${bucketName}' without retention? ` +
25192519
`The lock rule will apply to all matching objects indefinitely.`,
25202520
result: false,
25212521
});
@@ -2544,7 +2544,7 @@ describe("r2", () => {
25442544
]);
25452545
// age
25462546
await runWrangler(
2547-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --lock-days 1`
2547+
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days 1`
25482548
);
25492549
expect(std.out).toMatchInlineSnapshot(`
25502550
"Adding lock rule 'rule-age' to bucket 'my-bucket'...
@@ -2559,7 +2559,7 @@ describe("r2", () => {
25592559
// age
25602560
await expect(() =>
25612561
runWrangler(
2562-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --lock-days one`
2562+
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days one`
25632563
)
25642564
).rejects.toThrowErrorMatchingInlineSnapshot(
25652565
`[Error: Days must be a number.]`
@@ -2583,7 +2583,7 @@ describe("r2", () => {
25832583
// age
25842584
await expect(() =>
25852585
runWrangler(
2586-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --lock-days -10`
2586+
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days -10`
25872587
)
25882588
).rejects.toThrowErrorMatchingInlineSnapshot(
25892589
`[Error: Days must be a positive number: -10]`
@@ -2606,22 +2606,22 @@ describe("r2", () => {
26062606
]);
26072607
// date
26082608
await runWrangler(
2609-
`r2 bucket lock add ${bucketName} --id rule-date --prefix prefix-date --lock-date 2025-01-30`
2609+
`r2 bucket lock add ${bucketName} --id rule-date --prefix prefix-date --retention-date 2025-01-30`
26102610
);
26112611
expect(std.out).toMatchInlineSnapshot(`
26122612
"Adding lock rule 'rule-date' to bucket 'my-bucket'...
26132613
✨ Added lock rule 'rule-date' to bucket 'my-bucket'."
26142614
`);
26152615
});
2616-
it("it should fail to add an invalid date lock rule using command-line arguments", async () => {
2616+
it("it should fail to add an invalid date lock rule using command-line arguments if retention is not", async () => {
26172617
setIsTTY(true);
26182618
const bucketName = "my-bucket";
26192619

26202620
mockBucketLockGetExistingRules(bucketName, []);
26212621
// date
26222622
await expect(() =>
26232623
runWrangler(
2624-
`r2 bucket lock add ${bucketName} --id "rule-date" --prefix "prefix-date" --lock-date "January 30, 2025"`
2624+
`r2 bucket lock add ${bucketName} --id "rule-date" --prefix "prefix-date" --retention-date "January 30, 2025"`
26252625
)
26262626
).rejects.toThrowErrorMatchingInlineSnapshot(
26272627
`[Error: Date must be a valid date in the YYYY-MM-DD format: January 30, 2025]`
@@ -2642,9 +2642,32 @@ describe("r2", () => {
26422642
},
26432643
]);
26442644

2645+
await runWrangler(
2646+
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite --retention-indefinite`
2647+
);
2648+
expect(std.out).toMatchInlineSnapshot(`
2649+
"Adding lock rule 'rule-indefinite' to bucket 'my-bucket'...
2650+
✨ Added lock rule 'rule-indefinite' to bucket 'my-bucket'."
2651+
`);
2652+
});
2653+
it("it should add an indefinite lock rule using command-line arguments and prompt if not initially specified", async () => {
2654+
setIsTTY(false);
2655+
const bucketName = "my-bucket";
2656+
2657+
mockBucketLockPutNew(bucketName, [
2658+
{
2659+
id: "rule-indefinite",
2660+
enabled: true,
2661+
prefix: "prefix-indefinite",
2662+
condition: {
2663+
type: "Indefinite",
2664+
},
2665+
},
2666+
]);
2667+
26452668
mockConfirm({
26462669
text:
2647-
`Are you sure you want to add lock rule 'rule-indefinite' to bucket '${bucketName}' without an expiration? ` +
2670+
`Are you sure you want to add lock rule 'rule-indefinite' to bucket '${bucketName}' without retention? ` +
26482671
`The lock rule will apply to all matching objects indefinitely.`,
26492672
result: true,
26502673
});
@@ -2653,12 +2676,35 @@ describe("r2", () => {
26532676
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite`
26542677
);
26552678
expect(std.out).toMatchInlineSnapshot(`
2656-
"? 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.
2679+
"? 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.
26572680
🤖 Using fallback value in non-interactive context: yes
26582681
Adding lock rule 'rule-indefinite' to bucket 'my-bucket'...
26592682
✨ Added lock rule 'rule-indefinite' to bucket 'my-bucket'."
26602683
`);
26612684
});
2685+
it("it should fail to add a lock rule if retenion is indefinite but false", async () => {
2686+
setIsTTY(true);
2687+
const bucketName = "my-bucket";
2688+
2689+
mockBucketLockPutNew(bucketName, [
2690+
{
2691+
id: "rule-indefinite",
2692+
enabled: true,
2693+
prefix: "prefix-indefinite",
2694+
condition: {
2695+
type: "Indefinite",
2696+
},
2697+
},
2698+
]);
2699+
2700+
await expect(() =>
2701+
runWrangler(
2702+
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite --retention-indefinite false`
2703+
)
2704+
).rejects.toThrowErrorMatchingInlineSnapshot(
2705+
`[Error: Retention must be specified.]`
2706+
);
2707+
});
26622708
it("it should fail a lock rule without any command-line arguments", async () => {
26632709
setIsTTY(false);
26642710
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)