Skip to content

Commit

Permalink
Fix service account option to be a param (#6389)
Browse files Browse the repository at this point in the history
* treat the service account option as a param

* add changelog entry

---------

Co-authored-by: joehan <joehanley@google.com>
  • Loading branch information
colerogers and joehan authored Sep 27, 2023
1 parent f5aa458 commit 2657297
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
- Fixed an issue where the functions service account option was not treated as a param (#6389).
- Fixed an issue with deploying function groups containing v2 functions. (#6408)
- Use GetDefaultBucket endpoint to fetch Storage Default Bucket.
6 changes: 3 additions & 3 deletions src/deploy/functions/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export type Endpoint = Triggered & {
// The services account that this function should run as.
// defaults to the GAE service account when a function is first created as a GCF gen 1 function.
// Defaults to the compute service account when a function is first created as a GCF gen 2 function.
serviceAccount?: ServiceAccount | null;
serviceAccount?: Field<string> | ServiceAccount | null;

// defaults to ["us-central1"], overridable in firebase-tools with
// process.env.FIREBASE_FUNCTIONS_DEFAULT_REGION
Expand Down Expand Up @@ -457,8 +457,7 @@ export function toBackend(
bdEndpoint,
"environmentVariables",
"labels",
"secretEnvironmentVariables",
"serviceAccount"
"secretEnvironmentVariables"
);

proto.convertIfPresent(bkEndpoint, bdEndpoint, "ingressSettings", (from) => {
Expand All @@ -479,6 +478,7 @@ export function toBackend(
return (mem as backend.MemoryOptions) || null;
});

r.resolveStrings(bkEndpoint, bdEndpoint, "serviceAccount");
r.resolveInts(
bkEndpoint,
bdEndpoint,
Expand Down
46 changes: 46 additions & 0 deletions src/test/deploy/functions/build.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from "chai";
import * as build from "../../../deploy/functions/build";
import { ParamValue } from "../../../deploy/functions/params";

describe("toBackend", () => {
it("populates backend info from Build", () => {
Expand Down Expand Up @@ -110,4 +111,49 @@ describe("toBackend", () => {
).to.have.members(["service-account-1@", "service-account-2@"]);
}
});

it("populates multiple param values", () => {
const desiredBuild: build.Build = build.of({
func: {
platform: "gcfv2",
region: ["us-central1"],
project: "project",
runtime: "nodejs16",
entryPoint: "func",
maxInstances: "{{ params.maxinstances }}",
minInstances: "{{ params.mininstances }}",
serviceAccount: "{{ params.serviceaccount }}",
vpc: {
connector: "projects/project/locations/region/connectors/connector",
egressSettings: "PRIVATE_RANGES_ONLY",
},
ingressSettings: "ALLOW_ALL",
labels: {
test: "testing",
},
httpsTrigger: {
invoker: ["service-account-2@", "service-account-3@"],
},
},
});
const backend = build.toBackend(desiredBuild, {
maxinstances: new ParamValue("42", false, { number: true }),
mininstances: new ParamValue("1", false, { number: true }),
serviceaccount: new ParamValue("service-account-1@", false, { string: true }),
});
expect(Object.keys(backend.endpoints).length).to.equal(1);
const endpointDef = Object.values(backend.endpoints)[0];
expect(endpointDef).to.not.equal(undefined);
if (endpointDef) {
expect(endpointDef.func.id).to.equal("func");
expect(endpointDef.func.project).to.equal("project");
expect(endpointDef.func.region).to.equal("us-central1");
expect(endpointDef.func.maxInstances).to.equal(42);
expect(endpointDef.func.minInstances).to.equal(1);
expect(endpointDef.func.serviceAccount).to.equal("service-account-1@");
expect(
"httpsTrigger" in endpointDef.func ? endpointDef.func.httpsTrigger.invoker : []
).to.have.members(["service-account-2@", "service-account-3@"]);
}
});
});

0 comments on commit 2657297

Please sign in to comment.