Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion docs/cluster-connection.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,19 @@ spec:

> **Note:** The volume source can be any type that provides a file.

> **Note:** The Helm chart does not support extra volumes yet.
> **Tip:** When using the Helm chart, configure volumes via `extraVolumes` and `extraVolumeMounts` values:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing Secrets Store CSI example.

>
> ```yaml
> app:
> extraVolumes:
> - name: db-credentials
> secret:
> secretName: db-credentials-secret
> extraVolumeMounts:
> - name: db-credentials
> mountPath: /mnt/secrets
> readOnly: true
> ```

### Examples

Expand Down
6 changes: 6 additions & 0 deletions operator/src/main/helm/values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
# Workaround: quarkus-helm generates `extraVolumeMounts: {}` (object) instead of `[]` (array)
# when the value path uses a container name filter like `containers.(name == ...)`.
# This merge file corrects the default value type.
app:
extraVolumeMounts: []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

extraVolumes needs the same override.

This file corrects extraVolumeMounts only. I built your branch and read the generated chart:

app:
  extraVolumeMounts: []
  extraVolumes:
    - {}
  imagePullSecrets:
    - {}

So helm show values presents extraVolumes as - {}. A user who copies that block and appends a volume name ships an invalid first entry. The guard expression keeps the default install correct, so nothing breaks until a user edits the value. That is the worst moment for it to break.

4 changes: 4 additions & 0 deletions operator/src/main/kubernetes/kubernetes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ spec:
spec:
affinity: {}
imagePullSecrets: [~]
volumes: [~]
containers:
- name: postgresql-operator
volumeMounts: [~]
22 changes: 22 additions & 0 deletions operator/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,22 @@ quarkus:
paths:
- (kind == Deployment).spec.template.spec.affinity
description: Kubernetes affinity configuration for Pod scheduling
extra-volumes:
property: extraVolumes
value:
- null
paths:
- (kind == Deployment).spec.template.spec.volumes
expression: "{{- if eq (toYaml .Values.app.extraVolumes | trim) \"- {}\" }} null{{- else }}{{ toYaml .Values.app.extraVolumes | nindent 8 }}{{- end }}"
description: Extra volumes to add to the operator pod
extra-volume-mounts:
property: extraVolumeMounts
value:
- null
paths:
- (kind == Deployment).spec.template.spec.containers.(name == postgresql-operator).volumeMounts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The container name is hardcoded here. We should use ${quarkus.kubernetes.name} instead.

Five other paths in this file already do, including line 86 and lines 99 to 109.

A change to quarkus.kubernetes.name breaks this path silently. quarkus-helm finds no node, processValueReference does nothing, and the value never reaches the template. No build error, no test failure, just a chart that ignores extraVolumeMounts.

expression: "{{- if eq (toYaml .Values.app.extraVolumeMounts | trim) \"- {}\" }} null{{- else }}{{- toYaml .Values.app.extraVolumeMounts | nindent 12 }}{{- end }}"
description: Extra volume mounts to add to the operator container
console-color:
property: envs.QUARKUS_CONSOLE_COLOR
value-as-bool: ${quarkus.console.color}
Expand All @@ -130,6 +146,12 @@ quarkus:
"affinity":
name: app.affinity
type: object
"extraVolumeMounts":
name: app.extraVolumeMounts
type: array
"extraVolumes":
name: app.extraVolumes
type: array
expressions:
release-name-labels:
expression: "{{ .Release.Name }}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ void helmInstall_createsDeployment() throws IOException {

Objects.requireNonNull(appValues, "appValues should not be null");
assertThat(appValues.get("image")).isNotNull();
assertThat(appValues).containsKey("extraVolumes");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

containsKey passes on the broken default. extraVolumes is [{}] in the built chart, and this assertion still passes.

assertThat(appValues.get("extraVolumes")).isEqualTo(List.of());

Two further gaps in this test:

  • Nothing asserts type: array and the description in values.schema.json. A missing type falls back to string, and then helm install rejects a list.
  • Nothing renders the chart with a real volume. helm template needs no cluster, and it validates the values against the schema. A wrong nindent produces invalid YAML for every user who sets the value, and no current test sees that.

assertThat(appValues).containsKey("extraVolumeMounts");

assertThat(chartPath.resolve("LICENSE")).exists();
assertThat(chartPath.resolve("README.md")).exists();
Expand Down Expand Up @@ -173,9 +175,11 @@ void helmInstall_createsDeployment() throws IOException {

assertThat(deployment.getSpec())
.isNotNull()
.satisfies(spec ->
assertThat(spec.getTemplate().getSpec().getImagePullSecrets()).isEmpty()
);
.satisfies(spec -> {
assertThat(spec.getTemplate().getSpec().getImagePullSecrets()).isEmpty();
assertThat(spec.getTemplate().getSpec().getVolumes()).isEmpty();
assertThat(spec.getTemplate().getSpec().getContainers().getFirst().getVolumeMounts()).isEmpty();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

getFirst() hides the risk that the named container introduces.

kubernetes.yml now names a container. If that name ever stops matching quarkus.kubernetes.name, the Pod gets two containers, and getFirst() may still return the right one. A fix would look like this:

assertThat(spec.getTemplate().getSpec().getContainers())
        .singleElement()
        .satisfies(container -> {
            assertThat(container.getName()).isEqualTo("postgresql-operator");
            assertThat(container.getVolumeMounts()).isEmpty();
        });

});

var selector = deployment.getSpec().getSelector();

Expand Down
Loading