Pepr helm chart does not add `env` from `package.json`

[mjnagel]

Environment

N/A, local build only.

Definition of done:

• all Pepr settings from package.json are copied to values.yaml
• check to see if PEPR_PRETTY_LOG is changed if it breaks monitor command

Steps to reproduce

1. Create a pepr capability with some env values in package.json.
2. npx pepr build
3. Compare the manifest against the helm chart values.

Expected result

envs specified in package.json show up in both helm chart and manifest.

Actual Result

Specified envs only appear in the manifest. Chart values have the default pepr envs.

Visual Proof (screenshots, videos, text, etc)

Left side is manifest, right side is helm values:

Severity/Priority

Low/medium - there are options to workaround this by adding more helm values but the mismatch in behavior is unexpected.

Additional Context

Part of the reason I am afraid of using env helm values additions is a desire to not need to keep default envs in sync across updates since lists in helm will entirely override rather than appending. If Pepr structured the chart slightly differently to support additionalEnv or a map rather than a list that would also be a useful addition (can make this a separate issue for enhancements if needed).

[cmwylie19]

This feels a bit counter intuitive at first glance for the source of truth of the helm chart not to be the values.yaml.

The initial idea of the helm chart is to be a bit de-coupled, but you want to feed the everything in the package.json under pepr to values.yaml?

[mjnagel]

@cmwylie19 I think my main concerns are:

• Different behavior between the manifest and the helm chart (we were just attempting to switch between the two and did not expect that change in behavior)
• Values for env in particular don't lend themselves to being extended easily. Since helm values that are a list are "overwritten" vs just allowing extending, we'd have to be aware of the pepr default envs. I think I'd be happy with a solution that provided a better way to extend the default envs. Two ideas there:
  • Add a value for additionalEnv that is nothing in the values to start with, separate from Pepr's default envs. It may even make sense to have Pepr's default envs be in the manifest rather than in values.
  • Change the env value to a map of key: value for envs rather than a list. This would allow individual envs to set separately without needing to override the entire list.

I think we'd likely switch to the helm chart values for our envs IF that painpoint with the list were resolved, but it should probably be decided on whether the differing behavior between the two is expected/documented.

Edit: regarding...

This feels a bit counter intuitive at first glance for the source of truth of the helm chart not to be the values.yaml.

In total agreement here. I think my expectation would've been that the default values generated during a build include my envs specified in the package.json. Definitely not expecting envs to magically get pulled elsewhere, but would hope that they end up in the values.

[cmwylie19]

okay, i think i understand. So if we pull in our package.json envs into the values.yaml you will be unblocked? We can make sure there is more parody between pepr in package.json and values.yaml in general. To me the env in package.json is strange that it is a map instead of a env <[]EnvVar>, not sure if we should consider changing that too to be closer to the actual type

[mjnagel]

Yeah I think pulling into the values.yaml would be a good thing to do. I would personally think it also makes sense to put "necessary envs" into the deployment manifest directly rather than values since you wouldn't want someone to accidentally remove one or modify it in a poor way.

[cmwylie19]

i think that is reasonable since we can make comments in the yaml explaining where they are coming from. I feel a little strange putting them straight into the deployment without having them also in the values file. Maybe we could comment in the values file and also add them to the deployments by default with a comment?