Unable to deploy with VPC from external shared project

[steveoh]

[REQUIRED] Environment info

firebase-tools: 12.5.2

Platform: macos

[REQUIRED] Test case

Trying to use a shared vpc from a different project with .env and

FUNCTION_VPC="projects/shared/locations/us-central1/connectors/shared"

const vpc = params.defineString('FUNCTION_VPC');
setGlobalOptions({
  vpcConnector: vpc
});

[REQUIRED] Steps to reproduce

Try to deploy with these values and firebase tools mutates the vpcConnector string to an invalid value.

functions: HTTP Error: 400, Could not update Cloud Run service projects/my-project/locations/us-central1/services/function. spec.template.metadata.annotations: Expected a VPC access connector name like projects/*/locations/*/connectors/*, but obtained projects/not-shared-project/locations/us-central1/connectors/{{the full value of the FUNCTION_VPC}}

I assume the issue is around

firebase-tools/src/deploy/functions/runtimes/node/parseTriggers.ts

Lines 502 to 514 in 09c8d88

	if (annotation.vpcConnector != null) {
	let maybeId = annotation.vpcConnector;
	if (maybeId && !maybeId.includes("/")) {
	maybeId = `projects/${projectId}/locations/${region}/connectors/${maybeId}`;
	}
	endpoint.vpc = { connector: maybeId };
	proto.renameIfPresent(
	endpoint.vpc,
	annotation,
	"egressSettings",
	"vpcConnectorEgressSettings"
	);
	}

But my connector has a / in it so it shouldn't get modified. I wonder if it has to do with the Expression<string> type that has a .value() not being invoked or something until later? Or maybe the / is encoded when read from the environment file?

[REQUIRED] Expected behavior

The VPC value I specify is used

[REQUIRED] Actual behavior

The string is mutated and the deployment fails.

[thomasmburke]

Another reference to the connector in the source:

firebase-tools/src/deploy/functions/build.ts

Lines 496 to 500 in 09c8d88

	if (bdEndpoint.vpc) {
	if (bdEndpoint.vpc.connector && !bdEndpoint.vpc.connector.includes("/")) {
	bdEndpoint.vpc.connector = `projects/${bdEndpoint.project}/locations/${region}/connectors/${bdEndpoint.vpc.connector}`;
	}
	bkEndpoint.vpc = { connector: params.resolveString(bdEndpoint.vpc.connector, paramValues) };

[steveoh]

all of the connector building strings are protected by the / include but somehow I am getting through and it's getting appended. How is that possible?

Are there instructions on how to step through the code with like a javascript debug terminal in vs code if i knew where in the npm package to set a breakpoint?

[thomasmburke]

does the --debug potentially log the values of .env ?

[steveoh]

i'm stepping through my node_modules and found this which implies it doesn't know the value yet and hasn't expanded the value so it doesn't have the / and why it gets appended incorrectly. It's resolving too late.

That gets expanded to

bkEndpoint.vpc.connector = "projects/wrong-project/locations/us-central1/connectors/{{ params.FUNCTION_VPC }}"

moving the expansion above the check for the / fixes my issue. Unsure if that is viable for other intended purposes.

if (bdEndpoint.vpc) {
  bdEndpoint.vpc = { connector: params.resolveString(bdEndpoint.vpc.connector, paramValues) };

  if (bdEndpoint.vpc.connector && !bdEndpoint.vpc.connector.includes("/")) {
    bdEndpoint.vpc.connector = `projects/${bdEndpoint.project}/locations/${region}/connectors/${bdEndpoint.vpc.connector}`;
  }
  proto.copyIfPresent(bkEndpoint.vpc, bdEndpoint.vpc, "egressSettings");
}

[steveoh]

I'm happy to contribute that as a PR if you think it is a good solution?

[thomasmburke]

Not a maintainer of the repo - so don't have an in depth understanding of how param changes could impact other situations. You'd definitely get feedback you are looking for on a PR, just follow the contributing guide

Also - great find!

[steveoh]

Ok opened a PR to see the testing results as I don't have this repo checked out and cloned. Curious to see if this causes any failures.