Conversation
There was a problem hiding this comment.
Pull request overview
Adds declarative support for providing cloud integration credentials as inline JSON in the OpenCost Helm chart, while keeping the existing “bring your own secret” flow and enforcing mutual exclusivity between the two approaches.
Changes:
- Added
opencost.cloudIntegrationJSONtovalues.yaml(mutually exclusive withopencost.cloudIntegrationSecret) plus an example JSON block. - Introduced centralized helper logic to (a) fail fast when both values are set and (b) compute the effective secret name.
- Added a new
secret-cloud-integration.yamltemplate to create the secret when JSON is provided, and updated the Deployment to mount the computed secret name.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/opencost/values.yaml | Documents the new cloudIntegrationJSON value and provides an example payload. |
| charts/opencost/templates/secret-cloud-integration.yaml | New template to create the cloud-integration.json secret from inline JSON. |
| charts/opencost/templates/deployment.yaml | Adds mutual-exclusivity check call and mounts a computed cloud integration secret name. |
| charts/opencost/templates/_helpers.tpl | Adds helper functions for validation + secret name computation; includes new secret in config checksum. |
| charts/opencost/README.md | Documents the new value and clarifies the existing secret option. |
Comments suppressed due to low confidence (1)
charts/opencost/templates/deployment.yaml:363
- New behavior is introduced for the declarative JSON flow (creating
secret-cloud-integration.yamland switching the Deployment to mount the computed secret name), but there are no helm-unittest assertions covering it. Adding unit tests would help prevent regressions (e.g., assert that settingopencost.cloudIntegrationJSONrenders a Secret named<fullname>-cloud-integrationand that the Deployment mounts that name; and assert that setting bothcloudIntegrationSecretandcloudIntegrationJSONfails template rendering).
{{- if $cloudIntegrationSecretName }}
- name: cloud-integration
mountPath: /var/configs/cloud-integration.json
subPath: cloud-integration.json
readOnly: true
{{- end }}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| value: "false" | ||
| {{- end }} | ||
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.opencost.exporter.extraVolumeMounts .Values.opencost.customPricing.enabled .Values.opencost.cloudIntegrationSecret .Values.opencost.updateCaTrust.enabled}} | ||
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.opencost.exporter.extraVolumeMounts .Values.opencost.customPricing.enabled $cloudIntegrationSecretName .Values.opencost.updateCaTrust.enabled}} |
There was a problem hiding this comment.
The volumeMounts: block is guarded by an if or ... that does not include .Values.opencost.metrics.config.enabled, but inside the block you conditionally add the custom-configs mount when or .Values.opencost.customPricing.enabled .Values.opencost.metrics.config.enabled. As written, enabling opencost.metrics.config.enabled alone will never render this mount because the parent if stays false. Either include opencost.metrics.config.enabled in the parent condition or remove it from the inner mount condition to keep the logic consistent.
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.opencost.exporter.extraVolumeMounts .Values.opencost.customPricing.enabled $cloudIntegrationSecretName .Values.opencost.updateCaTrust.enabled}} | |
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.opencost.exporter.extraVolumeMounts .Values.opencost.customPricing.enabled .Values.opencost.metrics.config.enabled $cloudIntegrationSecretName .Values.opencost.updateCaTrust.enabled}} |
| {{- end }} | ||
| {{- end }} | ||
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.extraVolumes .Values.opencost.customPricing.enabled .Values.opencost.cloudIntegrationSecret .Values.opencost.ui.enabled .Values.opencost.updateCaTrust.enabled }} | ||
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.extraVolumes .Values.opencost.customPricing.enabled $cloudIntegrationSecretName .Values.opencost.ui.enabled .Values.opencost.updateCaTrust.enabled }} |
There was a problem hiding this comment.
Similarly for volumes:: the parent if or ... does not include .Values.opencost.metrics.config.enabled, but within the block you add the custom-configs volume when or .Values.opencost.customPricing.enabled .Values.opencost.metrics.config.enabled. If opencost.metrics.config.enabled is enabled by itself, this custom-configs volume will never be rendered. Consider adding opencost.metrics.config.enabled to the parent condition (or removing it from the inner condition) so the template behavior matches the intent.
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.extraVolumes .Values.opencost.customPricing.enabled $cloudIntegrationSecretName .Values.opencost.ui.enabled .Values.opencost.updateCaTrust.enabled }} | |
| {{- if or .Values.plugins.enabled .Values.opencost.exporter.persistence.enabled .Values.extraVolumes .Values.opencost.customPricing.enabled .Values.opencost.metrics.config.enabled $cloudIntegrationSecretName .Values.opencost.ui.enabled .Values.opencost.updateCaTrust.enabled }} |
| labels: | ||
| {{- include "opencost.labels" . | nindent 4 }} | ||
| data: | ||
| cloud-integration.json: {{ .Values.opencost.cloudIntegrationJSON | replace "\n" "" | b64enc }} |
There was a problem hiding this comment.
The secret data value is not quoted and is derived from b64enc, which can be parsed by YAML as a non-string scalar in edge cases (e.g., values that look like booleans/numbers). For consistency with templates/secret.yaml and to avoid YAML typing issues, quote the encoded value. Also, stripping all literal newlines from the provided JSON mutates user input unnecessarily; consider encoding the value as-is (or at most trimming a single trailing newline) so formatted JSON/Windows line endings aren’t altered.
| cloud-integration.json: {{ .Values.opencost.cloudIntegrationJSON | replace "\n" "" | b64enc }} | |
| cloud-integration.json: {{ .Values.opencost.cloudIntegrationJSON | trimSuffix "\n" | b64enc | quote }} |
| */}} | ||
| {{- define "opencost.cloudIntegration.secretConfigCheck" -}} | ||
| {{- if and .Values.opencost.cloudIntegrationSecret .Values.opencost.cloudIntegrationJSON -}} | ||
| {{- fail "\nopencost.cloudIntegrationSecret and opencost.cloudIntegrationJSON are mutually exclusive. Please specify only one." -}} |
There was a problem hiding this comment.
The fail message starts with a leading newline ("\n..."), which is inconsistent with other validation errors in this chart and can produce awkward output formatting. Consider removing the leading newline so the error message is clean and consistent with the other fail usages in this file.
| {{- fail "\nopencost.cloudIntegrationSecret and opencost.cloudIntegrationJSON are mutually exclusive. Please specify only one." -}} | |
| {{- fail "opencost.cloudIntegrationSecret and opencost.cloudIntegrationJSON are mutually exclusive. Please specify only one." -}} |
| # "athenaBucketName": "s3://AWS_cloud_integration_athenaBucketName", | ||
| # "athenaRegion": "AWS_cloud_integration_athenaRegion", | ||
| # "athenaDatabase": "AWS_cloud_integration_athenaDatabase", | ||
| # "athenaTable": "AWS_cloud_integration_athenaBucketName", |
There was a problem hiding this comment.
In the example JSON, the placeholder value for athenaTable appears to be copied from athenaBucketName ("AWS_cloud_integration_athenaBucketName"), which is confusing/misleading for users. Consider changing it to a table-specific placeholder (e.g., "AWS_cloud_integration_athenaTable") to match the field name.
| # "athenaTable": "AWS_cloud_integration_athenaBucketName", | |
| # "athenaTable": "AWS_cloud_integration_athenaTable", |
Added support for declarative cloud integration JSON values and aligned secret creation/mounting with the Kubecost chart pattern. Fixes #321
Key Changes
opencost.cloudIntegrationJSON(mutually exclusive withopencost.cloudIntegrationSecret) and example JSON incharts/opencost/values.yaml.secret-cloud-integration.yamlto create the secret from JSON and updated the deployment to mount the computed secret name.User Impact
opencost.cloudIntegrationJSONto have the chart create thecloud-integration.jsonsecret; existingopencost.cloudIntegrationSecretusage still works, and using both now fails fast.Testing
1) Existing secret flow
Key output:
secretName: cloud-costsmountPath: /var/configs/cloud-integration.jsonwithsubPath: cloud-integration.jsonResult: PASS — rendered deployment mounts the expected existing secret name.
2) Declarative JSON flow
Key output:
kind: Secretwithname: opencost-cloud-integrationcloud-integration.json: eyJhd3MiOltdfQ==secretName: opencost-cloud-integrationmounted at/var/configs/cloud-integration.jsonResult: PASS — secret is created from JSON and deployment mounts the computed secret name.