-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cloud Run No Build Initial MVP Implementation v2 #9707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
292c48b
57855da
b4f9633
f24830e
6670e13
dc1978e
487ad04
15a7b26
23c6d27
366fe34
e66d9de
1975dec
6b53bf9
f385cc3
e98b149
55ee19f
92124c2
2e3ceec
8e1e111
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,7 @@ | |
| ["iam.serviceAccounts.actAs"], | ||
| ); | ||
| passed = iamResult.passed; | ||
| } catch (err: any) { | ||
| logger.debug("[functions] service account IAM check errored, deploy may fail:", err); | ||
| // we want to fail this check open and not rethrow since it's informational only | ||
| return; | ||
|
|
@@ -61,7 +61,6 @@ | |
| /** | ||
| * Checks a functions deployment for HTTP function creation, and tests IAM | ||
| * permissions accordingly. | ||
| * | ||
| * @param context The deploy context. | ||
| * @param options The command-wide options object. | ||
| * @param payload The deploy payload. | ||
|
|
@@ -74,11 +73,13 @@ | |
| if (!payload.functions) { | ||
| return; | ||
| } | ||
| const filters = context.filters || getEndpointFilters(options, context.config!); | ||
| const wantBackends = Object.values(payload.functions).map(({ wantBackend }) => wantBackend); | ||
| const httpEndpoints = [...flattenArray(wantBackends.map((b) => backend.allEndpoints(b)))] | ||
| .filter(backend.isHttpsTriggered || backend.isDataConnectGraphqlTriggered) | ||
| .filter((f) => endpointMatchesAnyFilter(f, filters)); | ||
| .filter((f) => backend.isHttpsTriggered(f) || backend.isDataConnectGraphqlTriggered(f)) | ||
| .filter((f) => endpointMatchesAnyFilter(f, filters)) | ||
| // Services with platform: "run" are not GCFv1 or GCFv2 functions and are handled separately. | ||
| .filter((f) => f.platform !== "run"); | ||
|
|
||
| const existing = await backend.existingBackend(context); | ||
| const newHttpsEndpoints = httpEndpoints.filter(backend.missingEndpoint(existing)); | ||
|
|
@@ -97,7 +98,7 @@ | |
| try { | ||
| const iamResult = await iam.testIamPermissions(context.projectId, [PERMISSION]); | ||
| passed = iamResult.passed; | ||
| } catch (e: any) { | ||
| logger.debug( | ||
| "[functions] failed http create setIamPolicy permission check. deploy may fail:", | ||
| e, | ||
|
|
@@ -130,7 +131,7 @@ | |
| } | ||
|
|
||
| /** Callback reducer function */ | ||
| function reduceEventsToServices(services: Array<Service>, endpoint: backend.Endpoint) { | ||
| const service = serviceForEndpoint(endpoint); | ||
| if (service.requiredProjectBindings && !services.find((s) => s.name === service.name)) { | ||
| services.push(service); | ||
|
|
@@ -149,7 +150,7 @@ | |
| * Finds the required project level IAM bindings for the Pub/Sub service agent. | ||
| * If the user enabled Pub/Sub on or before April 8, 2021, then we must enable the token creator role. | ||
| * @param projectNumber project number | ||
| * @param existingPolicy the project level IAM policy | ||
|
Check warning on line 153 in src/deploy/functions/checkIam.ts
|
||
| */ | ||
| export function obtainPubSubServiceAgentBindings(projectNumber: string): iam.Binding[] { | ||
| const serviceAccountTokenCreatorBinding: iam.Binding = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -223,7 +223,7 @@ export async function prepare( | |
| ); | ||
| } | ||
|
|
||
| if (backend.someEndpoint(wantBackend, (e) => e.platform === "gcfv2")) { | ||
| if (backend.someEndpoint(wantBackend, (e) => e.platform === "gcfv2" || e.platform === "run")) { | ||
| const schPathSet = new Set<string>(); | ||
| for (const e of backend.allEndpoints(wantBackend)) { | ||
| if ( | ||
|
|
@@ -233,11 +233,16 @@ export async function prepare( | |
| schPathSet.add(e.dataConnectGraphqlTrigger.schemaFilePath); | ||
| } | ||
| } | ||
| const exportType = backend.someEndpoint(wantBackend, (e) => e.platform === "run") | ||
| ? "tar.gz" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit* I think zip deploy may add support for .zip soonish?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, do you think above would be best to add as a todo once they add the support or preemptively add it now? |
||
| : "zip"; | ||
| const packagedSource = await prepareFunctionsUpload( | ||
| options.config.projectDir, | ||
| sourceDir, | ||
| localCfg, | ||
| [...schPathSet], | ||
| undefined, | ||
| { exportType }, | ||
| ); | ||
| source.functionsSourceV2 = packagedSource?.pathToSource; | ||
| source.functionsSourceV2Hash = packagedSource?.hash; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: why exclude run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iiuc GCF and Run have different permissions (cloudfunctions.functions.setIamPolicy vs. run.services.setIamPolicy) and the specific check where it is excluded here is for GCF permission
GCF: https://docs.cloud.google.com/functions/docs/reference/iam/permissions#functions
Run: https://docs.cloud.google.com/run/docs/reference/iam/permissions#services
For current no build mvp implementation, I think the current approach is implemented so that we try to set the policy in setInvoker (within fabricator.ts) and if it fails iiuc there should be an IAM error from Cloud Run