Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/deploy/functions/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
schedule?: string;
timeZone?: string | null;
retryConfig?: ScheduleRetryConfig | null;
attemptDeadlineSeconds?: number | null;
}

/** Something that has a ScheduleTrigger */
Expand Down Expand Up @@ -179,13 +180,20 @@
return allMemoryOptions.includes(mem as MemoryOptions);
}

/**
* Is a given string a valid VpcEgressSettings?
*/
export function isValidEgressSetting(egress: unknown): egress is VpcEgressSettings {

Check warning on line 183 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
return egress === "PRIVATE_RANGES_ONLY" || egress === "ALL_TRAFFIC";
}

export const MIN_ATTEMPT_DEADLINE_SECONDS = 15;
export const MAX_ATTEMPT_DEADLINE_SECONDS = 1800; // 30 mins

/**
* Is a given number a valid attempt deadline?
*/
export function isValidAttemptDeadline(seconds: number): boolean {
return seconds >= MIN_ATTEMPT_DEADLINE_SECONDS && seconds <= MAX_ATTEMPT_DEADLINE_SECONDS;
}

/** Returns a human-readable name with MB or GB suffix for a MemoryOption (MB). */
export function memoryOptionDisplayName(option: MemoryOptions): string {
return {
Expand Down Expand Up @@ -551,8 +559,8 @@
existingBackend.endpoints[endpoint.region][endpoint.id] = endpoint;
}
unreachableRegions.gcfV2 = gcfV2Results.unreachable;
} catch (err: any) {

Check warning on line 562 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err.status === 404 && err.message?.toLowerCase().includes("method not found")) {

Check warning on line 563 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe call of an `any` typed value

Check warning on line 563 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .message on an `any` value

Check warning on line 563 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe call of an `any` typed value

Check warning on line 563 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .includes on an `any` value

Check warning on line 563 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
// customer has preview enabled without allowlist set
} else {
throw err;
Expand Down
38 changes: 38 additions & 0 deletions src/deploy/functions/build.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,44 @@ describe("toBackend", () => {
expect(endpointDef.func.serviceAccount).to.equal("service-account-1@");
}
});

it("throws if attemptDeadlineSeconds is out of range", () => {
const desiredBuild: build.Build = build.of({
func: {
platform: "gcfv2",
region: ["us-central1"],
project: "project",
runtime: "nodejs16",
entryPoint: "func",
scheduleTrigger: {
schedule: "every 1 minutes",
attemptDeadlineSeconds: 10, // Invalid: < 15
},
},
});
expect(() => build.toBackend(desiredBuild, {})).to.throw(
FirebaseError,
/attemptDeadlineSeconds must be between 15 and 1800 seconds/,
);

const desiredBuild2: build.Build = build.of({
func: {
platform: "gcfv2",
region: ["us-central1"],
project: "project",
runtime: "nodejs16",
entryPoint: "func",
scheduleTrigger: {
schedule: "every 1 minutes",
attemptDeadlineSeconds: 1801, // Invalid: > 1800
},
},
});
expect(() => build.toBackend(desiredBuild2, {})).to.throw(
FirebaseError,
/attemptDeadlineSeconds must be between 15 and 1800 seconds/,
);
});
});

describe("envWithType", () => {
Expand Down
13 changes: 13 additions & 0 deletions src/deploy/functions/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
schedule: string | Expression<string>;
timeZone?: Field<string>;
retryConfig?: ScheduleRetryConfig | null;
attemptDeadlineSeconds?: Field<number>;
}

export type HttpsTriggered = { httpsTrigger: HttpsTrigger };
Expand Down Expand Up @@ -456,7 +457,7 @@
// List param, we try resolving a String param instead.
try {
regions = params.resolveList(bdEndpoint.region, paramValues);
} catch (err: any) {

Check warning on line 460 in src/deploy/functions/build.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err instanceof ExprParseError) {
regions = [params.resolveString(bdEndpoint.region, paramValues)];
} else {
Expand Down Expand Up @@ -603,6 +604,18 @@
} else if (endpoint.scheduleTrigger.retryConfig === null) {
bkSchedule.retryConfig = null;
}
if (typeof endpoint.scheduleTrigger.attemptDeadlineSeconds !== "undefined") {
const attemptDeadlineSeconds = r.resolveInt(endpoint.scheduleTrigger.attemptDeadlineSeconds);
if (
attemptDeadlineSeconds !== null &&
!backend.isValidAttemptDeadline(attemptDeadlineSeconds)
) {
throw new FirebaseError(
`attemptDeadlineSeconds must be between ${backend.MIN_ATTEMPT_DEADLINE_SECONDS} and ${backend.MAX_ATTEMPT_DEADLINE_SECONDS} seconds (inclusive).`,
);
}
bkSchedule.attemptDeadlineSeconds = attemptDeadlineSeconds;
}
return { scheduleTrigger: bkSchedule };
} else if ("taskQueueTrigger" in endpoint) {
const taskQueueTrigger: backend.TaskQueueTrigger = {};
Expand Down
2 changes: 2 additions & 0 deletions src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ describe("buildFromV1Alpha", () => {
maxRetrySeconds: 120,
maxDoublings: 10,
},
attemptDeadlineSeconds: 300,
};

const yaml: v1alpha1.WireManifest = {
Expand Down Expand Up @@ -744,6 +745,7 @@ describe("buildFromV1Alpha", () => {
maxRetrySeconds: "{{ params.RETRY_DURATION }}",
maxDoublings: "{{ params.DOUBLINGS }}",
},
attemptDeadlineSeconds: "{{ params.ATTEMPT_DEADLINE }}",
};

const yaml: v1alpha1.WireManifest = {
Expand Down
2 changes: 2 additions & 0 deletions src/deploy/functions/runtimes/discovery/v1alpha1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@
schedule: "Field<string>",
timeZone: "Field<string>?",
retryConfig: "object?",
attemptDeadlineSeconds: "Field<number>?",
});
if (ep.scheduleTrigger.retryConfig) {
assertKeyTypes(prefix + ".scheduleTrigger.retryConfig", ep.scheduleTrigger.retryConfig, {
Expand Down Expand Up @@ -287,8 +288,8 @@
retry: ep.eventTrigger.retry,
};
// Allow serviceAccountEmail but prefer serviceAccount
if ("serviceAccountEmail" in (ep.eventTrigger as any)) {

Check warning on line 291 in src/deploy/functions/runtimes/discovery/v1alpha1.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
eventTrigger.serviceAccount = (ep.eventTrigger as any).serviceAccountEmail;

Check warning on line 292 in src/deploy/functions/runtimes/discovery/v1alpha1.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
}
copyIfPresent(
eventTrigger,
Expand Down Expand Up @@ -377,6 +378,7 @@
} else if (ep.scheduleTrigger.retryConfig === null) {
st.retryConfig = null;
}
copyIfPresent(st, ep.scheduleTrigger, "attemptDeadlineSeconds");
triggered = { scheduleTrigger: st };
} else if (build.isTaskQueueTriggered(ep)) {
const tq: build.TaskQueueTrigger = {};
Expand Down
54 changes: 54 additions & 0 deletions src/gcp/cloudscheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,5 +283,59 @@ describe("cloudscheduler", () => {
},
});
});

it("should not copy attemptDeadlineSeconds for v1 endpoints", async () => {
expect(
await cloudscheduler.jobFromEndpoint(
{
...V1_ENDPOINT,
scheduleTrigger: {
schedule: "every 1 minutes",
attemptDeadlineSeconds: 300,
},
},
"appEngineLocation",
"1234567",
),
).to.deep.equal({
name: "projects/project/locations/appEngineLocation/jobs/firebase-schedule-id-region",
schedule: "every 1 minutes",
timeZone: "America/Los_Angeles",
pubsubTarget: {
topicName: "projects/project/topics/firebase-schedule-id-region",
attributes: {
scheduled: "true",
},
},
});
});

it("should copy attemptDeadlineSeconds for v2 endpoints", async () => {
expect(
await cloudscheduler.jobFromEndpoint(
{
...V2_ENDPOINT,
scheduleTrigger: {
schedule: "every 1 minutes",
attemptDeadlineSeconds: 300,
},
},
V2_ENDPOINT.region,
"1234567",
),
).to.deep.equal({
name: "projects/project/locations/region/jobs/firebase-schedule-id-region",
schedule: "every 1 minutes",
timeZone: "UTC",
attemptDeadline: "300s",
httpTarget: {
uri: "https://my-uri.com",
httpMethod: "POST",
oidcToken: {
serviceAccountEmail: "1234567-compute@developer.gserviceaccount.com",
},
},
});
});
});
});
13 changes: 13 additions & 0 deletions src/gcp/cloudscheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export interface Job {
schedule: string;
description?: string;
timeZone?: string | null;
attemptDeadline?: string | null;

// oneof target
httpTarget?: HttpTarget;
Expand Down Expand Up @@ -195,6 +196,9 @@ function needUpdate(existingJob: Job, newJob: Job): boolean {
if (existingJob.timeZone !== newJob.timeZone) {
return true;
}
if (existingJob.attemptDeadline !== newJob.attemptDeadline) {
return true;
}
if (newJob.retryConfig) {
if (!existingJob.retryConfig) {
return true;
Expand Down Expand Up @@ -258,6 +262,15 @@ export async function jobFromEndpoint(
);
}
job.schedule = endpoint.scheduleTrigger.schedule;
if (endpoint.platform === "gcfv2" || endpoint.platform === "run") {
proto.convertIfPresent(
job,
endpoint.scheduleTrigger,
"attemptDeadline",
"attemptDeadlineSeconds",
nullsafeVisitor(proto.durationFromSeconds),
);
}
if (endpoint.scheduleTrigger.retryConfig) {
job.retryConfig = {};
proto.copyIfPresent(
Expand Down
Loading