eclipse-ee4j / starter

Eclipse Starter for Jakarta EE
Eclipse Public License 2.0
49 stars 40 forks source link

Enforce More Rigorous Parameter Validation #266

Closed scottkurz closed 1 year ago

scottkurz commented 1 year ago

This is a minor point in and of itself though maybe more interesting to anyone who's thought more about organizing the validation of the various options.

In the groovy script we use this logic to decide whether to delete the Dockerfile

private generateDocker(docker, runtime, File outputDirectory) {
    if (docker.equalsIgnoreCase("no")) {
        println "Docker support was not requested"
        FileUtils.forceDelete(new File(outputDirectory, "Dockerfile"))

In the README archetype template we use this logic to decide whether to add the docker build/run instructions:

#if ((${docker} == 'yes') and (${runtime} != 'glassfish'))

So for someone setting, say, -Ddocker=y (as opposed to 'yes') they'll get the Dockerfile but not the README instructions.

Probably most people that are running archetype:generate and would want to use theDocker image are going to know how to build & run it it, so not a huge impact, but still ... possibly worth considering if there's a need and an approach to be more consistent here.

m-reza-rahman commented 1 year ago

The docker flag should only accept “yes” or “no” as input. There is input validation in place in the Archetype: https://github.com/eclipse-ee4j/starter/blob/92968757eeec3f9e6278f47f7166746098a7730f/archetype/src/main/resources/META-INF/maven/archetype-metadata.xml#L35 Any other input should either be ignored or fail validation. What behavior are you seeing?

m-reza-rahman commented 1 year ago

I see the problem. Essentially the validation is only applied interactively and currently can be side-stepped. Weird that Archetypes even allow this. Regardless, I'll fix this by enforcing strict parameter validation in the Groovy script. I'll do this ASAP. The Archetype and UI need some minor work anyway.

scottkurz commented 1 year ago

I see the problem. Essentially the validation is only applied interactively and currently can be side-stepped.

Hadn't appreciated that point either, but yes, exactly, I was doing: mvn archetype:generate ... -Ddocker=y .....

Thanks for looking into this.

m-reza-rahman commented 1 year ago

Fixed by: https://github.com/eclipse-ee4j/starter/pull/269