Dynamicconfig.Client interface does not enforce consistent input types to client functions

[charlese-instaclustr]

Version of Cadence server, and client(which language)
This is very important to root cause bugs.

• Server version: Latest (0.25.0)

Describe the bug

Dynamicconfig.Client is the interface shared by the various dynamic config client implementations, e.g. file_based_client.go and config_store_client.go. However, the interface is quite unrestrictive; for example, it specifies an input type of interface {} for the UpdateValue function for the values being updated. This has permitted diverging implementations in the actual clients, such that config_store_client.go implicitly expects a different datatype to file_based_client.go here.

This means that calls to UpdateValue (and other dynamic config client functions with similar problems) will not succeed for all DC implementations. For one concrete example, adminHandler#AddSearchAttribute calls UpdateValue with the input format expected by file_based_client; calling AddSearchAttribute while running the config store client therefore corrupts the config store and renders the frontend.ValidSearchAttributes config value unusable.

To Reproduce
Is the issue reproducible?

• Yes

Steps to reproduce the behavior:
A clear and concise description of the reproduce steps.

• Run Cadence server with the config store dynamic config client enabled (config available in development.yaml)
• Add an initial value to the dynamic config for frontend.validSearchAttributes in the CLI: cadence admin config updc --dynamic_config_name frontend.validSearchAttributes --dynamic_config_value '{"Value": '{"testAttribute": 2}',"Filters":[]}'
• Verify that the config value has been added: cadence admin config listdc
• Trigger the AddSearchAttribute handler: cadence admin cluster asa --search_attr_key testAttribute2 --search_attr_type 2

The command will 'succeed', but after waiting ~10 seconds for the config to update, you will now find that the frontend.validSearchAttributes can no longer be written to or read from on this server.

Expected behavior
A clear and concise description of what you expected to happen.

The Dynamic Config client interface and the client implementations must be refactored to be truly consistent in their input / output types. All calls to Dynamic config client methods throughout the codebase should succeed regardless of the specific client implementation in use.

Additional context
Add any other context about the problem here, E.g. Stackstace, workflow history.

Some additional context on the UpdateValue function specifically. Note that the config_store_client clearly anticipates value having a type of []*types.DynamicConfigValue:

func (csc *configStoreClient) UpdateValue(name dc.Key, value interface{}) error {
	dcValues, ok := value.([]*types.DynamicConfigValue)
	if !ok && dcValues != nil {
		return errors.New("invalid value")
	}
	return csc.updateValue(name, dcValues, csc.config.UpdateRetryAttempts)
}

Contrastingly, file_based_client anticipates value having type map[string]interface{}; less obvious from the code snippet, but nonetheless the case and a clearly different data structure to the first snippet:

func (fc *fileBasedClient) UpdateValue(name Key, value interface{}) error {
	if err := ValidateKeyValuePair(name, value); err != nil {
		return err
	}
	keyName := name.String()
	currentValues := make(map[string][]*constrainedValue)

	confContent, err := ioutil.ReadFile(fc.config.Filepath)
	if err != nil {
		return fmt.Errorf("failed to read dynamic config file %v: %v", fc.config.Filepath, err)
	}

	if err = yaml.Unmarshal(confContent, currentValues); err != nil {
		return fmt.Errorf("failed to decode dynamic config %v", err)
	}

	cVal := &constrainedValue{
		Value: value,
	}
	currentValues[keyName] = []*constrainedValue{cVal}
	newBytes, _ := yaml.Marshal(currentValues)

	err = ioutil.WriteFile(fc.config.Filepath, newBytes, fileMode)
	if err != nil {
		return fmt.Errorf("failed to write config file, err: %v", err)
	}

	return fc.storeValues(currentValues)
}