cloud-run-button icon indicating copy to clipboard operation
cloud-run-button copied to clipboard

Both Cloud Run Button and Heroku Button - app.json Clash

Open SudharakaP opened this issue 5 years ago • 28 comments

I am trying to add the Cloud Run Button to one of our repositories which has the Heroku Deploy Button which also uses an app.json file.

This creates problems when trying to add the Cloud Run button to the same repository as the Cloud Run also assumes an app.json file and the Heroku app.json file which is different in format will crash the Cloud Run when trying to deploy. Also I am unable to define an app.json file for Cloud Run since one already exists for Heroku.

Is there a possibility to configure the name of the app.json file (or just name it differently) for example so that it doesn't clash with other cloud providers such as Heroku? :smile:

SudharakaP avatar Nov 25 '19 06:11 SudharakaP

If you can mention the error, we can help it not clash (by perhaps introducing a placeholder for fields that aren't supported).

ahmetb avatar Nov 25 '19 19:11 ahmetb

Yeah, our app.json parsing is strict so likely we just need more ignore flags. I think this is the file that is failing: https://github.com/SudharakaP/jhipster-registry/blob/add-gcp-cloud-run-deploy-button/app.json

I can investigate.

jamesward avatar Nov 25 '19 22:11 jamesward

@ahmetb : Thanks for the quick reply. The first error is;

Error: error attempting to read the app.json from the cloned repository: failed to parse app.json file: failed to parse app.json: json: cannot unmarshal string into Go struct field env.required of type bool

Now this is caused by the following line of app.yaml. When you remove the quotes around true it passes through that but gives the following error;

Error: error attempting to read the app.json from the cloned repository: failed to parse app.json file: failed to parse app.json: json: unknown field "buildpacks"

due to the buildpacks argument in the app.yaml file

Hope this helps. Let me know if you need any further details. 😄

SudharakaP avatar Nov 25 '19 22:11 SudharakaP

We can try and fix buildpacks argument, but I assume this is not a good idea. Because that value is critical to how the app is built. And we better not ignore it.

About required being a string in Heroku ecosystem –that's really sad, and I am not sure if we can fix it easily. We'd need to write a custom json parser and hook that into the normal json parser. (This is not trivial, kubernetes has IntOrBool type for this, maybe we can learn from it).

A cleaner solution I can think of is to introduce a new file name that's used during the lookup stage. For example, we'd suggest people to still use app.json, but if there's a cloudrun.json, we'd use that first.

Maybe this hack would be just for repos like yours, and we would not make it as prominent as app.json in the documentation.

ahmetb avatar Nov 25 '19 22:11 ahmetb

Officially, required is a boolean in app.json https://devcenter.heroku.com/articles/app-json-schema#env

Our parsing should be able to handle that.

For the buildpacks param, I think it'd be ok to ignore it for now, with a warning. And in cases where the auto-detection / defaults work, people can then use the button. E.g. if the button doesn't work because of our lack of support, then people should add the button to their repo.

jamesward avatar Nov 25 '19 22:11 jamesward

So required should be a boolean everywhere, in that case we shouldn't try to handle string for sure.

@SudharakaP are you able to deploy and run after deleting buildpacks field, and making required field a boolean as spec says?

ahmetb avatar Nov 25 '19 23:11 ahmetb

I'm guessing is actually not going to work because it is using a non-default buildpack. Maybe @jkutner can let us know if the https://github.com/jhipster/jhipster-registry-buildpack buildpack will work with the CNB heroku/buildpacks image.

jamesward avatar Nov 25 '19 23:11 jamesward

@ahmetb : After changing required to boolean and deleting the buildpacks field and trying to run; I get the following error;

sudharakapalamakumbura@cloudshell:~$ cloudshell_open --repo_url "https://github.com/SudharakaP/jhipster-registry.git" --git_branch "add-gcp-cloud-run-deploy-button"
[ ✓ ] Cloned git repository https://github.com/SudharakaP/jhipster-registry.git.
[ ? ] Value of JHIPSTER_PASSWORD environment variable (Admin password for the registry (used to login after clicking 'View App'). Must be at least 5 characters.) jhipster
[ ? ] Value of JAVA_OPTS environment variable (Java runtime options.) -Xmx300m -Xss512k -XX:CICompilerCount=2 -XX:ReservedCodeCacheSize=50m -XX:MaxMetaspaceSize=80m -XX:ParallelGCThreads=3 -Dfile.encoding=UTF-8
[ ? ] Value of SPRING_OPTS environment variable (Spring Boot options.) --server.undertow.io-threads=1 --server.undertow.eager-filter-init=false
[ ? ] Value of JHIPSTER_REGISTRY_VERSION environment variable (Version of the registry to deploy.) 5.0.2
[ ✓ ] Queried list of your GCP projects
[ ? ] Would you like to use existing GCP project jhipstergaemicro to deploy this app? Yes
[ ✓ ] Enabled Cloud Run API on project jhipstergaemicro.
[ ? ] Choose a region to deploy this application: us-central1
[ ! ] Attempting to build this application with its Dockerfile...
[ ! ] FYI, running the following command:
        docker build -t gcr.io/jhipstergaemicro/JHipster Registry .
[ ✖ ] Failed to build container image.
Error: this application doesn't have a Dockerfile; attempted to build it via heroku/buildpacks and failed: docker build failed: exit status 125, output:
invalid argument "gcr.io/jhipstergaemicro/JHipster Registry" for "-t, --tag" flag: invalid reference format: repository name must be lowercase
See 'docker build --help'.

sudharakapalamakumbura@cloudshell:~$

SudharakaP avatar Nov 26 '19 01:11 SudharakaP

This is because you need the custom buildpack to build this project.

jamesward avatar Nov 26 '19 01:11 jamesward

@jamesward : So if I understand correctly, app.json in cloud run currently doesn't support custom buildpacks? Trying to understand if there's a way we could get around this one. :thinking:

SudharakaP avatar Nov 26 '19 01:11 SudharakaP

The string JHipster comes from app.json > name field (Or repo name), no?

ahmetb avatar Nov 26 '19 02:11 ahmetb

@SudharakaP Correct, Cloud Run Button doesn't support custom buildpacks defined by app.json yet. @ahmetb The name is unrelated. There is a custom buildpack defined that is needed to build the jhipster stuff.

I think a workaround for this would be to use a Dockerfile for the build. But we might still run into app.json parsing issues, which we should fix.

jamesward avatar Nov 26 '19 03:11 jamesward

@ahmetb : Actually I think you are right; I changed the name field to all lowercase and also changed the PORT to point to the PORT environment variable and this makes it deploy successfully. However it prints out he following error in the log when trying to authenticate.

2019-11-25 21:16:52.914 PST Container Sandbox Limitation: Unsupported syscall setsockopt(0x24,0x1,0xa,0x3e88bb822150,0x4,0x0). Please, refer to https://gvisor.dev/c/linux/amd64/setsockopt for more information.

2019-11-25 21:16:52.927 PST POST 403 434 B16 msChrome 78 https://jhipster-registry-gv3mz4r7ta-uc.a.run.app/api/authenticate

I think this is because of a limitation of the container sandbox. I am trying to see if I could do the same thing on Anthos (where there's no container sandbox). Is the Cloud Run button supports deploying on Anthos as well?

@jamesward : Thanks much for confirming; I am new to Cloud Run and learning as I go along. :smile:

SudharakaP avatar Nov 26 '19 05:11 SudharakaP

The “syscall not supported” log is actually an info/warning. It does not mean your app is failing. Is your app actually throwing an exception? If not, it means it’s probably working fine.

ahmetb avatar Nov 26 '19 08:11 ahmetb

@ahmetb : Thanks for the information. I have to investigate more; since the authentication seems not to work; probably due to the removal of buildpacks. Anyways to summarize; it would be helpful if;

  1. The buildpack argument is supported (https://github.com/GoogleCloudPlatform/cloud-run-button/issues/3)

  2. Maybe have a cloudrun.json as suggested above so that we could have two different configurations for Heroku and GCP.

  3. Not a huge problem, but I also have a third issue I noticed; when I click on the Cloud Run button it connects to cloud shell and executes the command,

 cloudshell_open --repo_url "https://github.com/SudharakaP/jhipster-registry.git" --page "editor" --git_branch "add-gcp-cloud-run-deploy-button"

This fails with Error: flag provided but not defined: -page. So normally each time I have to rerun the command without the -page flag;

cloudshell_open --repo_url "https://github.com/SudharakaP/jhipster-registry.git" --git_branch "add-gcp-cloud-run-deploy-button"

SudharakaP avatar Nov 26 '19 14:11 SudharakaP

https://github.com/jhipster/jhipster-registry-buildpack is not a Cloud Native Buildpack, and Heroku's app.json only supports v2 (non-CNB) buildpacks right now.

jkutner avatar Nov 26 '19 18:11 jkutner

@jkutner Thanks!

There is an existing bug about invalid container image names based on app.json: https://github.com/GoogleCloudPlatform/cloud-run-button/issues/57

jamesward avatar Nov 26 '19 20:11 jamesward

Oh, and the error about the page param is something we are fixing right now: #114

jamesward avatar Nov 27 '19 23:11 jamesward

I think we fix this tactically by adding CNB support to https://github.com/jhipster/jhipster-registry-buildpack. I've created https://github.com/jhipster/jhipster-registry-buildpack/issues/3 and will try to get a PR today.

jkutner avatar Dec 06 '19 17:12 jkutner

Thanks @jkutner! So, in the CNB world, what is the right way to handle custom specified buildpacks?

jamesward avatar Dec 07 '19 08:12 jamesward

@jamesward it will depend on the platform, but there's no specification yet (we're working on one).

Today pack allows the --buildpack option, but it's up to each platform to do it's own thing for now. I think reading app.json buildpacks is ok, but you will run into cases where it contains a v2 buildpack.

jkutner avatar Dec 07 '19 16:12 jkutner

There's one other problem I have; it seems that from my testing that for Cloud Run we need to change the docker files EXPOSE value to ${PORT} since GCP seems to use this environment variable (https://cloud.google.com/run/docs/reference/container-contract). This is also a problem when you have both the Heroku button and the Google Cloud Run button since Heroku doesn't have a problem with a hard-coded port value. Is there some workaround to keep the PORT value the same in the docker file and still be able to say forward that to ${PORT} ? :thinking:

SudharakaP avatar Dec 07 '19 17:12 SudharakaP

Thanks @jkutner, doesn't the --buildpack option just override the detect phase and need to be a buildpack location in the builder image or a tar/tgz? In which case we won't be able to go from the usual app.json buildpack settings to something that works with pack.

@SudharakaP With both Cloud Run and Heroku the app just needs to listen on $PORT and it doesn't matter what is in the Dockerfile's EXPOSE statement.

jamesward avatar Dec 07 '19 17:12 jamesward

@jamesward yes, the buildpack either needs to be in the builder or provided to pack as a tgz. But there's no mechanism (yet) for pack to automatically turn a Github URL into a tgz, so it would be up to the platform (i.e. Cloud Run) to convert that Github URL into something pack can use (probably by downloading it, maybe with an API key to prevent 429s, and stripping the root dir).

We've considered adding this as a first class thing in pack: https://github.com/buildpack/rfcs/pull/26

But we're waiting to implement a Buildpack Registry first.

jkutner avatar Dec 09 '19 15:12 jkutner

I have the same problem and the workaround for me was creating an exclusive branch for Google Buton de deploy. you can see my solution here

In my case, in the app.json I have to declare "stack": "container", for Heroku, and this tag is not supported for google cloud run.

my app.json file

{
  "name": "Matrix",
  "description": "Matrix produces a virtual office for remote teams. In this project, you can run a virtual office to simulate the physical environment",
  "repository": "https://github.com/ResultadosDigitais/matrix",
  "keywords": ["online workspace", "virtual office", "remote teams"],
  "stack": "container",
  "env": {
    "GOOGLE_CLIENT_ID": {
      "description": "Google Client Id to the OAuth authentication",
      "value": "xxxxxxx.apps.googleusercontent.com"
    },
    "GOOGLE_SECRET": {
      "description": "Google Secret to the OAuth authentication",
      "value": "gXXXXXXXXXXXXXXss"
    },
    "GOOGLE_CALLBACK_URL": {
      "description": "The callback Url to the OAuth authentication",
      "value": "http://localhost:8080/auth/google/callback"
    },
    "COOKIE_SESSION_SECRET": {
      "description": "The secret of session",
      "value": "my-matrix-session"
    },
    "COOKIE_SESSION_MAX_AGE": {
      "description": "The Session duration",
      "value": "2592000000"
    },
    "WHITELIST_DOMAINS": {
      "description": "The list of valid email domains to enter in matrix",
      "value": "[]"
    },
    "ENFORCE_SSL": {
      "description": "If you running in HTTPS this variable forces redirect to HTTPS when user access with HTTP",
      "value": "true"
    }
  }
}

Here is possible to see the error.

image

my suggestions

  1. Ignore not supported tags
  2. Like my workaround where I defining a specific branch, maybe we can declare a specific *.json file

juliemar avatar Mar 22 '20 15:03 juliemar

I think our current plan is to add ignored fields as they arise. I'll create a PR for that.

jamesward avatar Mar 23 '20 16:03 jamesward

Hi, I think I am facing a similar issue. I tried deleting some of the fields, but still I am getting failed to parse app.json: json: unknown field "formation" This is my repository: https://github.com/SpEcHiDe/PublicLeech/blob/master/app.json

SpEcHiDe avatar Jul 25 '20 14:07 SpEcHiDe

@SpEcHiDe Thanks for reporting! Here is the PR: https://github.com/GoogleCloudPlatform/cloud-run-button/pull/193

jamesward avatar Jul 27 '20 15:07 jamesward