-
-
Notifications
You must be signed in to change notification settings - Fork 776
fixing enterprise ldap issue with execution view #4759
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
fixing enterprise ldap issue with execution view #4759
Conversation
Make sure we only generate coverage for runners and orquesta integration tests when ENABLE_COVERAGE environment variable is set to "yes". Coverage adds a lot of overhead so this should speed up PR builds.
invocation basis. Update tests which have a race / rely on timing to use longer wait time to avoid failure.
nightly build. This should substantially speed up PR builds.
"hosts_blacklist" runner attribute.
4b0d69e to
0abe410
Compare
|
Thanks for your contribution @jdmeyer3 , we will review it and merge. |
46c3c93 to
3cf45d3
Compare
3cf45d3 to
8fb27e7
Compare
"url_hosts_blacklist" and also add support for whitelist approach using "url_hosts_whitelist" runner parameter.
Previously we don't had test cases for HTTPClient class, but not the actual runner.
part of the nightly builds now.
are specified, but not all of them have corresponding nightly tasks. For example: TASK="foo1 foo2 foo3" and only "foo2-nightly" task exists. In this case, only "foo2-nightly" would run and other would be ignored.
Also enable it so we can test it's working.
…_blacklist Add support for blacklisting hosts to the HTTP runner
…_nighly_build Pattern for running particular Travis tasks as part of a nightly build
| 'runner.runner_parameters', | ||
| 'parameters' | ||
| 'parameters', | ||
| # necessary for ActionExecutionDB to determine permissions in enterprise ldap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, we could only include that if rbac is enabled, but since those attributes are quite small, it doesn't hurt to include them in all the responses...
|
Thanks for contributing this bug fix. I will go ahead and add a corresponding test case to our proprietary RBAC code base. |
|
While working on the tests, I noticed there is another bug hiding in there. When, |
…ithub.com/jdmeyer3/st2 into jdmeyer3-bug/enterprise_ldap_view_executions_error
works when RBAC is enabled. When RBAC is enabled, we simply use admin use when testing include and exclude attributes functionality.
|
And to clarify / add some context - I believe this issue would only affect WebUI by default, but not CLI, since WebUI tries to retrieve less fields (only the ones it needs) to speed things up. |
works when RBAC is enabled. When RBAC is enabled, we simply use admin use when testing include and exclude attributes functionality.
…m:jdmeyer3/st2 into bug/enterprise_ldap_view_executions_error
| test_exact_object_count = True | ||
|
|
||
| # True if those tests are running with rbac enabled | ||
| rbac_enabled = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this value set to True for the tests below?
|
|
||
| def test_get_all_exclude_attributes_and_include_attributes_are_mutually_exclusive(self): | ||
| if self.rbac_enabled: | ||
| self.use_user(self.users['admin']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain where self.rbac_enabled is set to True and also how is adding the user here test the changes?
Closes #4758
The error caused in #4758 is because the ldap rbac is expecting the document returned from the ActionExecutionDB to contain action.pack and action.uid, but they are currently not being returned.
Here is the traceback I got from the rbac
and here is the action object that is returned when it gets the key error
By having mongo return the action.pack, and the action.uid, it should satisfy the st2rbac requirements
