How to handle actions that update specific settings

[dbbaughe]

We already have a few actions supported in ISM that handle updating a specific concrete setting for an index.

Read only updates the index.blocks.write to true
https://opensearch.org/docs/im-plugin/ism/policies/#read_only

Read write updates the index.blocks.write to false
https://opensearch.org/docs/im-plugin/ism/policies/#read_write

Replica count updates the index.number_of_replicas to whatever number is provided
https://opensearch.org/docs/im-plugin/ism/policies/#replica_count

Allocation updates the require, include, exclude index allocation settings to the values provided
https://opensearch.org/docs/im-plugin/ism/policies/#allocation

Index priority updates the index.priority to provided number
https://opensearch.org/docs/im-plugin/ism/policies/#index_priority

Now a lot of these actions/steps in terms of implementation look exactly the same because all they are doing is updating an index setting, confirming a successful response to the update settings call and moving on. They don't really need to be complete separate actions as the code is pretty much duplicated but that's something we can always go and fix (code duplication).

Instead I would like to open up the discussion of future support of index settings.. which I see a few ways.

One thing we could do is just provide a generic IndexSettings or BasicSettings action which allows the users to update any and all index setting. And all it does it what most of these other index setting actions do -> call the update settings API -> get back a successful response -> and move on (or if failed, retry if configured, or move to failed). This would definitely introduce a lot of new things that can be done in ISM in a very powerful way, but the downside is you might not want to give permissions to someone to update any and all index setting. So we would need to introduce a deny/allow list type of thing to allow admins to scope the allowed index settings or block specific ones. And this would only be supported at a global level and not a user level. The other downside is if any specific index setting has any type of difference in how it's handled internally to treat it as a success then it will only support a dumbed down version of it to start with.

Alternatively we can continue down the road we've started which is introducing new actions for every type of individual index setting we want to support. This definitely allows us to add any special logic handling for each setting as it's needed, but it also means we need to actually choose every supported index setting and add the code to support it. The number of actions would grow very large just to add a lot of possibly useful index setting actions. In terms of code duplication.. that can always be handled in the implementation so shouldn't be a huge concern, but there would still be a large increase in actions.

If we choose the first option we would also need to see what we do with the existing individual index setting actions, i.e. keep them as is, deprecate them as they would also be supported in the generic basic index settings action, etc.

[bowenlan-amzn]

I like the generic IndexSetting action. It would be easier for user to use this one action instead of thinking about different action and wondering or asking where is the action to change specific setting.
For leaking permission to someone to update setting, I think this should be addressed in Update Policy API level but not action level, or provide a safety to allow some settings...

[qreshi]

I think the generic IndexSettings/BasicSettings option is a good approach as well for getting a large benefit while avoiding the possible explosion of the number of actions.

Regarding the handling of special logic, I think this can still be accomplished if we ever need it, even with the generic action. There could be some main entry point for the action execution that would determine whether it would follow the default path or do some additional work.

Example:

func handleSettings():
    when (settingName):
        SPECIAL_SETTING -> handleSpecialSetting(settingName, value)
        else -> updateSetting(settingName, value)

func handleSpecialSetting(settingName, value):
    // do some additional stuff
    updateSetting(settingName, value) 

As for the existing setting actions, we'd probably want to keep them for backwards compatability while also supporting them through the generic setting action.

Also, for the security aspect, I know there is a indices:admin/settings/update security action. I'm unfamiliar with ISM's current security model but if a user didn't have permissions to that, they shouldn't be able to update any index setting correct? We might still need the deny/allow list though should the admin want to bar the changing of a specific index setting globally. Unless that type of granularity of having update permissions for specific settings can also be defined in the user permissions and propagated in the security context to the policy execution.

[dbbaughe]

@qreshi

Hmm I wonder then should it be an action (IndexSettings/BasicSettings) that lets you pick a single setting and value?
Or should it just let you define an entire Settings JSON block that gets parsed and updated on the index so you can update many settings in a single action?

[qreshi]

@dbbaughe I suppose giving the option to define the JSON block would make it easier for users when defining their policy since they won't have to add a bunch of action calls to update multiple settings.

I can't think of any examples off the top of my head but could there be a scenario where special logic would be needed for a setting update that would not be idempotent? So say one setting was handled differently that every other one in a Settings JSON block and if we failed for one of those settings and we retry all of the Setting logic, could that be problematic? Or do we want to keep this IndexSettings action as lightweight as possible and avoid implicit behavior that the user might not be aware of?

[dbbaughe]

@qreshi Well it should be an all or nothing update call, i.e. even if a user defines 5 index settings in the JSON it's not like we update them one at a time internally. It's just a single update call -> and if it fails then it means none of the settings were applied so I don't think that should be an issue. It will definitely make the cause for failures of this single generic action explode out into a lot of possibilities as we already things like replica_count failing because of the number of shards assigned per node reaching the limit (instead of letting the setting update call succeed and just having unassigned shards). But, I don't believe we do any extra logic handling for these exceptions anyways so maybe not an issue.

[qreshi]

@dbbaughe Say we did want to design this in a way that it would be possible to add special logic for a setting if needed. In that case, the settings would probably need to be parsed into individually flattened settings and then those would be individual calls that could fail separately. So I'm wondering either:

1. We do not want to design that action in that way because we don't really see the need for special logic for settings updates (just want it to be a _settings call in ISM form essentially and if there needs to be additional logic, it's probably going to be its own action with an internal call to change that setting in a step or something)
2. We do want to design it in that way in which case it won't be just parsing and passing the JSON setting block into the update settings call as-is

[dbbaughe]

@qreshi

I think having a single setting action doing multiple setting calls that can succeed/fail independently is more confusing... to keep it simple we would just do a single setting call and apply the entire setting block included from user.

As for special handling... really depends on what it is, but we can always add that even w/ a single call. i.e. parse the JSON to see -> does it have this specific setting? -> do these extra things.