Skip to content

fix storage and realtime database targets - #4782

Merged
bkendall merged 9 commits into
masterfrom
bk-4752
Jul 26, 2022
Merged

fix storage and realtime database targets#4782
bkendall merged 9 commits into
masterfrom
bk-4752

Conversation

@bkendall

@bkendall bkendall commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

Description

--only storage:<target> commands were failing when targets were applied to storage because the handling of the <target> was busted. We were manipulating an object the wrong way and causing undefined to be passed along.

I've updated the logic in storage/prepare to look at the only option and filter/check for targets. It will only deploy targets it understands and fail out if an unknown target is provided.

Debugging this further (and reading https://firebase.google.com/docs/rules/manage-deploy#realtime-database) made me realize that we weren't doing any of this checking/filtering on realtime database either. I've updated logic for that product as well.

Fixes #4752

Scenarios Tested

  • storage deploy with a single config value in firebase.json
  • storage deploy with a target, using the correct name
  • storage deploy with a target, using the incorrect name
  • deploy of all targets

tested database deployments in the same ways as above.

@codecov-commenter

codecov-commenter commented Jul 26, 2022

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 16.98113% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.65%. Comparing base (ba686fd) to head (7bb4a41).

Files with missing lines Patch % Lines
src/database/rulesConfig.ts 0.00% 24 Missing ⚠️
src/deploy/storage/prepare.ts 9.52% 19 Missing ⚠️
src/deploy/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4782      +/-   ##
==========================================
- Coverage   57.74%   57.65%   -0.10%     
==========================================
  Files         287      287              
  Lines       18592    18623      +31     
  Branches     3643     3654      +11     
==========================================
+ Hits        10736    10737       +1     
- Misses       6977     7007      +30     
  Partials      879      879              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bkendall bkendall changed the title fix storage targets fix storage and realtime targets Jul 26, 2022
@bkendall bkendall changed the title fix storage and realtime targets fix storage and realtime database targets Jul 26, 2022

@tonyjhuang tonyjhuang left a comment

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.

Thanks for the fix!

@yuchenshi
yuchenshi marked this pull request as draft July 26, 2022 16:18
@bkendall
bkendall marked this pull request as ready for review July 26, 2022 16:20
@bkendall
bkendall changed the base branch from bk-api-consumer to master July 26, 2022 16:20
@bkendall
bkendall enabled auto-merge (squash) July 26, 2022 16:27
@bkendall
bkendall merged commit e239ebf into master Jul 26, 2022
@bkendall
bkendall deleted the bk-4752 branch July 26, 2022 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI does not support deploying storage rules to multiple buckets

3 participants