-
Notifications
You must be signed in to change notification settings - Fork 552
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
RBAC End to End Integration Test #17219
Conversation
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46631#018e67b1-4665-48ac-8044-df02882851ea ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46631#018e67c2-f4ba-4ba0-b8c4-f1d95398fb12 |
tests/rptest/tests/rbac_test.py
Outdated
try: | ||
res = self.superuser_admin.list_role_members(role=role) | ||
return user in RoleMemberList.from_response(res) | ||
except: | ||
return 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.
fyi: you can avoid the try/except ceremony to return False by passing retry_on_exc=True
to wait_until
and it will treat exceptions as if the predicate returned False
.
force push to prefer |
/ci-repeat 1 |
/cdt |
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.
lgtm
tests/rptest/tests/rbac_test.py
Outdated
|
||
def has_topics(self, client: RpkTool): | ||
tps = client.list_topics() | ||
return tps != [], [i for i in tps] |
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.
nitpick: second return value could just be tps
or maybe list(tps)
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.
list(tps)
works!
tests/rptest/tests/rbac_test.py
Outdated
backoff_sec=1) | ||
res = wait_until_result( | ||
lambda: | ||
(True, self.superuser_admin.list_role_members(role=self.role_name0) |
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.
question(non-blocking): do we need the True
here or would this work without it as well? Do we just need it for wait_until_result
to forward the result?
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.
Nice, I hadn't noticed that in the wait_until_result
doc string. I thought the tuple-ness was required.
force push to rebase on dev |
force push review suggestions |
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
empty force push for signoffs |
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.
lgtm
def sasl_allow_principal(self, *args, **kwargs): | ||
self._sasl_set_principal_access(*args, **kwargs, deny=False) | ||
|
||
def sasl_deny_principal(self, *args, **kwargs): | ||
self._sasl_set_principal_access(*args, **kwargs, deny=True) |
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.
praise: ❤️
Closes https://github.com/redpanda-data/core-internal/issues/1110
Backports Required
Release Notes