Skip to content

feat(envs): add configurations to include extra envs for containers#248

Merged
andrewazores merged 7 commits intocryostatio:mainfrom
tthvo:extra-env-vars
May 28, 2025
Merged

feat(envs): add configurations to include extra envs for containers#248
andrewazores merged 7 commits intocryostatio:mainfrom
tthvo:extra-env-vars

Conversation

@tthvo
Copy link
Member

@tthvo tthvo commented May 16, 2025

Fixes #203

Descriptions

Adding an field storage.config.extra.envVars and storage.config.extra.envSources to allow users to configure additional environment variables for the storage container.

This can be extended to other containers (i.e. core, database, etc).

@andrewazores
Copy link
Member

Looks great to me.

This can be extended to other containers (i.e. core, database, etc).

What are your thoughts on how to handle multi-container Pods?

@tthvo
Copy link
Member Author

tthvo commented May 16, 2025

What are your thoughts on how to handle multi-container Pods?

I guess we are all well set up to handle that since we have separate helm value section for each container (i.e. core, db, storage, grafana, datasource, oauth2Proxy and openshiftOauthProxy). So, each can have a section, for example:

core:
  config:
    extra:
      enVars: []
      envSources: []

grafana:
  config:
    extra:
      enVars: []
      envSources: []

In the future, if we are adding any new container, its customization probably is going to have its own section and follows the same pattern, right?

@andrewazores
Copy link
Member

Makes sense, but I do see one spot where it might get funny: the auth proxy container can be used in more than one Deployment. Already it can be present in both the main Cryostat Pod and the Reports Pod. I don't know what kind of additional configuration the user might want to set on this container, but with this structure it would only be possible to give both instances the same configurations, which might not be suitable.

(It would also be nice if there could be someway to reuse these snippets in different places and not copy-paste the same structures everywhere, but I think this is a Helm/yaml limitation)

@tthvo
Copy link
Member Author

tthvo commented May 16, 2025

(It would also be nice if there could be someway to reuse these snippets in different places and not copy-paste the same structures everywhere, but I think this is a Helm/yaml limitation)

Unfortunately, we need to copy-paste 😅 I guess we can generate the values.yaml from a template file, let's say values.yaml.tpl with go/template. But that sounds a bit over-engineering :D

Makes sense, but I do see one spot where it might get funny: the auth proxy container can be used in more than one Deployment. Already it can be present in both the main Cryostat Pod and the Reports Pod. I don't know what kind of additional configuration the user might want to set on this container, but with this structure it would only be possible to give both instances the same configurations, which might not be suitable.

Right, totally unaware of that 😅 I guess we can do the following where we have a common settings and additional settings for specific pod use case. This is similar to how k8s defines SecurityContext.

The settings with pod-specific context will take precedence. This works well because env var that are defined later takes precendence [0].

oauth2Proxy:
  config:
    ## @extra oauth2Proxy.config.extra Extra configurations for the OAuth2 Proxy container
    extra:
      ## @param oauth2Proxy.config.extra.envVars [array] Extra environment variables for the OAuth2 Proxy container. See: [Define Environment Variables](https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/)
      envVars: []
      ## @param oauth2Proxy.config.extra.envSources [array] Sources for extra variables for the OAuth2 Proxy container. See: [Define Environment Variables From ConfigMap](https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/#configure-all-key-value-pairs-in-a-configmap-as-container-environment-variables)
      envSources: []
    podContext: # This probably can be renamed
      main:
        ## @extra oauth2Proxy.config.podContext.main.extra Extra configurations for the OAuth2 Proxy container in main pod. This will overwrite common settings in oauth2Proxy.config.extra
        extra:
          envVars: []
          envSources: []
      report:
        ## @extra oauth2Proxy.config.podContext.report.extra Extra configurations for the OAuth2 Proxy container in report pod. This will overwrite common settings in oauth2Proxy.config.extra
        extra:
          envVars: []
          envSources: []

@tthvo
Copy link
Member Author

tthvo commented May 16, 2025

Or maybe this would be a better value settings (looks simpler). I am fine either way for as long as we document it. These values can be set in proxy templates _oauth2Proxy.tpl and _reports_authproxy.tpl, where common settings are placed first before specific pod overwrites. WDYT?

oauth2Proxy:
  config:
    ## @extra oauth2Proxy.config.extra Extra configurations for the OAuth2 Proxy container
    extra:
      ## @param oauth2Proxy.config.extra.envVars [array] Extra environment variables for the OAuth2 Proxy container. See: [Define Environment Variables](https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/)
      envVars: []
      ## @param oauth2Proxy.config.extra.envSources [array] Sources for extra variables for the OAuth2 Proxy container. See: [Define Environment Variables From ConfigMap](https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/#configure-all-key-value-pairs-in-a-configmap-as-container-environment-variables)
      envSources: []
      podContext:
        main:
            envVars: []
            envSources: []
        report:
          envVars: []
          envSources: []

@andrewazores
Copy link
Member

I like that second style better.

And I agree, generating the values.yaml from a template sounds like it's getting into over-engineering. It might be an idea worth keeping in mind in case the configuration keeps getting more complex, though, since we already do have a lot of duplicated template snippets across the various components.

@tthvo
Copy link
Member Author

tthvo commented May 20, 2025

yea haha I wish helm figures something out about this :D but sounds good! I'll circle back near end of week for follow-up PRs!

@tthvo tthvo changed the title feat(envs): add configurations to include extra envs for storage container feat(envs): add configurations to include extra envs for containers May 26, 2025
@tthvo tthvo force-pushed the extra-env-vars branch from 55a2d12 to 843a729 Compare May 27, 2025 20:54
tthvo added 7 commits May 27, 2025 13:58
Indentations of list entries (e.g. envs) are also adjusted to
be consistent with other templates.
Extra configurations for pod-specific use cases can be defined with
nested field 'config.extra.inPod.<pod-id>.<settings>, where:

- pod-id: An abitrary identify for cryostat component pods. For example,
  'main' is for the pod with cryostat core/main container.
- settings: Common extra settings such as envVars and envSources.
@tthvo tthvo force-pushed the extra-env-vars branch from 843a729 to 308af8f Compare May 27, 2025 20:58
@tthvo
Copy link
Member Author

tthvo commented May 27, 2025

Hey @andrewazores, this should be ready for reviews 😄

Instead of PRs, I separate the changes in separate commits for easy reviews. All containers now support extra env vars and env sources. PTAL!

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tthvo !

@andrewazores andrewazores merged commit fe84701 into cryostatio:main May 28, 2025
7 checks passed
@tthvo tthvo deleted the extra-env-vars branch May 28, 2025 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Request] Add ability to add additional env variables

2 participants