diff --git a/src/commands/crashlytics-heapdumps-status.ts b/src/commands/crashlytics-heapdumps-status.ts index a326b5645b4..0b0c8a802d6 100644 --- a/src/commands/crashlytics-heapdumps-status.ts +++ b/src/commands/crashlytics-heapdumps-status.ts @@ -27,7 +27,7 @@ export const command = new Command("crashlytics:heapdumps:status") table.push([ appId, config.heapDumpCollectionEnabled ? clc.green("Enabled") : clc.red("Disabled"), - config.gcsBucket || clc.yellow("(none)"), + config.bucket || clc.yellow("(none)"), ]); logger.info(""); diff --git a/src/commands/crashlytics-heapdumps.spec.ts b/src/commands/crashlytics-heapdumps.spec.ts index a474614f04d..98a123046b5 100644 --- a/src/commands/crashlytics-heapdumps.spec.ts +++ b/src/commands/crashlytics-heapdumps.spec.ts @@ -178,7 +178,7 @@ describe("crashlytics:heapdumps commands", () => { describe("crashlytics:heapdumps:status", () => { it("should return the status and configuration when enabled", async () => { sinon.stub(profilingManager, "getProfilingManagerConfig").resolves({ - gcsBucket: bucketName, + bucket: bucketName, heapDumpCollectionEnabled: true, }); @@ -189,14 +189,14 @@ describe("crashlytics:heapdumps commands", () => { expect(result).to.deep.equal({ appId, - gcsBucket: bucketName, + bucket: bucketName, heapDumpCollectionEnabled: true, }); }); it("should return the status and configuration when disabled with no bucket", async () => { sinon.stub(profilingManager, "getProfilingManagerConfig").resolves({ - gcsBucket: "", + bucket: "", heapDumpCollectionEnabled: false, }); @@ -207,14 +207,14 @@ describe("crashlytics:heapdumps commands", () => { expect(result).to.deep.equal({ appId, - gcsBucket: "", + bucket: "", heapDumpCollectionEnabled: false, }); }); it("should register and run crashlytics.heapdumps commands via command loader", async () => { sinon.stub(profilingManager, "getProfilingManagerConfig").resolves({ - gcsBucket: bucketName, + bucket: bucketName, heapDumpCollectionEnabled: true, }); @@ -239,7 +239,7 @@ describe("crashlytics:heapdumps commands", () => { expect(result).to.deep.equal({ appId, - gcsBucket: bucketName, + bucket: bucketName, heapDumpCollectionEnabled: true, }); }); diff --git a/src/crashlytics/profilingManager.spec.ts b/src/crashlytics/profilingManager.spec.ts index a5d94954b41..0248c13aad9 100644 --- a/src/crashlytics/profilingManager.spec.ts +++ b/src/crashlytics/profilingManager.spec.ts @@ -35,7 +35,9 @@ describe("profilingManager", () => { const hashedAppId = "abcdef123456"; const appId = `1:${projectNumber}:android:${hashedAppId}`; const expectedBucketName = `firebasecrashlytics-heap-dumps-${hashedAppId}`; + const expectedBucketResourceName = `projects/_/buckets/${expectedBucketName}`; const projectId = "my-test-project"; + const resourcePath = `/v1alpha/projects/${projectNumber}/apps/${appId}/profilingManagerConfig`; afterEach(() => { nock.cleanAll(); @@ -175,34 +177,23 @@ describe("profilingManager", () => { describe("getProfilingManagerConfig", () => { it("should return the configuration on successful response", async () => { const mockResponse = { - configuration: { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }, + name: `projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, }; - nock(crashlyticsApiOrigin()) - .get(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`) - .reply(200, mockResponse); + nock(crashlyticsApiOrigin()).get(resourcePath).reply(200, mockResponse); const result = await getProfilingManagerConfig(appId); - expect(result).to.deep.equal({ - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }); + expect(result).to.deep.equal(mockResponse); expect(nock.isDone()).to.be.true; }); - it("should return default values when configuration is empty", async () => { - nock(crashlyticsApiOrigin()) - .get(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`) - .reply(200, {}); + it("should return empty object when configuration is empty", async () => { + nock(crashlyticsApiOrigin()).get(resourcePath).reply(200, {}); const result = await getProfilingManagerConfig(appId); - expect(result).to.deep.equal({ - gcsBucket: "", - heapDumpCollectionEnabled: false, - }); + expect(result).to.deep.equal({}); expect(nock.isDone()).to.be.true; }); @@ -215,49 +206,64 @@ describe("profilingManager", () => { }); describe("updateProfilingManagerConfig", () => { - it("should send the update request successfully", async () => { + it("should send the PATCH update request with field mask successfully", async () => { nock(crashlyticsApiOrigin()) - .post(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`, { - projectNumber, - gmpAppId: appId, - configuration: { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }, + .patch(resourcePath, { + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, }) - .reply(200, {}); + .query({ updateMask: "bucket,heapDumpCollectionEnabled" }) + .reply(200, { + name: `projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, + }); - await updateProfilingManagerConfig(appId, { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }); + await updateProfilingManagerConfig( + appId, + { + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, + }, + "bucket,heapDumpCollectionEnabled", + ); expect(nock.isDone()).to.be.true; }); - it("should default empty configuration fields when omitted", async () => { + it("should send partial updateMask when updating only heapDumpCollectionEnabled", async () => { nock(crashlyticsApiOrigin()) - .post(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`, { - projectNumber, - gmpAppId: appId, - configuration: { - gcsBucket: "", - heapDumpCollectionEnabled: false, - }, + .patch(resourcePath, { + heapDumpCollectionEnabled: false, }) - .reply(200, {}); + .query({ updateMask: "heapDumpCollectionEnabled" }) + .reply(200, { + name: `projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: false, + }); - await updateProfilingManagerConfig(appId, {}); + await updateProfilingManagerConfig( + appId, + { + heapDumpCollectionEnabled: false, + }, + "heapDumpCollectionEnabled", + ); expect(nock.isDone()).to.be.true; }); it("should throw a FirebaseError if the appId is invalid", async () => { await expect( - updateProfilingManagerConfig("invalid-app", { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }), + updateProfilingManagerConfig( + "invalid-app", + { + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, + }, + "bucket,heapDumpCollectionEnabled", + ), ).to.be.rejectedWith(FirebaseError, "Unable to get the projectId from the AppId."); }); }); @@ -453,15 +459,16 @@ describe("profilingManager", () => { sinon.stub(serviceusage, "generateServiceIdentityAndPoll").resolves(); sinon.stub(resourceManager, "serviceAccountHasRoles").resolves(true); nock(crashlyticsApiOrigin()) - .post(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`, { - projectNumber, - gmpAppId: appId, - configuration: { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }, + .patch(resourcePath, { + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, }) - .reply(200, {}); + .query({ updateMask: "bucket,heapDumpCollectionEnabled" }) + .reply(200, { + name: `projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: true, + }); const result = await enableHeapDumpCollection(projectId, appId); @@ -469,25 +476,17 @@ describe("profilingManager", () => { expect(nock.isDone()).to.be.true; }); - it("should preserve existing bucket name and disable heap dump collection", async () => { + it("should disable heap dump collection via partial updateMask while preserving bucket", async () => { nock(crashlyticsApiOrigin()) - .get(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`) + .patch(resourcePath, { + heapDumpCollectionEnabled: false, + }) + .query({ updateMask: "heapDumpCollectionEnabled" }) .reply(200, { - configuration: { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: true, - }, + name: `projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, + bucket: expectedBucketResourceName, + heapDumpCollectionEnabled: false, }); - nock(crashlyticsApiOrigin()) - .post(`/v1/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`, { - projectNumber, - gmpAppId: appId, - configuration: { - gcsBucket: expectedBucketName, - heapDumpCollectionEnabled: false, - }, - }) - .reply(200, {}); await disableHeapDumpCollection(appId); diff --git a/src/crashlytics/profilingManager.ts b/src/crashlytics/profilingManager.ts index 961c7a65661..218d44fa691 100644 --- a/src/crashlytics/profilingManager.ts +++ b/src/crashlytics/profilingManager.ts @@ -1,24 +1,12 @@ -import { Client } from "../apiv2"; -import { crashlyticsApiOrigin } from "../api"; import { FirebaseError, getErrStatus } from "../error"; import { logger } from "../logger"; import { logLabeledBullet, logLabeledSuccess } from "../utils"; -import { parseProjectNumber, TIMEOUT } from "./utils"; +import { CRASHLYTICS_API_CLIENT, parseProjectNumber, TIMEOUT } from "./utils"; import * as storage from "../gcp/storage"; import * as resourceManager from "../gcp/resourceManager"; import * as serviceusage from "../gcp/serviceusage"; import { AppPlatform, listFirebaseApps, selectAppInteractively } from "../management/apps"; -/** - * Returns an authenticated v1 API client for the Firebase Crashlytics API. - */ -export function getCrashlyticsV1Client(): Client { - return new Client({ - urlPrefix: crashlyticsApiOrigin(), - apiVersion: "v1", - }); -} - export const CRASHLYTICS_SERVICE_NAME = "firebasecrashlytics.googleapis.com"; export const STORAGE_OBJECT_CREATOR_ROLE = "roles/storage.objectCreator"; export const HEAP_DUMP_BUCKET_PREFIX = "firebasecrashlytics-heap-dumps-"; @@ -40,25 +28,14 @@ export const DEFAULT_LIFECYCLE_RULES: storage.LifecycleRule[] = [ ]; const ANDROID_APP_ID_REGEX = /^\d+:(\d+):android:([a-fA-F0-9]+)$/; +const GCS_BUCKET_RESOURCE_PREFIX = "projects/_/buckets/"; export interface ProfilingManagerConfig { - gcsBucket?: string; + name?: string; + bucket?: string; heapDumpCollectionEnabled?: boolean; } -export interface GetProfilingManagerConfigResponse { - configuration?: ProfilingManagerConfig; -} - -export interface UpdateProfilingManagerConfigRequest { - projectNumber: string; - gmpAppId: string; - configuration: { - gcsBucket: string; - heapDumpCollectionEnabled: boolean; - }; -} - /** * Validates an Android GMP App ID and returns its lowercased hashed package name segment. * @param appId GMP App ID (e.g. 1:123456789:android:abcdef123456) @@ -135,48 +112,40 @@ export async function resolveAndroidAppId( export async function getProfilingManagerConfig(appId: string): Promise { const projectNumber = parseProjectNumber(appId); logger.debug(`[crashlytics] getProfilingManagerConfig called with appId: ${appId}`); - const response = await getCrashlyticsV1Client().request({ + const response = await CRASHLYTICS_API_CLIENT.request({ method: "GET", headers: { "Content-Type": "application/json", }, - path: `/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`, + path: `/projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, timeout: TIMEOUT, }); - const config = response.body?.configuration || {}; - return { - gcsBucket: config.gcsBucket || "", - heapDumpCollectionEnabled: config.heapDumpCollectionEnabled ?? false, - }; + return response.body; } /** * Updates the Crashlytics Profiling Manager configuration for an app. * @param appId GMP App ID - * @param config Profiling manager configuration + * @param config Profiling manager configuration fields to update + * @param updateMask Comma-separated field mask of fields to update */ export async function updateProfilingManagerConfig( appId: string, config: ProfilingManagerConfig, + updateMask: string, ): Promise { const projectNumber = parseProjectNumber(appId); logger.debug( - `[crashlytics] updateProfilingManagerConfig called with appId: ${appId}, config: ${JSON.stringify(config)}`, + `[crashlytics] updateProfilingManagerConfig called with appId: ${appId}, config: ${JSON.stringify(config)}, updateMask: ${updateMask}`, ); - await getCrashlyticsV1Client().request({ - method: "POST", + await CRASHLYTICS_API_CLIENT.request({ + method: "PATCH", headers: { "Content-Type": "application/json", }, - path: `/projects/${projectNumber}/apps/${appId}/appconfig:profilingManager`, - body: { - projectNumber, - gmpAppId: appId, - configuration: { - gcsBucket: config.gcsBucket ?? "", - heapDumpCollectionEnabled: config.heapDumpCollectionEnabled ?? false, - }, - }, + path: `/projects/${projectNumber}/apps/${appId}/profilingManagerConfig`, + queryParams: { updateMask }, + body: config, timeout: TIMEOUT, }); } @@ -296,10 +265,14 @@ export async function enableHeapDumpCollection( logLabeledBullet("crashlytics", "Configuring service agent permissions..."); await ensureHeapDumpP4saRole(projectId, projectNumber); logLabeledBullet("crashlytics", "Enabling Crashlytics heap dump collection..."); - await updateProfilingManagerConfig(appId, { - gcsBucket: bucketName, - heapDumpCollectionEnabled: true, - }); + await updateProfilingManagerConfig( + appId, + { + bucket: `${GCS_BUCKET_RESOURCE_PREFIX}${bucketName}`, + heapDumpCollectionEnabled: true, + }, + "bucket,heapDumpCollectionEnabled", + ); logLabeledSuccess("crashlytics", "Successfully enabled Crashlytics heap dump collection!"); return bucketName; } @@ -310,10 +283,12 @@ export async function enableHeapDumpCollection( */ export async function disableHeapDumpCollection(appId: string): Promise { logLabeledBullet("crashlytics", "Disabling Crashlytics heap dump collection..."); - const currentConfig = await getProfilingManagerConfig(appId); - await updateProfilingManagerConfig(appId, { - gcsBucket: currentConfig.gcsBucket, - heapDumpCollectionEnabled: false, - }); + await updateProfilingManagerConfig( + appId, + { + heapDumpCollectionEnabled: false, + }, + "heapDumpCollectionEnabled", + ); logLabeledSuccess("crashlytics", "Successfully disabled Crashlytics heap dump collection!"); }