Skip to content
Draft
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
2 changes: 1 addition & 1 deletion npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 12 additions & 3 deletions schema/firebase-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -966,13 +966,22 @@
}
]
},
"server": {
"additionalProperties": false,
"properties": {
"template": {
"type": "string"
}
},
"required": [
"template"
],
"type": "object"
},
"template": {
"type": "string"
}
},
"required": [
"template"
],
"type": "object"
},
"RemoteFunctionConfig": {
Expand Down
8 changes: 7 additions & 1 deletion src/commands/remoteconfig-get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,18 @@ export const command = new Command("remoteconfig:get")

let filename = undefined;
if (shouldUseDefaultFilename) {
filename = options.config.src.remoteconfig!.template;
filename = options.config.src.remoteconfig?.template;
} else {
utils.assertIsString(options.output);
filename = options.output;
}

if (!filename) {
throw new FirebaseError(
"No Remote Config template file specified in firebase.json or command line argument.",
);
}

const outTemplate = { ...template };
delete outTemplate.version;
fs.writeFileSync(filename, JSON.stringify(outTemplate, null, 2));
Expand Down
21 changes: 17 additions & 4 deletions src/deploy/remoteconfig/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,22 @@
* Gets Etag for Remote Config Project Template
* @param projectNumber Input is the Firebase Project's project number
* @param versionNumber Firebase Remote Config Template version number
* @param templateType Template type to get etag for (e.g. "firebase-server")
* @return {Promise<string>} Returns a Promise of the Remote Config Template Etag string
*/
export async function getEtag(projectNumber: string, versionNumber?: string): Promise<string> {
export async function getEtag(
projectNumber: string,
versionNumber?: string,
templateType?: string,
): Promise<string> {
const reqPath = `/projects/${projectNumber}/remoteConfig`;
const queryParams: { versionNumber?: string } = {};
const queryParams: { versionNumber?: string; templateType?: string } = {};
if (versionNumber) {
queryParams.versionNumber = versionNumber;
}
if (templateType) {
queryParams.templateType = templateType;
}
const response = await client.request<void, void>({
method: "GET",
path: reqPath,
Expand All @@ -37,17 +45,17 @@
export function validateInputRemoteConfigTemplate(
template: RemoteConfigTemplate,
): RemoteConfigTemplate {
const templateCopy = JSON.parse(JSON.stringify(template));

Check warning on line 48 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
if (!templateCopy || templateCopy === "null" || templateCopy === "undefined") {
throw new FirebaseError(`Invalid Remote Config template: ${JSON.stringify(templateCopy)}`);
}
if (typeof templateCopy.etag !== "string" || templateCopy.etag === "") {

Check warning on line 52 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .etag on an `any` value

Check warning on line 52 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .etag on an `any` value
throw new FirebaseError("ETag must be a non-empty string");
}
if (templateCopy.conditions && !Array.isArray(templateCopy.conditions)) {

Check warning on line 55 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .conditions on an `any` value

Check warning on line 55 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .conditions on an `any` value
throw new FirebaseError("Remote Config conditions must be an array");
}
return templateCopy;

Check warning on line 58 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe return of an `any` typed value
}

/**
Expand All @@ -55,7 +63,7 @@
* If force option is passed, etag value will be set to *. Otherwise, the etag will be created
* @param projectNumber Input is the Project number string
* @param template Remote Config template to deploy
* @param etag Remote Config Template's etag value

Check warning on line 66 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing @param "options.templateType"

Check warning on line 66 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing @param "options.force"
* @param options Optional object when publishing a Remote Config template. If the
* force {boolean} is `true` the Remote Config template is forced to update and circumvent the Etag
* @return Returns a Promise of a Remote Config template
Expand All @@ -64,15 +72,20 @@
projectNumber: string,
template: RemoteConfigTemplate,
etag: string,
options?: { force: boolean },
options?: { force: boolean; templateType?: string },
): Promise<RemoteConfigTemplate> {
const reqPath = `/projects/${projectNumber}/remoteConfig`;
const queryParams: { templateType?: string } = {};
if (options?.templateType) {
queryParams.templateType = options.templateType;
}
if (options?.force) {
etag = "*";
}
const response = await client.request<any, RemoteConfigTemplate>({

Check warning on line 85 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
method: "PUT",
path: reqPath,
queryParams,
headers: { "If-Match": etag },
body: {
conditions: template.conditions,
Expand All @@ -88,7 +101,7 @@
* Publishes a valid Remote Config template based on the Firebase Project Id using the deployTemplate function
* @param projectNumber Input is the Project number of the Firebase Project
* @param template The Remote Config template to be published
* @param etag Remote Config Template's etag value

Check warning on line 104 in src/deploy/remoteconfig/functions.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing @param "options.force"
* @param options Force boolean option
* @return Returns a Promise that fulfills with the published Remote Config template
*/
Expand All @@ -96,7 +109,7 @@
projectNumber: string,
template: RemoteConfigTemplate,
etag: string,
options?: { force: boolean },
options?: { force: boolean; templateType?: string },
): Promise<RemoteConfigTemplate> {
const temporaryTemplate = {
conditions: template.conditions,
Expand Down
34 changes: 30 additions & 4 deletions src/deploy/remoteconfig/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,45 @@ import { loadCJSON } from "../../loadCJSON";
import { getEtag } from "./functions";
import { validateInputRemoteConfigTemplate } from "./functions";
import { DeployOptions } from "../";
import { FirebaseError } from "../../error";

export default async function (context: any, options: DeployOptions): Promise<void> {
if (!context) {
return;
}
const filePath = options.config.src.remoteconfig?.template;
if (!filePath) {
return;
const onlyTargets = options.only?.split(",") ?? [];
const isServer = onlyTargets.includes("remoteconfig:server");
const isClient = onlyTargets.includes("remoteconfig") || !options.only;

if (isServer && isClient) {
throw new FirebaseError(
"Cannot deploy both Remote Config client and server templates in the same command.",
);
}

let filePath: string | undefined;
let templateType: string | undefined;

if (isServer) {
filePath = options.config.src.remoteconfig?.server?.template;
templateType = "firebase-server";
if (!filePath) {
throw new FirebaseError(
"No server template found in firebase.json. Please configure `remoteconfig.server.template`.",
);
}
} else {
filePath = options.config.src.remoteconfig?.template;
if (!filePath) {
return;
}
}

const template = loadCJSON(filePath);
const projectNumber = await needProjectNumber(options);
template.etag = await getEtag(projectNumber);
template.etag = await getEtag(projectNumber, undefined, templateType);
validateInputRemoteConfigTemplate(template);
context.remoteconfigTemplate = template;
context.remoteconfigTemplateType = templateType;
return;
}
6 changes: 4 additions & 2 deletions src/deploy/remoteconfig/release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import { RemoteConfigTemplate } from "../../remoteconfig/interfaces";

interface ReleaseContext {
remoteconfigTemplate?: RemoteConfigTemplate;
remoteconfigTemplateType?: string;
}

export default async function (context: ReleaseContext, options: any) {
if (!context?.remoteconfigTemplate) {
return;
}
const template = context.remoteconfigTemplate;
const templateType = context.remoteconfigTemplateType;
const projectNumber = await needProjectNumber(options);
const etag = await getEtag(projectNumber);
return publishTemplate(projectNumber, template, etag, options);
const etag = await getEtag(projectNumber, undefined, templateType);
return publishTemplate(projectNumber, template, etag, { ...options, templateType });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The etag is fetched again here to avoid race conditions, which is good. However, the template object passed to publishTemplate still contains the potentially stale etag from the prepare step. Inside publishTemplate, validation is performed on this template with its stale etag, which is inconsistent. To ensure validation uses the most up-to-date etag, you should create a new template object that includes the newly fetched etag before publishing.

  const templateWithNewEtag = { ...template, etag };
  return publishTemplate(projectNumber, templateWithNewEtag, etag, { ...options, templateType });

}
44 changes: 40 additions & 4 deletions src/deploy/remoteconfig/remoteconfig.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,37 @@ const currentTemplate: RemoteConfigTemplate = createTemplate("6");

describe("Remote Config Deploy", () => {
let sandbox: sinon.SinonSandbox;
let templateStub: sinon.SinonStub;
let etagStub: sinon.SinonStub;

beforeEach(() => {
sandbox = sinon.createSandbox();
templateStub = sandbox.stub(remoteconfig, "getTemplate");
etagStub = sandbox.stub(rcDeploy, "getEtag");
});

afterEach(() => {
sandbox.restore();
});

describe("getEtag", () => {
it("should get etag with templateType", async () => {
nock(remoteConfigApiOrigin())
.get(`/v1/projects/${PROJECT_NUMBER}/remoteConfig`)
.query({ templateType: "firebase-server" })
.reply(200, {}, { etag: "server-etag" });

const etag = await rcDeploy.getEtag(PROJECT_NUMBER, undefined, "firebase-server");
expect(etag).to.equal("server-etag");
expect(nock.isDone()).to.be.true;
});
});

describe("Publish the updated template", () => {
let templateStub: sinon.SinonStub;
let etagStub: sinon.SinonStub;

beforeEach(() => {
templateStub = sandbox.stub(remoteconfig, "getTemplate");
etagStub = sandbox.stub(rcDeploy, "getEtag");
});

it("should publish the latest template", async () => {
const ETAG = header.etag;
templateStub.withArgs(PROJECT_NUMBER).returns(currentTemplate);
Expand Down Expand Up @@ -111,6 +128,25 @@ describe("Remote Config Deploy", () => {
expect(nock.isDone()).to.be.true;
});

it("should publish the latest template with templateType", async () => {
const ETAG = header.etag;
const templateType = "firebase-server";
templateStub.withArgs(PROJECT_NUMBER).returns(currentTemplate);
nock(remoteConfigApiOrigin())
.put(`/v1/projects/${PROJECT_NUMBER}/remoteConfig`)
.query({ templateType: templateType })
.matchHeader("If-Match", ETAG)
.reply(200, expectedTemplateInfo);

const RCtemplate = await rcDeploy.publishTemplate(PROJECT_NUMBER, currentTemplate, ETAG, {
force: false,
templateType,
});

expect(RCtemplate).to.deep.equal(expectedTemplateInfo);
expect(nock.isDone()).to.be.true;
});

it("should reject if the api call fails", async () => {
const ETAG = header.etag;
etagStub.withArgs(PROJECT_NUMBER, "6").returns(ETAG);
Expand Down
5 changes: 4 additions & 1 deletion src/firebaseConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,10 @@ export type HostingConfig = HostingSingle | HostingMultiple;
export type StorageConfig = StorageSingle | StorageMultiple;

export type RemoteConfigConfig = {
template: string;
template?: string;
server?: {
template: string;
};
} & Deployable;

export type EmulatorsConfig = {
Expand Down
2 changes: 1 addition & 1 deletion src/init/features/remoteconfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ export async function doSetup(setup: Setup, config: Config): Promise<void> {
setup.config.remoteconfig = {
template: jsonFilePath,
};
config.writeProjectFile(setup.config.remoteconfig.template, "{}");
config.writeProjectFile(setup.config.remoteconfig.template!, "{}");
}
Loading