Gradle plugin configuration

[Vít Šesták]

Hello,
I am trying to modify the Gradle ODC plugin in order to meet our needs. These modifications are very general and it seems to be a good idea to create a pull request for them. I just want to add some configuration options. (Those options are available for Maven, but not for Gradle.)

In order to create a good pull request, I'd like to ask some questions.

1. Some default values are duplicated in src/main/groovy/com/tools/security/extension/DependencyCheckConfigurationExtension.groovy. This is suboptimal unless you are overriding something (e.g. outputDirectory). When I started writing the modifications, I used a null-based approach: When there is a null value, I suppose that the user wants the default one. This eliminates duplicated default values, but it introduces some edge cases (e.g. user can't read the default value in the config file). Another approach would be reading the default values through Settings (from dependencycheck.properties file), but this is not so easy, as Settings is thread-local.
a. Duplicated default values are OK.
b. Null approach is OK.
c. We should read the default values from Settings. (I think that this approach is the most clear one.)

2. Copying from DependencyCheckConfigurationExtension to Settings is rather complex and would be even more complex if we add either null handling or default values fetch. This would be OK for few properties, but for more properties, it might be better to do it declaratively, e.g. def settingsKeyMapping = [[Settings.KEYS.PROXY_SERVER, "proxyServer"], [Settings.KEYS.PROXY_PORT, "proxyPort"], …]. Maybe we would need to write some manual code for few of them.

3. Depth of configuration. Not sure how to describe it, but following example will probably clarify what I mean:
current state:
dependencyCheck{
    proxyServer = "127.0.0.1"
    proxyPort = 1234
}

suggested state:
dependencyCheck{
    proxy{
        server = "127.0.0.1"
        port = 1234
    }
}

Which one looks better for you?

We can preserve backward compatibility by writing some legacy getters/setters.

Regards,
Vít Šesták 'v6ak'

[Jeremy Long]

Vit,

I haven't spent much time working on the Gradle plugin (yet) and I we would definitely appreciate any enhancements. To answer your questions:

1) Regarding the settings I think the null approach is the best option. If you look at all of the other interfaces (Maven, Ant, CLI) you will see that DC reads the configuration file (via Settings.initialize()) and then changes any non-default settings (see populateSettings()).

2) I can't comment on 2 because I haven't dug enough into the Gradle documentation to understand all of the nuances of a gradle plugin - maybe Wei Ma could comment?

3) As to the depth of configuration - again I haven't really been using Gradle as Maven still fits my needs. Is it standard for gradle plugins to nest/group configuration options like this?

--Jeremy

--
You received this message because you are subscribed to the Google Groups "Dependency Check" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dependency-che...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Vít Šesták]

Hello,
after some time, I'd like to start working on that.

1. OK
2. I'd like to know more about that. The code has been slightly changed. What is not good, the duplicity actually became a triplicity, see https://github.com/jeremylong/DependencyCheck/commit/94ae6e76f15976de2ba50ed03e031639942eff95 . Is there a reason for such design?
3. I feel it is rather common.

Regards,
Vít Šesták 'v6ak'

On Tuesday, August 4, 2015 at 12:41:34 PM UTC+2, Jeremy Long wrote:

Vit,

I haven't spent much time working on the Gradle plugin (yet) and I we would definitely appreciate any enhancements. To answer your questions:

1) Regarding the settings I think the null approach is the best option. If you look at all of the other interfaces (Maven, Ant, CLI) you will see that DC reads the configuration file (via Settings.initialize()) and then changes any non-default settings (see populateSettings()).

2) I can't comment on 2 because I haven't dug enough into the Gradle documentation to understand all of the nuances of a gradle plugin - maybe Wei Ma could comment?

3) As to the depth of configuration - again I haven't really been using Gradle as Maven still fits my needs. Is it standard for gradle plugins to nest/group configuration options like this?

--Jeremy

[Vít Šesták]

I've written some mapping. Repeating the default settings on several places is no longer needed after my patch. Adding a new property now requires just adding a line to DependencyCheckConfigurationExtension and (e.g. just String nexusAnalyzerUrl) and adding a mapping line to DependencyCheckTask (e.g. [Settings.KEYS.ANALYZER_NEXUS_URL, "nexusAnalyzerUrl"],). If there is null value, it is not copied to the settings (and thus left default).

I have added some configuration properties (because I needed them…).

I haven't used nesting, because I needed to get it working rather than to get it perfect at first attempt. I can add it. Maybe I'll move the mapping to the DependencyCheckConfigurationExtension itself (and to classes related to nested configuration) in order to make it simpler and more easy to maintain.

While I have added suppressionFile to the config, it does not seem to have any effect. According to my logging, it seems to be passed to the Settings class, but it does not have any effect. While it is not a big issue for me at the moment, I'll probably investigate it later.

I have seen that the Groovy plugin includes even test dependencies. I'll probably fix it.

I don't want to have a separate fork of the plugin. I'd rather like to contribute. But I'd like to have an idea if my contribution goes in a desired direction. This is what I am not sure about. I am mostly unsure at the config: It looked crazy for me (as it required adding a new property on multiple places and adding default values to two places), so I have rewritten it, but there might have been a good reason for doing so.

Regards,
Vít Šesták 'v6ak'

On Tuesday, August 11, 2015 at 4:57:46 PM UTC+2, Vít Šesták wrote:

Hello,
after some time, I'd like to start working on that.

1. OK
2. I'd like to know more about that. The code has been slightly changed. What is not good, the duplicity actually became a triplicity, see https://github.com/jeremylong/DependencyCheck/commit/94ae6e76f15976de2ba50ed03e031639942eff95 . Is there a reason for such design?
3. I feel it is rather common.

Regards,
Vít Šesták 'v6ak'


On Tuesday, August 4, 2015 at 12:41:34 PM UTC+2, Jeremy Long wrote:

Vit,

I haven't spent much time working on the Gradle plugin (yet) and I we would definitely appreciate any enhancements. To answer your questions:

1) Regarding the settings I think the null approach is the best option. If you look at all of the other interfaces (Maven, Ant, CLI) you will see that DC reads the configuration file (via Settings.initialize()) and then changes any non-default settings (see populateSettings()).

2) I can't comment on 2 because I haven't dug enough into the Gradle documentation to understand all of the nuances of a gradle plugin - maybe Wei Ma could comment?

3) As to the depth of configuration - again I haven't really been using Gradle as Maven still fits my needs. Is it standard for gradle plugins to nest/group configuration options like this?

--Jeremy

[Vít Šesták]

So, suppressionFile seems to be my fault. I configured it just for the root project, not for subprojects.

Test dependencies seems not to be so easy to exclude in Gradle :( I could probably exclude some configurations, but I am not sure what is appropriate to exclude/include.

Regards,
Vít Šesták 'v6ak'

[Robert Wenner]

Vit,

great job with the plugin. I gave it a quick test last week and it
integrates quite nicely!
Thanks!!!

Robert

[Vít Šesták]

Robert,
I am not the author of the plugin. I've made some improvements, but these are not merged now. If you find them awesome, the credits don't belong to me. At least not at the moment :)
Regards,
Vít Šesták 'v6ak'