Skip to content

Add Helm values schema validation and new configuration options#94

Open
Saremox wants to merge 7 commits intomainfrom
improve-helm-charts
Open

Add Helm values schema validation and new configuration options#94
Saremox wants to merge 7 commits intomainfrom
improve-helm-charts

Conversation

@Saremox
Copy link
Copy Markdown
Owner

@Saremox Saremox commented Mar 11, 2026

This pull request introduces significant improvements to the Helm chart for the Redis Operator, focusing on enhanced flexibility, security, and maintainability. The changes modernize the handling of Kubernetes security contexts, image pull secrets, and operator configuration, while also updating CI workflows and chart schema validation. These updates make it easier to customize deployments, improve compatibility with Kubernetes best practices, and ensure correctness through schema validation.

Helm Chart Improvements

  • Added support for separate podSecurityContext and containerSecurityContext values, deprecating the old securityContext field for better alignment with Kubernetes standards. [1] [2] [3] [4] [5]
  • Refactored handling of image pull secrets to allow multiple sources (imagePullSecrets, imageCredentials.existsSecrets, auto-generated secrets), improving flexibility for private registries. [1] [2] [3]
  • Enhanced operator configuration: new operator.* values (log level, supported namespaces, metrics path, concurrency, etc.) and extraArgs for CLI flags, plus support for extra environment variables. [1] [2] [3]

Service Account and RBAC Enhancements

  • Service account naming is now handled via a dedicated template, with support for custom names and annotations; RBAC creation is controlled by a new rbac.create value. [1] [2] [3]
  • Cleaned up RBAC rules by removing unnecessary permissions for custom resource definitions.

Validation and Testing

  • Introduced a comprehensive values.schema.json file for Helm values, enforcing correct usage and validation for image credentials, service accounts, and operator configuration.
  • Improved Helm chart testing scripts to cover new configuration scenarios and validate template rendering.

CI Workflow Updates

  • Updated GitHub Actions workflows to use a consistent Kubernetes version matrix, improved naming, and made chart tests more robust and reproducible. [1] [2] [3]

Miscellaneous

  • Minor YAML fixes in example files to ensure string values are properly quoted. [1] [2]
  • Added documentation and comments to clarify deprecated fields and new configuration options. [1] [2] [3] [4]

These changes collectively make the Redis Operator chart more robust, easier to configure, and better aligned with Kubernetes best practices.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes the Redis Operator Helm chart by adding Helm values schema validation and expanding configuration options (security contexts, image pull secrets, operator CLI args/env), alongside updates to chart tests and CI.

Changes:

  • Add values.schema.json to validate Helm values and enforce key configuration constraints.
  • Refactor chart templates/values to support pod vs container security contexts, richer operator configuration (operator.*, extraEnv), and more flexible image pull secret inputs.
  • Strengthen chart rendering tests and update CI job structure/matrix for better reproducibility.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/helm-tests.sh Expands helm template scenarios to exercise new values and schema validation.
example/redisfailover/sidecars.yaml Quotes secret string value to avoid YAML type coercion.
example/redisfailover/extravolumes-mounts.yaml Quotes secret string value to avoid YAML type coercion.
charts/redisoperator/values.yaml Introduces new values (podSecurityContext, containerSecurityContext, rbac, operator.*, extraEnv, imagePullSecrets) and deprecates legacy ones.
charts/redisoperator/values.schema.json Adds JSON schema validation and conditional requirements for service accounts and registry credentials.
charts/redisoperator/templates/service-account.yaml Adds service account annotations support and gates RBAC creation behind rbac.create.
charts/redisoperator/templates/private-registry.yaml Refactors private registry secret generation logic based on configured pull secret sources.
charts/redisoperator/templates/deployment.yaml Implements new security context split, composes operator args list, sets POD_NAMESPACE, and refactors imagePullSecrets rendering.
.github/workflows/ci.yaml Updates CI job naming and adds a shared Kubernetes version matrix plus chart tests matrix + minikube step reuse.

Comment on lines +21 to +36
@@ -16,4 +33,4 @@ metadata:
type: kubernetes.io/dockerconfigjson
data:
.dockerconfigjson: {{ template "imagePullSecret" . }}
{{- end }} No newline at end of file
{{- end }}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The secret creation is currently gated on imageCredentials.create and len($imagePullSecrets) == 0. If users set imagePullSecrets for additional registries while also expecting this chart to create the registry secret (via imageCredentials.create=true + credentials), the secret will never be created. Consider removing the len(...) == 0 condition and letting users combine the auto-generated secret with other pull secrets, or enforce this precedence in the schema/docs.

Copilot uses AI. Check for mistakes.
Saremox and others added 3 commits March 11, 2026 11:37
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

sources:
- https://github.com/saremox/redis-operator
kubeVersion: ">=1.32.0-0"
kubeVersion: ">=1.22.0-0"
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

kubeVersion is relaxed to ">=1.22.0-0", but the CI Kubernetes matrix shown in this PR only runs integration tests on 1.32+. If the chart is intended to support 1.22, consider adding at least one older version to the test matrix (or keep kubeVersion aligned with what is exercised in CI) to avoid advertising untested compatibility.

Suggested change
kubeVersion: ">=1.22.0-0"
kubeVersion: ">=1.32.0-0"

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

{{- end }}
{{- end }}
{{- end }}
{{- if .Values.imageCredentials.create }}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

imageCredentials.create=true always appends the auto-generated secret name to imagePullSecrets, but templates/private-registry.yaml only creates that secret when imageCredentials.existsSecrets is empty. If users set both imageCredentials.create=true and imageCredentials.existsSecrets (which the schema allows), the Deployment will reference a non-existent secret and Pods will fail to pull images. Only append the generated secret when it will actually be created (or change the secret template to always create it when create=true).

Suggested change
{{- if .Values.imageCredentials.create }}
{{- if and .Values.imageCredentials.create (not .Values.imageCredentials.existsSecrets) }}

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +82
{{- range (splitList " " (trim .)) }}
{{- $arg := trim . -}}
{{- if $arg }}
{{- $args = append $args $arg -}}
{{- end }}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Splitting the deprecated image.cli_args string on spaces changes the previous behavior (it used to be passed as a single argument) and breaks cases where values include quoting/embedded spaces (e.g. --foo="a b"). For backward compatibility, keep image.cli_args as a single arg (or support a proper list-based legacy field) and avoid naive whitespace splitting.

Suggested change
{{- range (splitList " " (trim .)) }}
{{- $arg := trim . -}}
{{- if $arg }}
{{- $args = append $args $arg -}}
{{- end }}
{{- $arg := trim . -}}
{{- if $arg }}
{{- $args = append $args $arg -}}

Copilot uses AI. Check for mistakes.
@@ -65,6 +74,9 @@ securityContext:
drop:
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

There is a trailing space after drop: under containerSecurityContext.capabilities which can be noisy in diffs and may trip stricter YAML linters. Remove the trailing whitespace to keep the values file clean.

Suggested change
drop:
drop:

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment on lines +9 to +10
{{- $podSecurityContext = merge (pick $legacySecurityContext "fsGroup" "fsGroupChangePolicy" "supplementalGroups" "supplementalGroupsPolicy" "sysctls") (.Values.podSecurityContext | default dict) -}}
{{- $containerSecurityContext = merge (omit $legacySecurityContext "fsGroup" "fsGroupChangePolicy" "supplementalGroups" "supplementalGroupsPolicy" "sysctls") (.Values.containerSecurityContext | default dict) -}}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The comment says new podSecurityContext / containerSecurityContext values should override the legacy securityContext, but Sprig/Helm merge keeps existing keys from the first map (it does not overwrite). As written, legacy keys can win over the new values. Consider swapping merge order (new values first) or using an overwrite merge function so the override behavior matches the intent.

Suggested change
{{- $podSecurityContext = merge (pick $legacySecurityContext "fsGroup" "fsGroupChangePolicy" "supplementalGroups" "supplementalGroupsPolicy" "sysctls") (.Values.podSecurityContext | default dict) -}}
{{- $containerSecurityContext = merge (omit $legacySecurityContext "fsGroup" "fsGroupChangePolicy" "supplementalGroups" "supplementalGroupsPolicy" "sysctls") (.Values.containerSecurityContext | default dict) -}}
{{- $podSecurityContext = merge (.Values.podSecurityContext | default dict) (pick $legacySecurityContext "fsGroup" "fsGroupChangePolicy" "supplementalGroups" "supplementalGroupsPolicy" "sysctls") -}}
{{- $containerSecurityContext = merge (.Values.containerSecurityContext | default dict) (omit $legacySecurityContext "fsGroup" "fsGroupChangePolicy" "supplementalGroups" "supplementalGroupsPolicy" "sysctls") -}}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants