search

mattermost / mattermost-load-test

[DEPRECATED] replaced by https://github.com/mattermost/mattermost-load-test-ng
Apache License 2.0
58 stars 43 forks source link

MM-16040: Stop relying on DEPRECATED_DO_NOT_USE_EnableOnlyAdminIntegrations #149

Closed reflog closed 5 years ago

reflog commented 5 years ago

Replaced DEPRECATED_DO_NOT_USE_EnableOnlyAdminIntegrations with Role patch

Fixes: https://mattermost.atlassian.net/browse/MM-16040

reflog commented 5 years ago

@jespino, apart from the code construct, it /seems/ weird to have to permanently modify a role to test this. I admit I don't have a good working knowledge of the permission system at play here, but I'm worried about the impact of testing this on a production system and the resulting "permanency" thereof.

@lieut-data - I agree that this seems odd, but this is done all over the load-test. This change specifically just mirrors what the following code does server side https://github.com/mattermost/mattermost-server/blob/master/utils/authorization.go#L197

lieut-data commented 5 years ago

Agree that there is a certain degree of hackery in the load test, but I don't think it's the case that it munges with the definition of roles anywhere.

Maybe what I'm looking for is a way to do the changes as you've linked in a more structured way -- right now, it /feels/ like we're doing the equivalent of editing database records. @jespino, is there a first class API for accomplishing this instead?

jespino commented 5 years ago

@lieut-data I think the current approach is the existing approach, roles contains a list of "permissions", you want to add permissions to a role, you edit the role and append the new permissions to the existing list. We have that "table of config-to-permissions" for the initial migration, but the approach in the future is to add and remove the permissions directly without any "mapping" to config fields, because that fields must be removed in the future.