Trino. Ability to override catalog individual properties

[arturmkr]

Desctiption:

Currently, in the Trino Helm chart, certain configurations are provided as multiline strings using the YAML block syntax (|-). For example:

catalogs:
  glue: |-
    connector.name=hive
    hive.metastore=glue
    hive.metastore.glue.region=eu-central-1
    fs.native-s3.enabled=true
    s3.region=eu-central-1
    s3.path-style-access=true

While this structure is useful for grouping related settings, it has a significant limitation: it is not possible to override individual properties within the block from another values.yaml file. For example, I cannot override s3.path-style-access alone in MyEnv.yaml.

Use Case:

As a user of Trino Helm charts, I want to customize individual properties within grouped configurations without redefining the entire block. This would allow me to:
• Avoid duplicating unchanged configuration in multiple environments.
• Reduce errors and maintenance overhead when managing environment-specific values.

Proposed Solution:

Refactor the configuration blocks into individual key-value pairs in the Helm chart, like so:

catalogs:
  glue:
    connector.name=hive
    hive.metastore=glue
    hive.metastore.glue.region=eu-central-1
    fs.native-s3.enabled=true
    s3.region=eu-central-1
    s3.path-style-access=true

This way, I can override specific properties in MyEnv.yaml without redefining the entire block:

catalogs:
  glue:
    s3.path-style-access: false

[mosabua]

I am not sure about this but I dont think connector.name=hive is a valid key-value pair in yaml.

I think this would very much be a breaking change, and I am wondering if there really is much benefit.

Also what would it take in terms of implementation... for example we definitely do NOT want to define all the valid property names in the chart anywhere since they change all the time and vary a lot between releases.

[nineinchnick]

It's not unreasonable to ask for this. We already added support for lists in a few places where we had config properties only set as one string. We can make it backward compatible by checking the type of the value. I also don't think we have to validate available properties in the chart itself.

What about passing properties through the tpl function, as suggested in #283? You could keep it as a list in a custom value, and process it using any valid Helm expressions in the catalog entry. Support for this in additionalConfigFiles was added in #240, check out the example in the PR description.

[mosabua]

Thanks for clarifying @nineinchnick .. if it is doable without breaking things and without overcomplifying the chart source with hardcoded properties then this could be a great change.

The tpl approach also sounds reasonable .. at least for config.properties, node.properties and the catalogs. For other properties files like various access control ones I am a bit weary of doing .. it would just get a bit much. But thats just a gut feeling .. could be unreasonable