Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

chore(ci): several fixes/improvements to Firebase deployments #17114

Closed
wants to merge 9 commits into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Feb 5, 2021

Several fixes and improvements related to deploying to the code-angularjs-org and docs-angularjs-org Firebase projects.
See individual commits for details.

Fixes #17111 and angular/angularjs.org#251.

…kage-lock.json`)

Since we are using `yarn` to install dependencies, it makes sense to use
`yarn.lock` as the lockfile (and get rid of the unused
`package-lock.json`).
…tories

We have the `scripts/{code,docs}.angularjs.org-firebase/` directories,
which contain the necessary code and config for deploying built files to
the `code-angularjs-org` and `docs-angularjs-org` Firebase projects
respectively.

Previously, some of the files that needed to be deployed to Firebase (or
Google Cloud) were placed outside these directories (e.g. in
`deploy/{code,docs}/`).

Since these files are only used for deploying to Firebase/Google Cloud,
this commit changes the deployment process to instead copy the files
inside the directories. In a subsequent commit, this will allow
simplifying the deployment process, by running it from inside each
directory instead of having to copy the `firebase.json` files to the
repository root (and adjust the paths).

These are the destination directory changes:

| Before       | After                                       |
|--------------|---------------------------------------------|
| deploy/code/ | scripts/code.angularjs.org-firebase/deploy/ |
| deploy/docs/ | scripts/docs.angularjs.org-firebase/deploy/ |
@google-cla google-cla bot added the cla: yes label Feb 5, 2021
@gkalpak gkalpak changed the title (ci): several fixes/improvements to Firebase deployments chore(ci): several fixes/improvements to Firebase deployments Feb 5, 2021
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I mostly skimmed this and I admit I didn't look at every line. But it looks good to me - assuming it works in real life 😸

Typo in 3rd commit reporitory
And commits 7 and 8 need a target.

…ng directory

Previously, in order to deploy to Firebase from
`scripts/docs.angularjs.org-firebase/`, we had to copy the
`firebase.json` file to the repository root and adjust the contained
paths accordingly.

By running the `firebase` CLI directly (instead of via `yarn`), we are
able to deploy from `docs.angularjs.org-firebase/` directly. This
simplifies the deployment (and local testing) process and paves the way
for also deploying from `code.angularjs.org-firebase/` in a subsequent
commit.
Previously, we only deployed to Firebase hosting for the
`docs-angularjs-org` Firebase project. This meant that the deployed
functions used an old version of Node.js and started failing once
support was dropped for that version. See angular#17111 for details.

This commit fixes this by ensuring that we deploy to all enabled
Firebase services (currently functions and hosting).

Fixes angular#17111
Fixes angular/angularjs.org#251
Previously, we only deployed the built files to Google Cloud Storage for
the `code-angularjs-org` Firebase project. This meant that the other
Firebase services (such as functions, hosting, storage) were not updated
when we made changes to their source code or configuration.
(This isn't a problem at the moment, since the code/configuration for
these service changes infrequently, but could bite us in the future.)

This commit fixes this by ensuring that we deploy to all enabled
Firebase services (currently functions, hosting and storage).
Previously, when deploying `scripts/{code,docs}.angularjs.org-firebase/`
to Firebase, we explicitly specified the target projects. Since the
project IDs are also specified in the respective `.firebaserc` files,
this was unnecessary (and meant we had multiple places to update if the
IDs changed).

This commit simplifies the process by automatically targeting the
default projects (as configured in the `.firebaserc` files) when
deploying to Firebase in CI.
This commit updates `firebase-admin`, `firebase-functions` and
`firebase-tools` to latest versions to take advantage of the most recent
fixes and improvements.
…sendStoredFile()`

Previously, the `sendStoredFile()` Firebase function used `.split('/')`
to split the request path into segments and later used `path.join()` to
join them back together. This worked fine on *nix based systems, which
use `/` as the path separator. On Windows, however, where `\` is the
path separator, the re-constructed paths could not be retrieved from the
Google Cloud Storage bucket. This prevented the Firebase emulators from
working correctly when testing the function locally on Windows.

This commit fixes the issue by using `.join('/')` to join the path
segments back together.
@gkalpak gkalpak marked this pull request as ready for review February 6, 2021 09:48
@gkalpak gkalpak closed this in 63d613f Feb 6, 2021
@gkalpak gkalpak deleted the fix-deployment branch February 6, 2021 09:53
gkalpak added a commit that referenced this pull request Feb 6, 2021
…sendStoredFile()`

Previously, the `sendStoredFile()` Firebase function used `.split('/')`
to split the request path into segments and later used `path.join()` to
join them back together. This worked fine on *nix based systems, which
use `/` as the path separator. On Windows, however, where `\` is the
path separator, the re-constructed paths could not be retrieved from the
Google Cloud Storage bucket. This prevented the Firebase emulators from
working correctly when testing the function locally on Windows.

This commit fixes the issue by using `.join('/')` to join the path
segments back together.

Closes #17114
gkalpak added a commit that referenced this pull request Feb 6, 2021
In #17114, the `deploy-code` CI job was modified to also run some
commands via `yarn`. It turns out that this breaks on CI, because the
`deploy-code` job uses the `cloud-sdk` executor that does not have
`yarn` installed. ([Example failure][1])

This commit fixes this by splitting the `deploy-code` job into two jobs:
- `deploy-code-files` uses the `cloud-sdk` and uploads the files to
  Google Cloud.
  (This is essentially the `deploy-code` job from before #17114.)
- `deploy-code-firebase` uses the `default` executor (which has `yarn`
  installed) and deploys to Firebase.
  (This essentially includes the new bits added in #17114.)

[1]: https://circleci.com/gh/angular/angular.js/2712
gkalpak added a commit that referenced this pull request Feb 6, 2021
In #17114, the `deploy-code` CI job was modified to also run some
commands via `yarn`. It turns out that this breaks on CI, because the
`deploy-code` job uses the `cloud-sdk` executor that does not have
`yarn` installed. ([Example failure][1])

This commit fixes this by splitting the `deploy-code` job into two jobs:
- `deploy-code-files` uses the `cloud-sdk` and uploads the files to
  Google Cloud.
  (This is essentially the `deploy-code` job from before #17114.)
- `deploy-code-firebase` uses the `default` executor (which has `yarn`
  installed) and deploys to Firebase.
  (This essentially includes the new bits added in #17114.)

[1]: https://circleci.com/gh/angular/angular.js/2712
@Splaktar
Copy link
Member

Splaktar commented Mar 2, 2021

I finally had a chance to look through this. It all looks good to me! Lots of long-needed updates and alignments with the recent updates to Firebase.

AnkushLambdatest pushed a commit to AnkushLambdatest/angular.js that referenced this pull request Jun 29, 2022
…sendStoredFile()`

Previously, the `sendStoredFile()` Firebase function used `.split('/')`
to split the request path into segments and later used `path.join()` to
join them back together. This worked fine on *nix based systems, which
use `/` as the path separator. On Windows, however, where `\` is the
path separator, the re-constructed paths could not be retrieved from the
Google Cloud Storage bucket. This prevented the Firebase emulators from
working correctly when testing the function locally on Windows.

This commit fixes the issue by using `.join('/')` to join the path
segments back together.

Closes angular#17114
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Some) documentation website is broken
3 participants