Feature: Improve automated containerized deployment #2550

syalioune · 2023-03-02T13:42:27Z

Description

Allow to configure several items through environment variables on first startup for containerized deployments :

ConfigProperty values
Default admin user and credentials

Addressed Issue

Additional Details

This flexibility given by this PR could also be part of #2524 solution. Experienced users or first timers could disable NVD mirroring with VULN_SOURCE_NVD_ENABLED=false environment variable (default is true)

Checklist

I have read and understand the contributing guidelines
~~This PR fixes a defect, and I have provided tests to verify that the fix is effective~~
This PR implements an enhancement, and I have provided tests to verify that it works as intended
~~This PR introduces changes to the database model, and I have added corresponding update logic~~
This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

nscuro · 2023-03-02T20:20:04Z

src/main/java/org/dependencytrack/persistence/DefaultObjectGenerator.java

+
+    String generateEnvVariableName(ConfigPropertyConstants configProperty) {
+        StringBuilder sb = new StringBuilder();
+        sb.append(configProperty.getGroupName().toUpperCase().replaceAll("[\\-\\.]", "_"));


Should we require a common prefix here, too? Similar to DEPENDENCY_TRACK_ADMIN_USERNAME.

Perhaps DT would be a better choice than DEPENDENCY_TRACK, to keep the length reasonable :D

I thought about it and didn't add a prefix out of pure lazyness. For consistency sake it would require env variable like DEFAULT_TEMPLATES_OVERRIDE_ENABLED to have the prefix also.
I'll add the DT prefix to match alpine behaviour.

valentijnscholten · 2023-04-11T15:40:27Z

src/main/java/org/dependencytrack/persistence/DefaultObjectGenerator.java

@@ -48,6 +48,24 @@ public class DefaultObjectGenerator implements ServletContextListener {

    private static final Logger LOGGER = Logger.getLogger(DefaultObjectGenerator.class);

+    static final String DEFAULT_ADMIN_USERNAME = "admin";
+
+    static final String ADMIN_USERNAME_ENV_VARIABLE = "DEPENDENCY_TRACK_ADMIN_USERNAME";


Should it be made even more clear that this is just about the initial value for these variables, i.e. DEPENDENCY_TRACK_INITIAL_ADMIN_USERNAME? To prevent people on Slack asking why the "change of the admin password env variable doesn't work".

Yes good idea !

See DependencyTrack#2443. Allow configuration by environment variables. Signed-off-by: syalioune <[email protected]>

Shielding unit test from theCaseInsensitiveEnvironment field absence (JVM dependent) Signed-off-by: syalioune <[email protected]>

Adding a prefix (DT) for all dependency track specific env variable name with backward compatibility for templates Signed-off-by: syalioune <[email protected]>

Using DT_INIT prefix to be more explicit about the usage of the environment variables Signed-off-by: syalioune <[email protected]>

melba-lopez

@syalioune thanks for adding this feature!! I've been thinking about how to do this on my laptop before it gets deployed. looking forward to using it :)

I only have 1 minor comment on 2 lines. It may be a misinterpretation on my part, but wanted to make sure.

melba-lopez · 2023-08-22T18:36:38Z

src/main/java/org/dependencytrack/model/ConfigPropertyConstants.java

@@ -79,8 +79,8 @@ public enum ConfigPropertyConstants {
    KENNA_TOKEN("integrations", "kenna.token", null, PropertyType.ENCRYPTEDSTRING, "The token to use when authenticating to Kenna Security"),
    KENNA_CONNECTOR_ID("integrations", "kenna.connector.id", null, PropertyType.STRING, "The Kenna Security connector identifier to upload to"),
    ACCESS_MANAGEMENT_ACL_ENABLED("access-management", "acl.enabled", "false", PropertyType.BOOLEAN, "Flag to enable/disable access control to projects in the portfolio"),
-    NOTIFICATION_TEMPLATE_BASE_DIR("notification", "template.baseDir", SystemUtils.getEnvironmentVariable("DEFAULT_TEMPLATES_OVERRIDE_BASE_DIRECTORY", System.getProperty("user.home")), PropertyType.STRING, "The base directory to use when searching for notification templates"),
-    NOTIFICATION_TEMPLATE_DEFAULT_OVERRIDE_ENABLED("notification", "template.default.override.enabled", SystemUtils.getEnvironmentVariable("DEFAULT_TEMPLATES_OVERRIDE_ENABLED", "false"), PropertyType.BOOLEAN, "Flag to enable/disable override of default notification templates"),
+    NOTIFICATION_TEMPLATE_BASE_DIR("notification", "template.baseDir", SystemUtils.getEnvironmentVariable("DT_DEFAULT_TEMPLATES_OVERRIDE_BASE_DIRECTORY", SystemUtils.getEnvironmentVariable("DEFAULT_TEMPLATES_OVERRIDE_BASE_DIRECTORY", System.getProperty("user.home"))), PropertyType.STRING, "The base directory to use when searching for notification templates"),


@syalioune maybe im misinterpreting this line. in the deploy-docker.md and docker-compose.yml you replaced:

DEFAULT_TEMPLATES_OVERRIDE_BASE_DIRECTORY
with
DT_DEFAULT_TEMPLATES_OVERRIDE_BASE_DIRECTORY

should this state:
....SystemUtils.getEnvironmentVariable("DT_DEFAULT_TEMPLATES_OVERRIDE_BASE_DIRECTORY", System.get.....

similar comment for the next line "DEFAULT_TEMPLATES_OVERRIDE_ENABLED" is called, but was removed earlier.

Hello @melba-lopez
Sorry I missed your comment.
The intent is to be backward compatible with previous releases where DEFAULT_TEMPLATES_OVERRIDE_.* variables were valid so the code fetches the values from (decreasing priority order) :

DT_DEFAULT_TEMPLATES_.*

DEFAULT_TEMPLATES_.*

user.home

nscuro · 2023-10-16T16:35:21Z

Thanks for your work on this @syalioune.

I am going to postpone merging this though. I've had a few discussions around this and having environment variables that are only effective upon first launch of the application turn out to be rather confusing to users. It was pointed out that something akin to what Grafana is doing would be preferable.

Keeping it under the umbrella of "provisioning" makes it more obvious what it's doing.

It also makes more sense given that we need similar functionality for other areas of the application, e.g. alert configurations, policy definitions, and more (see for example #3101).

syalioune · 2023-10-18T18:00:59Z

I am going to postpone merging this though

No worries 👍

sylvainOL · 2023-12-19T06:49:46Z

Hello, I've seen that #2550 has not been integrated in 4.9 or 4.10. Do you know the plan to handle it?

thanks, I'm very interested in this so we can create a small operator (mainly for configuration) dealing with Dependency Track

nscuro · 2023-12-19T10:26:28Z

@sylvainOL There's no one actively working on this ATM. But given your plans to build an operator around DT, we'd love to hear from you how configuration should look like ideally. What would make your life the easiest?

sylvainOL · 2023-12-19T15:58:31Z

Hello @nscuro,

After a quick research, I think I've got two big missing point to simplify creating a (configuration) operator:

default admin password. We could change it the first time (if API is available) but it's preferrable to be able to give a secret / env variable for setting it.
first API key: AFAIK, the first API key creation flow is not very linear. Same as for admin password, it would be nice to be able to retrieve it from env variable

After, as @phoenixadb (disclosure, we're on same company ;) ) said in #2443, if basic configuration (enabling the different vuln databases) could be also done via env variable it would be good.
If not, a simple API endpoint for that is mandatory of course.

And by configuration operator, I mean a basic one but if community is interested, we can opensource it (when we have something, we don't have anything ad far of today).

nscuro reviewed Mar 2, 2023

View reviewed changes

syalioune force-pushed the feature/2443-improve-automated-deployment branch from bd5396f to f792e54 Compare March 19, 2023 21:39

nscuro added this to the 4.9 milestone Apr 10, 2023

valentijnscholten reviewed Apr 11, 2023

View reviewed changes

syalioune added 4 commits April 11, 2023 19:58

Feature: Improve automated containerized deployment

8497d22

See DependencyTrack#2443. Allow configuration by environment variables. Signed-off-by: syalioune <[email protected]>

Feature: Improve automated containerized deployment

cef88f3

Shielding unit test from theCaseInsensitiveEnvironment field absence (JVM dependent) Signed-off-by: syalioune <[email protected]>

Feature: Improve automated containerized deployment

0284361

Adding a prefix (DT) for all dependency track specific env variable name with backward compatibility for templates Signed-off-by: syalioune <[email protected]>

Feature: Improve automated containerized deployment

ad9f25e

Using DT_INIT prefix to be more explicit about the usage of the environment variables Signed-off-by: syalioune <[email protected]>

syalioune force-pushed the feature/2443-improve-automated-deployment branch from f792e54 to ad9f25e Compare April 11, 2023 18:09

marcosene approved these changes Jun 27, 2023

View reviewed changes

melba-lopez added the enhancement New feature or request label Jul 28, 2023

melba-lopez reviewed Aug 22, 2023

View reviewed changes

melba-lopez added docker Pull requests that update Docker code pending release labels Aug 22, 2023

nscuro removed the pending release label Oct 16, 2023

nscuro removed this from the 4.9 milestone Oct 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Improve automated containerized deployment #2550

Feature: Improve automated containerized deployment #2550

syalioune commented Mar 2, 2023 •

edited

Loading

nscuro Mar 2, 2023

syalioune Mar 3, 2023 •

edited

Loading

valentijnscholten Apr 11, 2023

syalioune Apr 11, 2023

melba-lopez left a comment

melba-lopez Aug 22, 2023

melba-lopez Aug 22, 2023

syalioune Sep 21, 2023

nscuro commented Oct 16, 2023

syalioune commented Oct 18, 2023

sylvainOL commented Dec 19, 2023

nscuro commented Dec 19, 2023

sylvainOL commented Dec 19, 2023

Feature: Improve automated containerized deployment #2550

Are you sure you want to change the base?

Feature: Improve automated containerized deployment #2550

Conversation

syalioune commented Mar 2, 2023 • edited Loading

Description

Addressed Issue

Additional Details

Checklist

nscuro Mar 2, 2023

Choose a reason for hiding this comment

syalioune Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

valentijnscholten Apr 11, 2023

Choose a reason for hiding this comment

syalioune Apr 11, 2023

Choose a reason for hiding this comment

melba-lopez left a comment

Choose a reason for hiding this comment

melba-lopez Aug 22, 2023

Choose a reason for hiding this comment

melba-lopez Aug 22, 2023

Choose a reason for hiding this comment

syalioune Sep 21, 2023

Choose a reason for hiding this comment

nscuro commented Oct 16, 2023

syalioune commented Oct 18, 2023

sylvainOL commented Dec 19, 2023

nscuro commented Dec 19, 2023

sylvainOL commented Dec 19, 2023

syalioune commented Mar 2, 2023 •

edited

Loading

syalioune Mar 3, 2023 •

edited

Loading