Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dataset Quality] Enhance Serverless FTR Tests #196599

Open
mohamedhamed-ahmed opened this issue Oct 16, 2024 · 5 comments
Open

[Dataset Quality] Enhance Serverless FTR Tests #196599

mohamedhamed-ahmed opened this issue Oct 16, 2024 · 5 comments
Labels
Feature:Dataset Health Team:obs-ux-logs Observability Logs User Experience Team

Comments

@mohamedhamed-ahmed
Copy link
Contributor

📝 Summary

Our Dataset Quality FTR serverless tests are all being run with either and Admin or Editor role which might result in false positives in the future and might hide some problems that won't show due to the high privileges given by these 2 roles.

It would be ideal to create users with the minimum privileges needed to run the tests. Previously it wasn't possible to do so on serverless until the work in this PR has been merged.

An example of how this can be implemented is provided in the PR linked above.

✔️ Acceptance Criteria

The Dataset Quality FTR serverless tests are updated so that they run without Admin users and are given only the necessary roles

@mohamedhamed-ahmed mohamedhamed-ahmed added Feature:Dataset Health Team:obs-ux-logs Observability Logs User Experience Team labels Oct 16, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@pheyos
Copy link
Member

pheyos commented Oct 17, 2024

@mohamedhamed-ahmed I think you should not rely on custom roles for serverless Observability testing.

  1. Custom roles are currently not enabled for O11y projects, so all O11y tests making use of custom roles would not be able to run against MKI and thus not be part of our release Quality Gates.
  2. Our customer don't have custom roles available in O11y, so you would not be testing the product how our customers are using it.
  3. If there's a problem with the built-in serverless O11y roles (admin, editor, viewer), then these role definitions should be fixed (instead of trying to work around the limitations in tests), because that would mean our customers can't use proper roles.

@mohamedhamed-ahmed
Copy link
Contributor Author

@pheyos Thanks for your points.

What is confusing to me is that if my understanding is correct, These are eventually going to be enabled for O11y project as well.

Our concern here is that we are running all our tests with Admin role for example which is given more privileges than what we are actually expecting for our tests to need which might result in unexpected outcome for the tests in the future.

I get your point of not having custom roles for the customers and only have (admin, editor, viewer) at the moment, but once custom roles are enabled for O11y projects in the future then we might need to amend all these tests. Please correct me if I am missing something there

@pheyos
Copy link
Member

pheyos commented Oct 18, 2024

@mohamedhamed-ahmed let me try to clarify:

These are eventually going to be enabled for O11y project as well.

Yes, maybe. And maybe not. We don't know about future decisions and I think it's important that our tests are covering the product as we have it today and not how it might look like in the future.

Our concern here is that we are running all our tests with Admin role for example which is given more privileges than what we are actually expecting for our tests to need

A totally valid concern. The tests should use the minimum required role for any action they perform. As such they should only run as admin if the action under test actually requires that role. Same for the editor and viewer roles. I can't tell you what the roles should be able to do in your application - ideally every app devloper knows which of their features require what level of privileges and adjusts the tests accordingly

which might result in unexpected outcome for the tests in the future.

I don't really get that point. As long as you don't change the required privileges for an action in the UI (e.g. an editor can or can not perform a certain action), nothing should change with test outcome.

but once custom roles are enabled for O11y projects in the future then we might need to amend all these tests

I don't think so. Similar to my point before: if you have a test that e.g. runs as an editor today and can perform certain actions, you wouldn't need to change that test just because more fine-grained roles can be created in the system. You will probably want to create additional test for custom roles, but a test like editor can do X would stay unchanged.

So my suggestion would be to

  • get your testing right for the currently existing set of roles (admin, editor, viewer) and go with the minimum required privileges for each test.
  • get your role definitions right - e.g. If you think a certain action in the UI should not be admin-only, discuss that internally and consider giving the editor role proper privileges
  • care for custom roles only when it's actually decided to introduce them - and view them as a separate feature (many customers might still stick with just the default roles we give them, so this needs to be tested properly)

@mohamedhamed-ahmed
Copy link
Contributor Author

@pheyos Thanks a million for the elaboration here.

What you are suggesting makes sense to me at this point in time, I would then postpone these modification till we see how things are changed in the future.

Also agree to the point of keeping the tests as is and add additional ones 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dataset Health Team:obs-ux-logs Observability Logs User Experience Team
Projects
None yet
Development

No branches or pull requests

3 participants