Repository: firebase/firebase-tools (ts)

Issue:
storage deployment to specific target deploy to all targets
<!-- DO NOT DELETE
validate_template=true
template_path=.github/ISSUE_TEMPLATE/bug_report.md
-->

<!--
Thank you for contributing to the Firebase community!

Think you found a bug?
=======================
Yeah, we're definitely not perfect! Please use this template and include a minimal repro when opening the issue. If you know how to solve the issue, please create a Pull Request, and we'd be happy to review it!

Have a feature request?
========================
Great, we love hearing how we can improve our products! However, do not use this template to submit a feature request. Please submit your feature requests to: https://firebase.google.com/support/contact/bugs-features/

Have a usage question?
=======================
We get lots of those and we love helping you, but GitHub is not the best place for them and they will be closed. Please take a look at the guide first: https://firebase.google.com/docs/cli/

If the official documentation doesn't help, try asking through our official support channel: https://firebase.google.com/support/

Additional locations to check for solutions or assistance from the community:
- Stack Overflow: https://stackoverflow.com/
- Firebase Slack Community: https://firebase.community/

*Please avoid duplicate posting across multiple channels!*
-->

When trying to deploy storage rules to a specific target the deployment goes to all targets

### [REQUIRED] Environment info

**firebase-tools:**. 9.13.0

**Platform:** macOS

### [REQUIRED] Test case

"firebase.json":
```
{
...
"storage": [
    {
      "target": "main",
      "rules": "storage.rules"
    },
    {
      "target": "backup",
      "rules": "storage-backup.rules"
    }
  ]
}
```

".firebaserc"
```
{
  "targets": {
    "my-firebase-project": {
      "storage": {
        "backup": [
          "my-firebase-project-backup"
        ],
        "main": [
          "my-firebase-project.appspot.com"
        ]
      }
    }
  }
}
```

### [REQUIRED] Steps to reproduce

deploy only backup target
firebase deploy --only storage:backup

### [REQUIRED] Expected behavior

deploy rules only for the backup target

### [REQUIRED] Actual behavior

i  storage: uploading rules storage.rules...
i  storage: latest version of storage-backup.rules already up to date, skipping upload...
✔  storage: released rules storage-backup.rules to firebase.storage
✔  storage: released rules storage.rules to firebase.storage


Pull request:
fix storage targets logic
<!--

Thank you for contributing to the Firebase community! Please fill out the form below.

Run the linter and test suite
==============================
Run `npm test` to make sure your changes compile properly and the tests all pass on your local machine. We've hooked up this repo with continuous integration to double check those things for you.

-->

### Description

`storage` targets weren't being fully respected. It turned out that while `prepare` was filtering them out, the `release` step was trying to deploy them all anyway. With this update, `release` now looks at an appropriate list for deployment and the only targets are working again.

```
» firebase deploy --only storage

=== Deploying to 'xxx'...

i  deploying storage
i  firebase.storage: checking storage.rules for compilation errors...
i  firebase.storage: checking storage.other.rules for compilation errors...
✔  firebase.storage: rules file storage.other.rules compiled successfully
✔  firebase.storage: rules file storage.rules compiled successfully
i  storage: uploading rules storage.rules...
i  storage: latest version of storage.other.rules already up to date, skipping upload...
✔  storage: released rules storage.other.rules to firebase.storage
✔  storage: released rules storage.rules to firebase.storage

✔  Deploy complete!

Project Console: xxx
» firebase deploy --only storage:storage-other

=== Deploying to 'xxx'...

i  deploying storage
i  firebase.storage: checking storage.other.rules for compilation errors...
✔  firebase.storage: rules file storage.other.rules compiled successfully
i  storage: latest version of storage.other.rules already up to date, skipping upload...
✔  storage: released rules storage.other.rules to firebase.storage

✔  Deploy complete!

Project Console: xxx
```

This completes and...

Fixes #3496

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9d853c89e..1d808e656 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,11 +1,12 @@
 - Turns on content compression in the Hosting emulator. (#2328)
 - Replaces underlying terminal coloring library.
-- Make storage emulator multipart parsing handle quotes in boundary header. (#3953)
-- Make storage emulator content type case insensitive. (#3953)
-- Add storage emulator support to init.js useEmulator flag. (#4805)
-- Populate resource correctly in storage rules evaluation. (#4329)
-- Read name from metadata as backup for gcs upload into storage emulator. (#3953)
-- Fix bug where invalid CPU was set for 16GiB functions. (#4823)
-- Fix bug where failed function discovery crashed the entire emulator. (#4826)
-- Fix LIST security rule evaluation in storage emulator. (#4827)
-- Fix bug where vpc connector setting broke function deploy for old functions SDK. (#4834)
+- Makes storage emulator multipart parsing handle quotes in boundary header. (#3953)
+- Makes storage emulator content type case insensitive. (#3953)
+- Adds storage emulator support to init.js useEmulator flag. (#4805)
+- Populates resource correctly in storage rules evaluation. (#4329)
+- Reads name from metadata as backup for gcs upload into storage emulator. (#3953)
+- Fixes bug where invalid CPU was set for 16GiB functions. (#4823)
+- Fixes bug where failed function discovery crashed the entire emulator. (#4826)
+- Fixes LIST security rule evaluation in storage emulator. (#4827)
+- Fixes bug where vpc connector setting broke function deploy for old functions SDK. (#4834)
+- Fixes issue where storage deploy targets weren't being fully respected. (#3496)
diff --git a/src/deploy/storage/prepare.ts b/src/deploy/storage/prepare.ts
index 6bd2e71a6..08271c017 100644
--- a/src/deploy/storage/prepare.ts
+++ b/src/deploy/storage/prepare.ts
@@ -31,15 +31,12 @@ export default async function (context: any, options: Options): Promise<void> {
     }
   }
 
-  _.set(context, "storage.rules", rulesConfig);
-
   const rulesDeploy = new RulesDeploy(options, RulesetServiceType.FIREBASE_STORAGE);
-  _.set(context, "storage.rulesDeploy", rulesDeploy);
+  const rulesConfigsToDeploy: any[] = [];
 
   if (!Array.isArray(rulesConfig)) {
     const defaultBucket = await gcp.storage.getDefaultBucket(options.project);
     rulesConfig = [Object.assign(rulesConfig, { bucket: defaultBucket })];
-    _.set(context, "storage.rules", rulesConfig);
   }
 
   for (const ruleConfig of rulesConfig) {
@@ -48,8 +45,9 @@ export default async function (context: any, options: Options): Promise<void> {
       options.rc.requireTarget(context.projectId, "storage", target);
     }
     if (allStorage || onlyTargets.has(target)) {
-      rulesDeploy.addFile(ruleConfig.rules);
-      onlyTargets.delete(target);
+      rulesDeploy.addFile(ruleConfig.rules); // Add the rules to the deploy object.
+      rulesConfigsToDeploy.push(ruleConfig); // Copy the rule config into our list of configs to deploy.
+      onlyTargets.delete(target); // Remove the target from our only list.
     }
   }
 
@@ -59,5 +57,9 @@ export default async function (context: any, options: Options): Promise<void> {
     );
   }
 
+  _.set(context, "storage.rules", rulesConfig);
+  _.set(context, "storage.rulesConfigsToDeploy", rulesConfigsToDeploy);
+  _.set(context, "storage.rulesDeploy", rulesDeploy);
+
   await rulesDeploy.compile();
 }
diff --git a/src/deploy/storage/release.ts b/src/deploy/storage/release.ts
index f27d65cd1..bc3177b8b 100644
--- a/src/deploy/storage/release.ts
+++ b/src/deploy/storage/release.ts
@@ -6,16 +6,17 @@ import { RulesDeploy, RulesetServiceType } from "../../rulesDeploy";
  * Releases Firebase Storage rules.
  * @param context The deploy context.
  * @param options The CLI options object.
+ * @return the list of buckets deployed.
  */
-export default async function (context: any, options: any): Promise<void> {
-  const rules = get(context, "storage.rules", []);
+export default async function (context: any, options: any): Promise<string[]> {
+  const rulesConfigsToDeploy: any[] = get(context, "storage.rulesConfigsToDeploy", []);
   const rulesDeploy: RulesDeploy = get(context, "storage.rulesDeploy");
-  if (!rules.length || !rulesDeploy) {
-    return;
+  if (!rulesConfigsToDeploy.length || !rulesDeploy) {
+    return [];
   }
 
   const toRelease: Array<{ bucket: string; rules: any }> = [];
-  for (const ruleConfig of rules) {
+  for (const ruleConfig of rulesConfigsToDeploy) {
     if (ruleConfig.target) {
       options.rc.target(options.project, "storage", ruleConfig.target).forEach((bucket: string) => {
         toRelease.push({ bucket: bucket, rules: ruleConfig.rules });
@@ -26,12 +27,10 @@ export default async function (context: any, options: any): Promise<void> {
   }
 
   await Promise.all(
-    toRelease.map((release) => {
-      return rulesDeploy.release(
-        release.rules,
-        RulesetServiceType.FIREBASE_STORAGE,
-        release.bucket
-      );
+    toRelease.map((r) => {
+      return rulesDeploy.release(r.rules, RulesetServiceType.FIREBASE_STORAGE, r.bucket);
     })
   );
+
+  return toRelease.map((r) => r.bucket);
 }
diff --git a/src/rulesDeploy.ts b/src/rulesDeploy.ts
index 05f59c593..d25713071 100644
--- a/src/rulesDeploy.ts
+++ b/src/rulesDeploy.ts
@@ -1,5 +1,5 @@
 import * as _ from "lodash";
-import * as clc from "colorette";
+import { bold } from "colorette";
 import * as fs from "fs-extra";
 
 import * as gcp from "./gcp";
@@ -75,7 +75,7 @@ export class RulesDeploy {
       src = fs.readFileSync(fullPath, "utf8");
     } catch (e: any) {
       logger.debug("[rules read error]", e.stack);
-      throw new FirebaseError("Error reading rules file " + clc.bold(path));
+      throw new FirebaseError(`Error reading rules file ${bold(path)}`);
     }
 
     this.rulesFiles[path] = [{ name: path, content: src }];
@@ -142,10 +142,9 @@ export class RulesDeploy {
       // Try to add the role to the service account
       if (addRole) {
         await addServiceAccountToRoles(projectNumber, saEmail, [CROSS_SERVICE_RULES_ROLE], true);
-        utils.logBullet(
-          `${clc.bold(
-            clc.cyan(RulesetType[this.type] + ":")
-          )} updated service account for cross-service rules...`
+        utils.logLabeledBullet(
+          RulesetType[this.type],
+          "updated service account for cross-service rules..."
         );
       }
     } catch (e: any) {
@@ -177,10 +176,9 @@ export class RulesDeploy {
     const newRulesetsByFilename = new Map<string, Promise<string>>();
     for (const [filename, files] of Object.entries(this.rulesFiles)) {
       if (latestRulesetName && _.isEqual(files, latestRulesetContent)) {
-        utils.logBullet(
-          `${clc.bold(clc.cyan(RulesetType[this.type] + ":"))} latest version of ${clc.bold(
-            filename
-          )} already up to date, skipping upload...`
+        utils.logLabeledBullet(
+          RulesetType[this.type],
+          `latest version of ${bold(filename)} already up to date, skipping upload...`
         );
         this.rulesetNames[filename] = latestRulesetName;
         continue;
@@ -189,11 +187,7 @@ export class RulesDeploy {
         await this.checkStorageRulesIamPermissions(files[0]?.content);
       }
 
-      utils.logBullet(
-        `${clc.bold(clc.cyan(RulesetType[this.type] + ":"))} uploading rules ${clc.bold(
-          filename
-        )}...`
-      );
+      utils.logLabeledBullet(RulesetType[this.type], `uploading rules ${bold(filename)}...`);
       newRulesetsByFilename.set(filename, gcp.rules.createRuleset(this.options.project, files));
     }
 
@@ -208,10 +202,7 @@ export class RulesDeploy {
       if (err.status !== QUOTA_EXCEEDED_STATUS_CODE) {
         throw err;
       }
-      utils.logBullet(
-        clc.bold(clc.yellow(RulesetType[this.type] + ":")) +
-          " quota exceeded error while uploading rules"
-      );
+      utils.logLabeledBullet(RulesetType[this.type], "quota exceeded error while uploading rules");
 
       const history: ListRulesetsEntry[] = await gcp.rules.listAllRulesets(this.options.project);
 
@@ -237,9 +228,7 @@ export class RulesDeploy {
             await gcp.rules.deleteRuleset(this.options.project, gcp.rules.getRulesetId(entry));
             logger.debug(`[rules] Deleted ${entry.name}`);
           }
-          utils.logBullet(
-            clc.bold(clc.yellow(RulesetType[this.type] + ":")) + " retrying rules upload"
-          );
+          utils.logLabeledWarning(RulesetType[this.type], "retrying rules upload");
           return this.createRulesets(service);
         }
       }
@@ -270,10 +259,9 @@ export class RulesDeploy {
         ? `${resourceName}/${subResourceName}`
         : resourceName
     );
-    utils.logSuccess(
-      `${clc.bold(clc.green(RulesetType[this.type] + ":"))} released rules ${clc.bold(
-        filename
-      )} to ${clc.bold(resourceName)}`
+    utils.logLabeledSuccess(
+      RulesetType[this.type],
+      `released rules ${bold(filename)} to ${bold(resourceName)}`
     );
   }
 
@@ -283,11 +271,7 @@ export class RulesDeploy {
    * @param files The files to compile.
    */
   private async compileRuleset(filename: string, files: RulesetFile[]): Promise<void> {
-    utils.logBullet(
-      `${clc.bold(clc.cyan(this.type + ":"))} checking ${clc.bold(
-        filename
-      )} for compilation errors...`
-    );
+    utils.logLabeledBullet(this.type, `checking ${bold(filename)} for compilation errors...`);
     const response = await gcp.rules.testRuleset(this.options.project, files);
     if (_.get(response, "body.issues", []).length) {
       const warnings: string[] = [];
@@ -312,15 +296,11 @@ export class RulesDeploy {
 
       if (errors.length > 0) {
         const add = errors.length === 1 ? "" : "s";
-        const message = `Compilation error${add} in ${clc.bold(filename)}:\n${errors.join("\n")}`;
+        const message = `Compilation error${add} in ${bold(filename)}:\n${errors.join("\n")}`;
         throw new FirebaseError(message, { exit: 1 });
       }
     }
 
-    utils.logSuccess(
-      `${clc.bold(clc.green(this.type + ":"))} rules file ${clc.bold(
-        filename
-      )} compiled successfully`
-    );
+    utils.logLabeledSuccess(this.type, `rules file ${bold(filename)} compiled successfully`);
   }
 }


Test patch:
diff --git a/src/test/deploy/storage/release.spec.ts b/src/test/deploy/storage/release.spec.ts
new file mode 100644
index 000000000..43686bd4b
--- /dev/null
+++ b/src/test/deploy/storage/release.spec.ts
@@ -0,0 +1,47 @@
+import { expect } from "chai";
+import * as sinon from "sinon";
+
+import { RulesDeploy } from "../../../rulesDeploy";
+
+import { default as release } from "../../../deploy/storage/release";
+import { RC } from "../../../rc";
+
+describe("storage.release", () => {
+  it("should not release anything if there are no deployable configs", async () => {
+    const rulesDeploy = sinon.createStubInstance(RulesDeploy);
+    rulesDeploy.release.resolves();
+    await expect(release({ storage: { rulesDeploy } }, {})).to.eventually.deep.equal([]);
+    expect(rulesDeploy.release).to.not.be.called;
+  });
+
+  it("should release rules for a single deploy config", async () => {
+    const rulesDeploy = sinon.createStubInstance(RulesDeploy);
+    rulesDeploy.release.resolves();
+    const context = {
+      storage: {
+        rulesDeploy,
+        rulesConfigsToDeploy: [{ bucket: "foo", rules: "true" }],
+      },
+    };
+
+    await expect(release(context, {})).to.eventually.deep.equal(["foo"]);
+    expect(rulesDeploy.release).to.be.calledOnce;
+  });
+
+  it("should release rules based on targets", async () => {
+    const project = "my-project";
+    const rulesDeploy = sinon.createStubInstance(RulesDeploy);
+    rulesDeploy.release.resolves();
+    const rc = sinon.createStubInstance(RC);
+    rc.target.withArgs(project, "storage", "my-target").returns(["bar"]);
+    const context = {
+      storage: {
+        rulesDeploy,
+        rulesConfigsToDeploy: [{ target: "my-target", rules: "true" }],
+      },
+    };
+
+    await expect(release(context, { project, rc })).to.eventually.deep.equal(["bar"]);
+    expect(rulesDeploy.release).to.be.calledOnce;
+  });
+});