values/readme has 'server.configannotation', but _helpers.tpl has server.includeConfigAnnotation

[michel-thebeau-WR]

Describe the bug
By observation only: server.configAnnotation option would be ignored because templates/_helpers.tpl refers to server.includeConfigAnnotation instead.

To Reproduce
Inspect the source.
values.yaml:

openbao-helm/charts/openbao/values.yaml

Line 686 in b73f988

configAnnotation: false

README:

openbao-helm/charts/openbao/README.md

Line 163 in b73f988

| server.configAnnotation | bool | `false` | |

templates/_helpers.tpl:

openbao-helm/charts/openbao/templates/_helpers.tpl

Line 451 in b73f988

{{- if .Values.server.includeConfigAnnotation }}

Expected behavior
There would be alignment between the README.md, values.yaml and _helpers.tpl

Environment

• Kubernetes version: N/A
• openbao-helm version: 0.7.0 and main branch

Chart values:

default chart values

Additional context

• The reporter has not tested this configuration, but understands that server.configAnnotation wouldn't result in the sha of config being added to annotations. Workaround would be to use server.includeConfigAnnotation for helm override value.
• opendev/starlingx developer will test a fix soon probably

[michel-thebeau-WR]

Reproduced and workaround confirmed:

# helm override as documented:
server:
  configAnnotation: true

# Result:
annotation absent from server pod

# helm override matching __helpers.tpl
server:
  includeConfigAnnotation: true

# Result:
metadata:
  annotations:
    openbao.hashicorp.com/config-checksum: 60c1c10eb30047d28bfb60edc8f2ff98f809b1e2b991ae4ce632ab4a99efc254

I was going to offer reference to a tested patch, but it is mixed with another fix/condition that I don't think is a valid bug. So just snip the suggested bits here:

--- a/charts/openbao/templates/_helpers.tpl
+++ b/charts/openbao/templates/_helpers.tpl

-  {{- if .Values.server.includeConfigAnnotation }}
+  {{- if .Values.server.configAnnotation }}

[cipherboy]

@michel-thebeau-WR Sadly we're unable to merge changes without DCO from the author / authorized representative. Would you be able to open a PR for this and the other one? Thanks!

[michel-thebeau-WR]

Developer Certificate of Origin; add signed-off-by line.

I'll have some reading to do; at the very least I have to push the change to github somewhere so it can do commit comparison.

[michel-thebeau-WR]

The mismatch in values.yaml is in the original commit:

commit d186b6f Add annotation on config change (hashicorp#1001)
Date: Mon Mar 18 11:03:56 2024 -0700

Copied to README.md for openbao docs:

commit 5544941 begin changes to using openbao everywhere instead of vault
Date: Thu May 16 12:04:32 2024 +0200

A change should include unit/tests.

I prefer configAnnotation, so I'm going with that for the pull request for now.

[michel-thebeau-WR]

Oooo, look what I found: https://github.com/apps/dco

[cipherboy]

@michel-thebeau-WR Yes, that is enabled on this repository: https://github.com/openbao/openbao-helm/pull/33/checks?check_run_id=35545003905 :-)

Hoping @JanMa will have time to take a look at your PRs, but if not, I'll review and merge next week sometime!

[michel-thebeau-WR]

I shared the link with Tae Park as well.