Skip to content

Conversation

@kilfoyle
Copy link
Contributor

@kilfoyle kilfoyle commented Jul 10, 2025

According to issue #2089, the Helm ca-cert-validity setting documented on the Apply ECK configuration settings page should rather be caValidity. This also adds a tip for generating the list of valid settings.

I'm not familiar with these settings at all so I'd appreciate if anyone can confirm that this change is correct.


eck-cacert

@kilfoyle kilfoyle requested review from eedugon, pebrc and thbkrkr July 10, 2025 13:34
@kilfoyle kilfoyle requested a review from a team as a code owner July 10, 2025 13:34
@github-actions
Copy link

github-actions bot commented Jul 10, 2025

🔍 Preview links for changed docs

@eedugon
Copy link
Contributor

eedugon commented Jul 10, 2025

This is interesting @kilfoyle !

This is actually a bug caused by me, let me explain.

I thought the helm chart passes directly everything inside config as configuration parameters to ECK, and in ECK, the theoretical configuration parameter to configure that is ca-cert-validity.

image

That's documented here.

The problem is that the helm chart is using its own configuration parameters instead of directly relying on the actual ECK parameters (this shows how caValidity is added to the actual ca-cert-validity setting).

To solve this we need to change a bit the narrative, because I wanted to tell users (on purpose) that they can configure ANY ECK setting documented in the ECK reference doc using the helm chart under the config section of the values...

And that's definitely incorrect now that I see the chart, sorry!

Let me share some small suggestions to address this change in a different way. For example, what you have added as a TIP should moved more to the beginning of the section, and probably we should add a note saying that the Helm chart does NOT use the standard configuration parameters documented in (LINK TO REFERENCE), and suggest to use the helm show values command to see the list of configurable settings (which should be aligned with the official config settings but with different syntax).

cc: @elastic/cloud-k8s , just in case you want to add anything here.

Copy link
Contributor

@eedugon eedugon left a comment

Choose a reason for hiding this comment

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

I'm approving, but to avoid future misunderstandings I think we have to update a bit the introduction of that section, as my current original intro would cause confusion if we don't explain that the helm chart uses its own settings for the ECK configuration flags (they are not just passed through).

@kilfoyle
Copy link
Contributor Author

Thanks a lot @eedugon for explaining! I've updated the section so that we can explain at the top of the Helm section about the Helm settings being different from the ECK settings. I assume that Option 2, using the --set command, should similarly use the Helm version rather than the ECK version of the parameter. I've updated the PR changes as shown below.


eck-cacert2

@eedugon
Copy link
Contributor

eedugon commented Jul 10, 2025

beautiful!!!!! Many thanks for this!


```sh
helm upgrade elastic-operator elastic/eck-operator --set config.ca-cert-validity=43800h -n elastic-system
helm upgrade elastic-operator elastic/eck-operator --set config.caValidity=43800h -n elastic-system
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch :)

@kilfoyle kilfoyle merged commit b699987 into elastic:main Jul 11, 2025
7 checks passed
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