Skip to content
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

feat: Allow to pass NVD API key via environment variable #6454

Merged

Conversation

kwin
Copy link
Contributor

@kwin kwin commented Feb 8, 2024

Clarify precedence of API key plugin parameters in javadoc This closes #6443

Fixes Issue #6443

Have test cases been added to cover the new functionality?

no

@boring-cyborg boring-cyborg bot added the maven changes to the maven plugin label Feb 8, 2024
@kwin kwin changed the title Allow to pass NVD API key via environment variable feat: Allow to pass NVD API key via environment variable Feb 8, 2024
@jeremylong
Copy link
Owner

It would likely be good to include a default env variable name - and if it exists use it.

@kwin
Copy link
Contributor Author

kwin commented Feb 24, 2024

@jeremylong Do you have a good idea for a default environment variable name?

The problem is eclipse-sisu/sisu.plexus#29 here, i.e. it is not easy to invalidate the default value. In order to deliberately not use the environment variable name with the default value one would need to provide an invalid environment name (as empty strings do not overwrite the default value). Also with the default value it is no longer clear what the intention of the user is (i.e. when to emit warnings/error in case of unassigned environment variables).

Given all that I would rather not give a default value.

Clarify precedence of API key plugin parameters in javadoc
This closes jeremylong#6443
@kwin kwin force-pushed the feature/nvd-api-key-via-env-variable branch from baf5289 to 69dc50b Compare February 24, 2024 14:45
@jeremylong
Copy link
Owner

Regarding the default environment variable name - why would it be a problem if there was a default? Do you think random machines using ODC would have an environment variable: NVD_API_KEY setup that isn't an NVD API KEY?

@kwin
Copy link
Contributor Author

kwin commented Mar 4, 2024

Do you think random machines using ODC would have an environment variable: NVD_API_KEY setup that isn't an NVD API KEY?

Unlikely but may happen. My concern is more that it becomes impossible to emit a proper warning in case something is not setup as it should be. With this PR the following sources are considered

  1. explicit API key
  2. API key from environment variable
  3. API key in settings.xml in configured server id

Each option is only considered if the accoding plugin parameter is set.
Once you give 2 a default, that would always need to be considered and it is unclear under which circumstances a warning should be emitted if the API key cannot be found somewhere and what message it should contain...

@jeremylong jeremylong added this to the 9.0.10 milestone Mar 7, 2024
@jeremylong jeremylong merged commit ea72798 into jeremylong:main Mar 7, 2024
6 checks passed
@kwin kwin deleted the feature/nvd-api-key-via-env-variable branch March 7, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maven changes to the maven plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maven Plugin: Allow to read NVD API Key from environment variable
2 participants