Summary

When we add a new general source/destination config option that applies to all plugins, such as the recent addition of table_concurrency and resource_concurrency (cloudquery/plugin-sdk#268), we break backwards-compatibility between the CLI and previous versions of plugins. So if users upgrade the CLI, they will be forced to also upgrade all plugins to at least use the same SDK version, or otherwise it won't work.

The error users will get relates to the field not being recognized by older versions of the plugins:

rpc error: code = InvalidArgument desc = failed to decode spec: json: unknown field "table_concurrency"

This happens even if we include default values for the new config options and/or they are not made to be required.

Explanation

This situation arises because we have two (or more) versions of the plugin-sdk running at the same time, all communicating with one another.

For the sake of simplicity, let's use an example where the CLI is using version 1.0.1, and the source plugin is using the previous version, v1.0.0:

• CLI (plugin-sdk v1.0.1)
• Source (plugin-sdk v1.0.0)

The CLI will read yaml config and convert it to JSON. This JSON includes the new fields that were added in v1.0.1 (perhaps using default values). This JSON spec will now be sent to the source plugin, which is not aware of the new fields. At this point the source plugin will return an error indicating that it found an unrecognized field in the config: https://github.com/cloudquery/plugin-sdk/blob/main/internal/servers/source.go#L61-L68 . The sync will not be able to proceed, and the user will be forced to upgrade their plugin.

Workarounds

The only workaround to the current situation is for users to upgrade the versions of their plugins to be at least as high as the version used by the CLI.

Implications

This implies that any additional config option we add right now is a breaking change. Ideally, we want to be able to add config options and have them ignored by older versions of the plugins (maybe with a warning).

Possible Solutions

Can we remove the dec.DisallowUnknownFields() check when parsing the JSON? Maybe we should, but this will mean we are unable to warn users if their configs are incorrect. I'm opening this issue to discuss this and other potential solutions.