Skip to content

[RFC] Healthy amount of configuration options #5624

@nibix

Description

@nibix

Introduction

This RFC is about the software engineering practices applied for the security plugin. Its goal is to establish practises that ensure that we have a product that is both adequatly maintainable, testable and operable.

It is about the question: When shall we introduce config options for features? And when shall we possibly remove config options for features?

Example

In my recent work, I had often work with the class SystemIndexAccessEvaluator: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java

This class provides quite a few config options that can be specified in opensearch.yml:

  • plugins.security.config_index_name
  • plugins.security.filter_securityindex_from_all_requests
  • plugins.security.system_indices.indices
  • plugins.security.system_indices.enabled
  • plugins.security.unsupported.restore.securityindex.enabled
  • plugins.security.system_indices.permission.enabled

Additional configurable state is indirectly sourced from the core SystemIndexRegistry and the ActionPrivileges.

Especially the options system_indices.enabled, system_indices.permission.enabled and filter_securityindex_from_all_requests interact quite heavily in the implementation.

This leads to the situation that we get many possible cases. Just for testing the method evaluateSystemIndicesAccess() thoroughly, one would need to check any combinations of the following cases:

  1. plugins.security.system_indices.enabled is enabled vs disabled
  2. plugins.security.system_indices.permission.enabled is enabled vs disabled
  3. plugins.security.filter_securityindex_from_all_requests is enabled vs disabled
  4. Request contains system index vs request contains no system index
  5. Request contains security index vs no security index
  6. User is normal user vs user is plugin user
  7. Request is on all indices vs request is on specific indices
  8. Request is write operation vs request is read operation
  9. Regular user has system index permission vs has not

This leads to 2^9 = 512 different cases we'd need to understand and test (ok, some combinations of cases are impossible, but we still get a very high number of cases).

I would argue that these are too many cases. Maintaining tests which cover so many cases is difficult. And mentally understanding the code paths is even more difficult.

Considerations

We need to keep a few considerations in mind:

  1. Config options are useful when new code paths are introduced which might cause severe operational issues. Having the possibilty to quickly switch back to the old implementation is a good thing.
  2. Config options are useful to keep backwards compatibility
  3. Config options are useful to give users a choice

However, considerations 1 and 2 lose their validity after a while. If code has proven to be reliable, having such config options causes more friction than it provides benefit. Also, backwards compatiblity can be given up at certain points, like major releases.

The consideration of "giving users a choice" also needs to be critically considered. There are cases when giving users a choice does not make sense. For example:

  • I would argue that there is no production system which changed the default name of the security config index.
  • It is not clear to me what's the benefit of disabling system index permissions. Just toggling this switch won't have any effect, as additional permission configuration in the roles is neccessary to actually grant access to system indices.

Questions

  • Do we want some to define some guidelines on config options?
  • Do we want to have a process to retire config options?
  • Are there other considerations?

Metadata

Metadata

Assignees

No one assigned

    Labels

    triagedIssues labeled as 'Triaged' have been reviewed and are deemed actionable.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions