Unable to set dbtype/connection_string via plugin_data

[arvindth]

It's not clear if this is an overlooked bug or a desired feature, but it looks like neither the database_type nor the connection_string can be set via .Values.dataStore.sql.plugin_data, even though this line in values.yaml seems to indicate it's possible.

This section in _helpers.tpl overwrites both database_type and connection_string from plugin_data, and there doesn't seem to be a way to skip the overwrite.

Perhaps the section could be wrapped with a

{{- if .Values.dataStore.sql.databaseType }}

so that if we don't specify the databaseType, the values from plugin_data get used? But then it's not clear what the desired outcome should be if plugin_data doesn't contain the required database_type and connection_string values.

Or perhaps the plugin_data should get merged in afterwards, but then it's not clear what should happen if the values are specified in both places.

[kfox1111]

You can set connection string and database_type from: https://github.com/spiffe/helm-charts-hardened/blob/532852d9072e50be976ad7cd6dedb0f8bafe3e4c/charts/spire/charts/spire-server/values.yaml#L159-L171

[faisal-memon]

@arvindth Any thoughts on @kfox1111 comment?

[arvindth]

Yes, that's exactly what I wound up doing. And I suppose that's my question - is it expected that we're not able to specify connection_string and database_type via plugin_data even though https://github.com/spiffe/spire/blob/main/doc/plugin_server_datastore_sql.md lists them as potential fields?

The reason I filed this issue is that I added it via plugin_data, and then scratched my head for a while wondering why it didn't apply until I went through _helpers.tpl and realized that it gets immediately overwritten.

What's even the point of supporting plugin_data then? Is it only for the other fields listed in https://github.com/spiffe/spire/blob/main/doc/plugin_server_datastore_sql.md?

[kfox1111]

It was added early on I think before we reached a consensus on how to add new options. It was to allow all the options to be usable before there was an official api for each option. connection_string and database_type were the first two settings to have an official api, and then I don't think it was ever revisited.

@faisal-memon we may want to consider getting rid of plugin_data here as part of the pre 1.0 stuff, adding whatever missing flags at the same time?

[faisal-memon]

Yes. That makes sense.

[arvindth]

@kfox1111 @faisal-memon this answers my question. Let me know if you wanted to keep this issue open for any followup. Otherwise, I'll go ahead and close it in the next day or so.