Skip to content

Conversation

@albertzaharovits
Copy link
Contributor

@albertzaharovits albertzaharovits commented Jul 29, 2024

This indexes the built-in reserved roles into the .security index, in order to be visible to the query-roles action.

At a high level, the implementation indexes the built-in reserved roles (32 of them) when the query-roles API is invoked (and they're not already indexed).

In order to distinguish when the built-in roles have changed (i.e. edited in the code), so that they need to be indexed again, a new metadata field _version is added to every one of them. The _version field for a reserved role needs to be incremented whenever the role is changed. The roles are automatically indexed again (upon calling the query-roles endpoint) if the role _version in the definition in code is newer than the _version from the indexed role.

The versions of indexed built-in roles are stored in a map at the .security index metadata (custom) level. A new transport action is implemented that sets the .security index metadata. This action is called after all the built-in roles have been successfully indexed. The versions map of indexed built roles is exposed for read by the SecurityIndexManager.

Some unintuitive consequences of the implementation:

  • the query-roles API now technically requires the .security index be available for write (only initially) - previously, more intuitively, the requirement was that it be available for search only
  • if the .security index does not exist, the current implementation doesn't create the index, it will simply return empty results (as soon as the index is created, by e.g. creating a native. role, the query roles API will be seeing 1 + 32 roles - we can change this behavior to instead create the .security index when calling the query-roles API)
  • roles defined in files are still not visible in the API
  • in a mixed versions cluster, during a rolling upgrade, after some changes to built-in roles, if the query-roles API hits the newer node, the newer role versions will be indexed and be made available to the query-roles API, but the old versions of the roles are still effective on the old cluster nodes (though querying the roles on old cluster nodes returns the new roles)


public class SetIndexMetadataPropertyAction extends ActionType<SetIndexMetadataPropertyResponse> {

public static final String NAME = "indices:internal/admin/metadata/custom/set";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New transport action to set any metadata custom property on a single index.
This is used to "mark" the .security index after the built-in reserved roles have been indexed.

Tuple<ClusterState, Map<String, String>> execute(ClusterState state) {
IndexMetadata indexMetadata = state.metadata().getIndexSafe(index);
Map<String, String> existingValue = indexMetadata.getCustomData(key);
if (Objects.equals(expected, existingValue)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The action works as a compare-and-swap rather than a simple write.
I think this greatly improves the non-happy path handling when calling this action.

return RESERVED_ROLES.values();
}

public static Map<String, String> versionMap() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A map of reserved role name to version.

/* !!!!!! IMPORTANT !!!!!!!
* reserved role version must be incremented when builtin reserved roles are changed
*/
public static final String RESERVED_ROLE_VERSION_VALUE = "2";
Copy link
Contributor Author

@albertzaharovits albertzaharovits Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the version so that we index, but not overwrite, built-in reserved roles after they're changed in code.

listener::onFailure
)
);
nativeRolesStore.ensureBuiltinRolesAreQueriable(ActionListener.wrap(onResponse -> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The built-in reserved roles are maybe indexed just before the query roles action is executed.
Normally, the query-roles action is a "read" type of action, but this change makes it a possible write.

listener::onFailure
)
);
nativeRolesStore.ensureBuiltinRolesAreQueriable(ActionListener.wrap(onResponse -> {
Copy link
Contributor Author

@albertzaharovits albertzaharovits Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The built-in reserved roles are potentially indexed just before the query roles action is executed.
Before, the query-roles action is a "read" type of action, but this change makes it a possible write.

Comment on lines +551 to +560
} else if (frozenSecurityIndex.areReservedRolesIndexed()) {
logger.debug("security index already contains the latest reserved roles indexed");
listener.onResponse(null);
} else {
indexReservedRoles(
frozenSecurityIndex,
ActionListener.wrap(onResponse -> frozenSecurityIndex.markReservedRolesAsIndexed(listener), listener::onFailure)
);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if the index metadata custom map indicates if the built-in reserved roles have already been indexed. If not, they are, and then the index metadata is marked as such.

Comment on lines +506 to +510
for (String roleName : reservedRolesVersions.keySet()) {
if (Integer.parseInt(reservedRolesVersions.get(roleName)) > Integer.parseInt(reservedRolesVersionMap.get(roleName))) {
return false;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If indexed roles are "newer" compared to the local version, the reserved roles must have been changed in the code, and a newer node indexed them in this mixed cluster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants