Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helm chart improvements including allowing user password to be pulled from K8s secret #753

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Speissi
Copy link

@Speissi Speissi commented Jun 10, 2024

What

  • Make the configs for min_pool_size and sever_lifetime configurable under the user section.
  • Allow enabling `server_tls``
  • Change statement_timeout to use default 0 if not specified
  • Allow pulling user password from existing K8s secret

Why

  • Currently, the global server_lifetime gets overwritten by the hardcoded server_lifetime value under the user section, which can cause confusion. So, if it is not explicitly set in the values.yaml, it should not be set per user at all, and the global value should be respected.
  • min_pool_size should also be configurable and if not set the default value will be 3 as per the original version of the chart.
  • If the DB cluster does not access insecure connections the connection will fail without server_tls = true.
  • statement_timeout has default value of 0 and should be explicitly enabled.
  • Passing plain password in Helm values should be avoided.

@Speissi Speissi changed the title Helm chart: make user-specific min_pool_size and server_lifetime configurable Helm chart improvements including allowing user password to be pulled from K8s secret Jun 10, 2024
@Speissi
Copy link
Author

Speissi commented Jun 10, 2024

Checking out the workflow https://github.com/postgresml/pgcat/blob/main/.github/workflows/generate-chart-readme.yaml Has this ever worked? I don't think there is a README.md for the chart? Also the paths are a bit funky if this is expected to work. For example

readme-generator --values "charts/${chart}/values.yaml" --readme "charts/${chart}/README.md" --schema "/tmp/schema.json"

charts/${chart}/values.yaml translates into 'charts/pgcat/templates/values.yaml' which does not seem right and should probably be charts/pgcat/values.yaml

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.

1 participant