GoogleCloudPlatform / buildpacks

Builders and buildpacks designed to run on Google Cloud's container platforms
Apache License 2.0
974 stars 144 forks source link

gcloud run deploy fails because npm run build is executed automatically #287

Open mscudlik opened 1 year ago

mscudlik commented 1 year ago

starting today, all deployments to google cloud run using the following command fail:

gcloud run deploy <service-name> --project <project-name> --region <region> --source . [... additonal service params]

Root cause is that gcloud run deply/cloud build is running the npm script "npm build" automatically, which does not work because not all files are available. We have build the application already in our own CI/CD environment and only upload the compiled files. Running "npm build" should not be executed again. This was the behavior until today.

How can we disable this behavior?

We want to deploy our app to cloud run using the source based approach, so only the container image should be build.

Our .gcloudignore looks like this:

/*                  # exclude all, BUT
!dist/              # already compiled code (from typescript) 
!package.json       # package and package-lock.json to fetch the dependencies
!package-lock.json

Seems like this has been introduced with https://github.com/GoogleCloudPlatform/buildpacks/commit/7ce45107de34c675c6d1e9d2da89c5b41c20c035

According to the documentation, only "gcp-build" should be executed automatically: https://cloud.google.com/docs/buildpacks/nodejs

smsohan commented 1 year ago

We're looking at this change and will get back to you shortly.

evilandfox commented 1 year ago

Same problem. Are there any arguments for gcloud run deploy which disable this feature?

jama22 commented 1 year ago

Hey folks, sorry for catching ya'll by surprise. The docs release fell behind the update in the buildpacks.

You should be able to bypass this behavior by setting following environment variable to an empty value e.g. GOOGLE_NODE_RUN_SCRIPTS=

edit:

Forgot to mention that you can use the cloud run flag for setting environment variables or use a project.toml file (buildpacks configs)

matthewrobertson commented 1 year ago

The workaround here is to drop a file named project.toml into the root of your project with the following content:

[[build.env]]
name = "GOOGLE_NODE_RUN_SCRIPTS"
value = ""

Explanation: GOOGLE_NODE_RUN_SCRIPTS takes a comma separated list of npm scripts to run as part of the build. Providing an empty string, will cause no scripts to run.

steveoh commented 1 year ago

https://cloud.google.com/docs/buildpacks/nodejs#executing_custom_build_steps_during_deployment says that it should only run a script called gcp-build. my build failed when trying to run npm build. Even with a procfile with other instructions.

image

my work around was to rename my build script to build:something

evilandfox commented 1 year ago

The workaround here is to drop a file named project.toml into the root of your project with the following content:

[[build.env]]
name = "GOOGLE_NODE_RUN_SCRIPTS"
value = ""

Explanation: GOOGLE_NODE_RUN_SCRIPTS takes a comma separated list of npm scripts to run as part of the build. Providing an empty string, will cause no scripts to run.

It works, thanks

jama22 commented 1 year ago

@steveoh would you be able to share your Procfile? setting the environment variable should work.

mscudlik commented 1 year ago

The workaround here is to drop a file named project.toml into the root of your project with the following content:

[[build.env]]
name = "GOOGLE_NODE_RUN_SCRIPTS"
value = ""

Explanation: GOOGLE_NODE_RUN_SCRIPTS takes a comma separated list of npm scripts to run as part of the build. Providing an empty string, will cause no scripts to run.

Thanks, i can confirm that our deployment is working again after adding the file. From what i understood this is a workaround which will be removed/solved differently in the future, right?

Now i have to know internals from the buildpack in my project even though i just run "gcloud run deploy" for deployment and not building the software.(even though the container is internally build i know)

steveoh commented 1 year ago

@steveoh would you be able to share your Procfile? setting the environment variable should work.

yeah it's super simple

web: node index.js

elbakerino commented 1 year ago

@steveoh the Procfile worked until yesterday for me, now just the Procfile is not enough.

With both, the project.toml and Procfile, it finally deploys again.

Maybe that depends on which "default" npm-scripts are used in package.json?

both scripts are not meant for the server in my case

jama22 commented 1 year ago

From what i understood this is a workaround which will be removed/solved differently in the future, right? ... Now i have to know internals from the buildpack in my project even though i just run "gcloud run deploy" for deployment and not building the software.(even though the container is internally build i know)

One of the goals of the GCP buildpacks project is to be idiomatic to the language family we're supporting while meeting a broad range of use cases and project types. We enabled this behavior to meet the needs of a lot of Node.JS projects who need to run npm run build without having to set up a separate build process before deployment (e.g. Typescript projects, or even just basic SPAs)

Unfortunately, that has the side effect of impacting projects that do invest in CI pipelines where build & config happens prior to containerizing with gcloud run deploy. Our rollout for this change happened out of order (code before docs) and created this confusion, and I apologize for that.

With that context out of the way, I still feel that the build time environment variable is a good guard against this behavior. Using a project.toml is the buildpacks-idiomatic way to set env-var; but off the top I can think of a few ways we might be able to improve that experience

  1. Introduce a --set-build-env-var flag in gcloud run deploy so you don't have to create a project.toml
  2. Make it so that when you set gcp-build: '' in package.json we interpret that as a no-op and ignore everything in build

(1) might be a reasonable improvement to Cloud Run and I can explore that with the product team (2) is also something simple we could introduce to this buildpack, and that kinda jives with how Heroku rolled out their change, although theirs was an opt-in before it was made the default

wdyt @@mscudlik @anniefu ?

steveoh commented 1 year ago

The docs already call out a gcp-build npm script. I don't see why calling build needs to also happen?

rahul-sekhar commented 1 year ago

I just wanted to call out that this is a breaking change that could catch a lot of people unawares. It is blocking our deployment flow and it took me a while to find the root cause (when I compared a successful build to a failed one, I don't even see any difference in buildpack version numbers). Please be more intentional about rolling out breaking changes like this.

mscudlik commented 1 year ago

@jama22: i would appreciate it not to be forced to create a project.toml using an internal environment name "GOOGLE_NODE_RUN_SCRIPTS" (should it be GOOGLE_BUILD_NODE_RUN_SCRIPTS by the way? they are just used for building, not running or anything else) and knowing internals from google cloud build.

And while it might make sense to be able to set environment variables for the build using google cloud build standalone, i'm not so sure if this makes sense using gcloud deploy, as there are other environment variables, i.e. used for the application in cloud run.

So this seems to be a bit weird for me: gcloud run deploy <service-name> --source . --project <project> [...] --set-env-vars="FOO=BAR" --set-build-env-var="GOOGLE_NODE_RUN_SCRIPTS="

From gcloud run deploy perspective, maybe this could be abstracted a bit more with "build options", e.g. --build true or something (knowing there might be multiple options)

JohnGale87 commented 1 year ago

This issue is affecting gcloud app deploy now too. We were deploying our NodeJS app to AppEngine and all of a sudden it just stops working.

cherlin commented 1 year ago

Adding GOOGLE_EXPERIMENTAL_NODEJS_NPM_BUILD_ENABLED as a keyword here, for my fellow humans wondering what kind of bad experiment Google is running and spending half a day on it before ending up in this issue 🤦

FrozenKiwi commented 1 year ago

any update on how to get google app deploy working? My deploys no longer work, even with the project.toml included

cherlin commented 1 year ago

@FrozenKiwi We couldn't get it working with the project.toml either. Adding an empty gcp-build script to the package.json did it though!

FrozenKiwi commented 1 year ago

@cherlin YESSSSS!!!!!!!!!!!! thankyouthankyouthankyou, I was tearing what's left of my hair out.

elyobo commented 1 year ago

It may not matter, but there is some small difference between the empty gcp-build and declaring the GOOGLE_NODE_RUN_SCRIPTS build env var noted over in https://github.com/GoogleCloudPlatform/buildpacks/issues/290. Declaring the env var runs everything with NODE_ENV production; declaring gcp-build still installs dev dependencies, runs the (empty) script, then uninstalls the dev dependencies using npm --prune. Also noted in that issue was a bug that it would then actually launch the app with NODE_ENV development true, which broke things for us, but that has apparently been resolved.

jonolayton commented 1 year ago

@FrozenKiwi We couldn't get it working with the project.toml either. Adding an empty gcp-build script to the package.json did it though!

Relevant docs: https://cloud.google.com/docs/buildpacks/nodejs#using_google_node_run_scripts

samatcolumn commented 1 year ago

This broke us too. We're using Node.js v16 and Google Cloud Functions for Firebase and our firebase.json looks like this:

{
  "functions": {
    "predeploy": [
      "npm --prefix \"$RESOURCE_DIR\" run build"
    ],
    "source": "functions"
  }
}

So when we run firebase deploy it does the npm run build step. Doing it in the buildpack is a waste of time and, more importantly, broke us because certain types were not available in Cloud Build.

We've been deploying this way for years so this is an undocumented breaking change. For now I am going to rename my build script to compile but IMO this should be rolled back.

giact commented 1 year ago

I just ran into this issue when using gcloud functions deploy, which I solved by passing --update-build-env-vars="GOOGLE_NODE_RUN_SCRIPTS="

deriegle commented 1 year ago

Spent way too much time tracking this down. This should've 💯 been marked as a breaking change.

colerogers commented 1 year ago

Hi Firebase dev here! If anyone is using Cloud Functions for Firebase and firebase-deploy (like @samatcolumn), we patched our CLI to disable this feature a few weeks back in firebase-tools v11.27.0. If you upgrade to at least that version, this error should go away without any additional work. Thanks!

aminere commented 1 year ago

It's unfortunate that this breaking change is not adequately documented

Current workaround is to change the build command in package.json to build:local

robinvw1 commented 1 year ago

Oof. 🤯 Just spent 2 hours figuring out why our CI/CD pipeline was failing, and then I finally found this issue. I agree with others about marking this as a breaking change.

Renaming my build command in package.json also fixed the problem for me.

mctrafik commented 1 year ago

+1 to this being incredibly annoying and broken.

For Google Cloud Run and Google Cloud Functions, the solution is to add an empty gcp-build script.

mctrafik commented 1 year ago

@jama22 I spoke to Google support that said your answer should be considered 'official' which is bonkers to me.

Our usecase is that we use an internal registry with private access and all of our build commands require credentials that we will not be sending to Google for security reasons. This means every single team, developer and project requires to try to deploy code to GCP via trial and error until they find the internal logs for buildpack setup and figure out that there's a check for gcp-build like I had to do. It takes days. I just had another developer waste 2 days on this.

The worst part is that this happens when a new version of node is used. So everyone in my company, when they will slowly be upgrading node versions will over the next few years discover this bug and struggle. Everyone will. This change should not have happened. And if it did, it shouldn't be in the way that it did.

I.e. if you want to make this change permanent there need to be dozens of guides and an e-mail to all current GCP nodeJs runtime developers notifying us of the breaking change that is SO DIFFICULT to troubleshoot on prod.

jama22 commented 1 year ago

@mctrafik et al., thanks for sharing your respective situations.

It takes days. I just had another developer waste 2 days on this.

I want to apologize for the frustrations that this change has caused and I appreciate ya'll spending the time to give us this feedback. I wanted to share some of the changes we're making to help with the discovery and debug-ability of this issue:

We've also identified process and communication gaps:

And if it did, it shouldn't be in the way that it did.

I agree, ultimately this was a breaking change to the Node.js buildpacks. We underestimated the potential negative impact of this change and we strive to do better in the future. A few things we're committed to doing:

  1. Get more involvement and feedback from the community on major upcoming changes before they happen. For changes that may have a broad impact on the project, I'll be posting them in the project's Discussions board to get community feedback
  2. Known breaking changes will always have a 1 year notice period before coming into effect. Those will always be communicated via release notes, here on the GitHub project, and also through official Google communication channels (if you're a customer)

The worst part is that this happens when a new version of node is used.

I might be missing some additional details here, but that shouldn't be the case. Node runtime versions are decoupled from the actual buildpack behavior

elyobo commented 1 year ago

Thanks for the response and the additional info @jama22, that generally sounds pretty good.

I'd really like to have a more aggressive push notification in cases like this, e.g. Google manages to send me emails about upcoming breaking changes to lots of services and that "push" means that I see them without having to check in across the seemingly infinite number of services I use for potentially upcoming breaking changes - generally I'll check notes for breaking changes when I'm upgrading, but it's not practical to be polling everything in case there are breaking changes that affect a version I'm already using as was the case here.

On the last note, I think it's because in some cases (e.g. app engine flex, others?) buildpacks only became a thing with node 18, so this behaviour suddenly occurs on upgrade.

elbakerino commented 1 year ago

@jama22 for me it occurred when I've started to switch all NodeJS projects to pure ESM, upgrading some from ~14 -> 18 .

Thus the first debugging stuff was my new build chain - not the damn names of the build scripts.

What got me: it worked everywhere else, other Cloud CIs were working, that's when "oh Google changed something again in buildpacks". Thanks to not only relying on Google Cloud, the debugging was not even hours.

JayGoldberg commented 1 year ago

Please note you can subscribe to Changelog via feed (e.g. see the top of https://cloud.google.com/appengine/docs/standard/nodejs/release-notes for a feed link), or by using Bigquery:

SELECT
  *
FROM
  `bigquery-public-data.google_cloud_release_notes.release_notes`
WHERE TRUE
  AND LOWER(product_name) LIKE '%app engine%'
  AND release_note_type = "BREAKING_CHANGE"
ORDER BY published_at DESC

And set email alerts or webhooks using Apps Script and the builtin Bigquery service.

We've found that customers get "notification fatigue" from receiving emailed notices about changes. I'm not clear if this was a candidate for such a mailing as those are usually reserved for breaking changes that will cause outages, such as runtime changes affecting running code vs this which manifests as a deployment issue.

elyobo commented 1 year ago

Thanks Jay.

I don't think the distinction between "deployment" and "runtime" is necessarily clear, we have applications that are deployed automatically on schedule with updated local databases and so the this was a breaking change to the regular operation of the system.

Email fatigue is a valid concern, allowing opt in or opt out might adequately address it.

The feed is helpful though, thanks.

Pixelatex commented 11 months ago

Super annoyingly, for anyone else using github actions to deploy... that devs on that team don't allow any empty values to be passed. So the only workaround in that case is using the empty gcp-build script.

I mentioned this thread in an open PR that's trying to resolve this issue for anyone interested.

ephilippo commented 11 months ago

In my App Engine project, adding a project.toml did not populate any environment variables.

Instead, I was able to add the variable to my app.yaml as noted here:

build_env_variables:
  GOOGLE_NODE_RUN_SCRIPTS: ''

Yet another workaround for those (like me) trawling this thread for solutions.