GoogleContainerTools / jib

🏗 Build container images for your Java applications.
Apache License 2.0
13.66k stars 1.44k forks source link

Build succeeds with invalid JVM flags #192

Closed TadCordle closed 6 years ago

TadCordle commented 6 years ago

When the user configures a bad JVM flag, e.g.

<configuration>
    ...
    <jvmFlags>
        <jvmFlag>-Xaaaaaaaaaaaaaaaaaaaaaaa</jvmFlag>
    </jvmFlags>
</configuration>

jib:build succeeds, and the problem doesn't show itself until the user tries to run the built image. It would probably be best for problems like this to be caught as early as possible.

briandealwis commented 6 years ago

How could we validate the arguments? Users may choose to use a different JVM like OpenJ9 from AdoptOpenJDK's docker images which supports other arguments.

coollog commented 6 years ago

This issue might not be fixable. What I could think of is to just give a warning for non-standard JVM flags, but then that is cluttering the output with warnings if the user intended to use those flags.

pilhuhn commented 6 years ago

@coollog What is non-standard? There are hundreds of options - especially in the -XX: area, which are even different depending on JVM version and vendor.

briandealwis commented 6 years ago

I wonder if this would be better handled with behavioural testing of the running container with something like goss/serverspec.

coollog commented 6 years ago

@pilhuhn Yea, standard as in what shows up for java -help vs. non-standard as in what shows up for java -X and others like prefixed with-XX: or -D. For "standard" options, there is only a select few like -javaagent and -verbose:..., so Jib could give a warning if using unrecognized flags not prefixed with -X or -D.

@briandealwis Yea, this might just be something the developer would handle themselves by writing their own tests that run their jib-built containers.

patflynn commented 6 years ago

I'd definitely hesitate to provide more strongly typed and validated behavior around jvm flags.

I tend to agree with @coollog that jib shouldn't try to solve this problem.

TadCordle commented 6 years ago

Yeah, this seems like a pretty old issue that doesn't actually need to be fixed. I think we should just close this.

coollog commented 6 years ago

Closed until someone has a strong opinion to have Jib support this.