jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.58k stars 4.02k forks source link

Executable JAR is not recommended, not supported by buildpack #10255

Closed saturnism closed 5 years ago

saturnism commented 5 years ago
Overview of the issue

Currently, JHipster uses Spring Boot Maven plugin to build a self executable jar (i.e., Zip file w/ a self-executable expansion). This is done in the pom.xml

pom.xml w/ Spring Boot Maven Plugin

<plugin>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-maven-plugin</artifactId>
    <configuration>
        <mainClass>${start-class}</mainClass>
        <executable>true</executable>
        </configuration>
...

The JAR can be executed as if it's a binary (e.g., $ target/myapp.jar in addition to $ java -jar target/myapp.jar). However, this self expansion is not supported by all tools, including Cloud Native Buildpack (https://github.com/cloudfoundry/archive-expanding-cnb/issues/2).

Recommend setting this flag to false by default.

Motivation for or Use Case

This prevents Cloud Native Buildpack to work w/ JHipster

Reproduce the error

Use cnb pack to pack JHipster source to container. It'll fail with:

build-step-build] Cloud Foundry Archive Expanding Buildpack 1.0.0-BUILD-SNAPSHOT
[build-step-build]     Expanding /workspace/demo-0.0.1-SNAPSHOT.jar to /workspace
[build-step-build] zip: not a valid zip file
[build-step-build] Error: failed to build: exit status 103
[build-step-build] Build Failed
Related issues

https://github.com/cloudfoundry/archive-expanding-cnb/issues/2

Suggest a Fix

Don't produce executable JAR. Set executable to false

JHipster Version(s)

6.2.0

JHipster configuration
Entity configuration(s) entityName.json files generated in the .jhipster directory
Browsers and Operating System
saturnism commented 5 years ago

/cc @jdubois @PierreBesson @deepu105 wdyt?

PierreBesson commented 5 years ago

I guess this would be acceptable for most people. Is it possible to generate the startup script besides the jar (by that I mean the startup script that is prepended to the jar) ? It might be useful for some people.

PierreBesson commented 5 years ago

So guess the embeddableLaunchScript can only be generated inside of the jar. Still I think it's an ok change to do as. People that want to deploy the app as a system service can easily put the property back to true.

However I would like to set a property in .yo-rc.json called embeddableLaunchScript and set it to true for apps generated by older version of JHipster and false for recent versions. @SudharakaP do you see what I mean ? Can you implement it ?

SudharakaP commented 5 years ago

@PierreBesson : Sure I am willing to implement it, but sorry, not sure I get exactly what you mean. πŸ˜• You mean put a property in yo-rc.json and depending on that set the <executable> field in pom.xml?

PierreBesson commented 5 years ago

Yes put an if in the template depending on a yo-rc property. Then in the jhipster init method somewhere we have a method that patch the yo-rc if some props are undefined, you should add executable = true there.

PierreBesson commented 5 years ago

Just found out this cool project to create executable jars as a separare goal : https ://github.com/salesforce/grpc-java-contrib/blob/master/canteen/README.md

SudharakaP commented 5 years ago

Yes put an if in the template depending on a yo-rc property. Then in the jhipster init method somewhere we have a method that patch the yo-rc if some props are undefined, you should add executable = true there.

Wonderful, thanks. Got it. πŸ‘

Just found out this cool project to create executable jars as a separare goal : https ://github.com/salesforce/grpc-java-contrib/blob/master/canteen/README.md

Interesting project; Shall we add this as well. I suppose we can just add this and remove the <executable> part once and for all; correct?

PierreBesson commented 5 years ago

Let's not rush things. I have yet to test this to see if this is better than what spring provides.

SudharakaP commented 5 years ago

Let's not rush things. I have yet to test this to see if this is better than what spring provides.

@PierreBesson : Sure. πŸ˜„ I'll make the change as we discussed previously for now.

SudharakaP commented 5 years ago

@PierreBesson @saturnism : I've made the change; let me know if you see any problems or issues. πŸ˜„

saturnism commented 5 years ago

thanks! btw, does this affect the gradle build?

SudharakaP commented 5 years ago

@saturnism : We use bootJar task for creating the jar on gradle side; but we haven't enabled launchScript. So the JAR created by gradle isn't fully executable. So the answer is no; we don't have to do anything to the gradle side. By default it is not a fully executable jar.

EDIT: @PierreBesson : However it got me thinking that perhaps we might want to add support for fully executable jars on the gradle side as well?

atomfrede commented 5 years ago

It is easy to add. If you like I can do it after this is merged, but it is just adding

bootJar {
    launchScript()
}

to the bootJar task.

https://docs.spring.io/spring-boot/docs/current/gradle-plugin/reference/html/#packaging-executable-configuring-launch-script

SudharakaP commented 5 years ago

@atomfrede : Yes, I just wanted to verify with Pierre before doing that since I wasn't sure if it's needed. Anyways, feel free to add that (or I can do that too if you prefer).

saturnism commented 5 years ago

i vote for consistency btwn maven and gradle builds

SudharakaP commented 5 years ago

@saturnism @atomfrede : I've pushed the Gradle stuff as well. :smile:

PierreBesson commented 5 years ago

@SudharakaP I have opened another MR to introduce the backward compatibility I was talking about : https://github.com/jhipster/generator-jhipster/pull/10324

SudharakaP commented 5 years ago

@SudharakaP I have opened another MR to introduce the backward compatibility I was talking about : #10324

@PierreBesson : Wonderful. Thanks. I think we need to put this in our documentation; otherwise I think a user will have a hard time guessing what embeddableLaunchScript means. I am trying to think of the most appropriate place to put this; any suggestions?

saturnism commented 5 years ago

thank you for the pr!

PierreBesson commented 5 years ago

BTW @saturnism how do we use Cloud Native Buildpack with JHipster. Do we have to create our own builder ? The default builder fails for me as it is not correctly setting the maven prod profile (following: https://buildpacks.io/docs/app-journey/).

saturnism commented 5 years ago

it's an open question for me too. https://buildpacks.io/docs/using-pack/building-app/ can specify env vars, and we can potentially add env var activation for the profile.

but adding @nebhale to see if there is a better way :D

jkutner commented 5 years ago

@PierreBesson @saturnism this should work:

$ pack build jhipster-app --builder heroku/buildpacks:18 --buildpack heroku/nodejs,heroku/java -e MAVEN_CUSTOM_OPTS="-Pprod,heroku -DskipTests"

But that creates an app on the Heroku stack (Ubuntu 18.04 plus some packages). That might not be what everyone wants.

Creating a jhipster builder image would allow users to have more choice over the os/stack. But it would probably also mean creating a jhipster buildpack because the Heroku buildpack expects the Heroku stack (and really commands like curl, tar, etc).

PierreBesson commented 5 years ago

@jkutner is it hard to maintain our own buildpack ? Personally I like the approach of "source-to-image" that buildpacks provide. Maybe we can open a new issue about this.

jkutner commented 5 years ago

I don't think we want to maintain a buildpack for JHipster. We should probably give options, which right now are:

CF support two stacks (i forget what they are though). Eventually, there may be more options from other vendors.

Then Jhispter and set up the right command options for the chosen builder/stack.

saturnism commented 5 years ago

@jkutner do you know what pack uses by default? I thiiiink I found BP_BUILD_ARGUMENTS https://github.com/cloudfoundry/build-system-cnb

saturnism commented 5 years ago

ah, it was only done 5 days ago, by @nebhale ;) https://github.com/cloudfoundry/build-system-cnb/issues/11

jkutner commented 5 years ago

@saturnism that's only the default because you must have chosen the CF buildpack as the default at some point. pack is agnostic between CF/Heroku