-
Notifications
You must be signed in to change notification settings - Fork 1.4k
docs: Documentation artifactory settings fix #7999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Documentation artifactory settings fix #7999
Conversation
|
I guess my personal preference would be for that comment to somehow express that it's recommended that you use any of the supported repositories (Central or custom Artifactory/Nexus). If you run Artifactory in proxy mode, it should be safe to disable the Central analyzer. |
|
I have a related PR at #8000 for Nexus - I could cherry-pick this commit over there and make some edits there - or you could send PRs to that branch if you want to collaborate somewhere? I agree it'd be good if
|
chadlwilson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally we probably need to sync the documentation across the other docs for ant, and maven/gradle as well, but this is better than current, so no objections.
|
@marcelstoer The issue is that when using --enableArtifactory, it is mandatory to also use the --disableCentral parameter. Otherwise, ODC will still attempt to connect to Maven Central, and Artifactory will not work. Artifactory does not function as an addition to Central but only as a complete replacement for it. Therefore, in my comment, I wrote that it must(!) be disabled, as otherwise it simply won’t work. In your comment, however, it is only suggested that this is safe and won’t disrupt functionality. Regarding the note "It is recommended to use any of the repositories (Central or custom Artifactory/Nexus)," I completely agree with you, but I’m not sure where this important addition could be placed. Under the --disableCentral parameter? Perhaps in a comment, you could provide a sample text of how you personally envision this? |
|
@chadjvw If possible, I would separate all these types of analyzers into a distinct category or somehow mark them among the other parameters. Then, it would be possible to add a header to them stating that it is recommended to use any of the supported repositories (Central or custom Artifactory/Nexus). Additionally, the table of contents could immediately include information for all analyzers. |
Ok, I wasn't aware of this constraint. Could you point to reference to back this up? We have been running ODC with |
|
@marcelstoer i think it’s probably just the ordering/preference of the analyzers, and so if you have both enabled it tries central first and then fails/gets stuck before getting to artifactory or nexus. Line 13 in c566904
If it succeeds on central the POM would already be there and the artifactory search would be a no-op. |
|
@marcelstoer @chadlwilson Yes, it’s likely triggering this way due to the priority of the analyzers. I’m currently testing on my project. Right now, Maven Central is returning a 502 error and isn’t working. Meanwhile, ODC is getting stuck hard at the [INFO] Finished Jar Analyzer (0 seconds) stage. So, I have to interrupt the execution and read the logs. with params: --enableArtifactory --disableCentral (The scanner completes the operation successfully.) Without the --disableCentral parameter, my work is paralyzed. |
|
I don't think that's a "constraint" though. It seems.pretty consistent and intuitive that if an analyzer is stuck then your whole analysis will be stuck and you need to disable it to use an alternative approach. Anyway, the next version will disable Central automatically on repeated failures. |
|
@chadlwilson Then, all the more reason to describe it more clearly in the documentation: when enableArtifactory is used, it adds it with a priority of 2, while, for example, enableNexus adds it with a priority of 3, and all these parameters can work together. Upon first reading, I thought that when one is enabled, the others are disabled. Yes, it would be a good solution if the analyzer could switch to the next option if the first one is unavailable. In that case, it turns out that disableCentral is not mandatory (only in the current situation with Maven Central as a workaround). |
|
Sure. You know how it is with open source though - this is one of the most heavily used security tooling projects in the world by large companies, governments, and others…. yet extremely few contributors around 1 primary maintainer (who has a full time job elsewhere) - and infinitely more complaints (often from folks who hardly use GitHub) than solutions. especially low interest when it comes to developer distaste for writing and refactoring docs! :) https://github.com/dependency-check/DependencyCheck/graphs/contributors So - many thanks for creating a couple of PRs! |
|
@chadlwilson Then I think I need to change the comment from "To use Artifactory, you will need to disable the central analyzer by adding the --disableCentral parameter." To "If you want to use only(!) Artifactory without Central, you should disable it with the --disableCentral parameter." |
jeremylong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of Change
Extended description of CLI parameters
Related issues
#7988
#7991 (comment)
Have test cases been added to cover the new functionality?
no