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

Fix: RBAC Subject and Cluster level binding #301

Conversation

ludovic-boutros
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • The commit messages are descriptive
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • An issue has been created for the pull requests. Some issues might require previous discussion.
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Fix Unable to apply subject-level RBAC bindings to cluster #285

  • What is the current behavior? (You can also link to an open issue here)
    Subject level binding uses Cluster level binding currently (which is incorrect) and cluster level binding with resources does not use the correct API endpoint.
    Therefore an error is returned.

  • What is the new behavior (if this is a feature change)?
    Subject level binding now use subject level binding and cluster level binding uses the correct API endpoint.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No

  • Other information:

IMPORTANT: Please review the CONTRIBUTING.md file for detailed contributing guidelines.
IMPORTANT: Your pull request MUST target master.

PLEASE REMOVE THIS TEMPLATE BEFORE SUBMITTING

@ludovic-boutros ludovic-boutros force-pushed the fix-rbac-cluster-binding-with-resources branch 2 times, most recently from 3efd0dc to 151cae4 Compare July 9, 2021 06:21
@lsolovey
Copy link
Contributor

FYI I found another bug in subject-level RBAC workflows: #305
It seems that current PR should fix it as well.

@ludovic-boutros
Copy link
Contributor Author

FYI I found another bug in subject-level RBAC workflows: #305
It seems that current PR should fix it as well.

Indeed. I've fixed this as well on our internal version.
I don't know if this fix should include it as well but it's possible to include it.

@purbon
Copy link
Collaborator

purbon commented Sep 11, 2021

Hi @ludovic-boutros and @lsolovey, this is Pere, the main author and maintainer (alone) of this project during my free time. I apologise for the long hiatus here, as you can totally empathy with between work, family and pandemic one can do what one can do.
I will be taking care of this PR as soon as possible. if there is any change or adaptation I see fit happy to discuss or just take it changes collaboratively

greetings.

@purbon
Copy link
Collaborator

purbon commented Sep 11, 2021

@ludovic-boutros if you don't mind I would go ahead with yours and then we can sort out with @lsolovey how to move forward together with the other fix. I hope that is ok for you.

@purbon purbon force-pushed the fix-rbac-cluster-binding-with-resources branch from 151cae4 to d77dd43 Compare September 11, 2021 06:52
@purbon
Copy link
Collaborator

purbon commented Sep 11, 2021

Hi @ludovic-boutros I would really like this addition, however when I run the integration test for rbac (sorry, they still have to be run manually.

  • start the docker/rbac-sasl examlpe
  • Add basic cluster permissions via running the script ./create-basic-roles.sh
  • $> mvn clean integration-test -Prbac

I get errors:

[ERROR] Errors:
[ERROR]   RBACPRoviderRbacIT.connectAclsCreation:217 » IO java.io.IOException: Something...
[ERROR]   RBACPRoviderRbacIT.connectClusterLevelAclCreation:328 » IO java.io.IOException...
[ERROR]   RBACPRoviderRbacIT.controlcenterAclsCreation:277 » IO java.io.IOException: Som...

All returning this exception:

java.io.IOException: java.io.IOException: Something happened with the connection, response status code: 400 body: {"status_code":400,"message":"Cannot grant resource role bindings to a cluster scoped role.","type":"CLIENT_ERROR"}

	at com.purbon.kafka.topology.clients.JulieHttpClient.doRequest(JulieHttpClient.java:124)
	at com.purbon.kafka.topology.clients.JulieHttpClient.doPost(JulieHttpClient.java:73)
	at com.purbon.kafka.topology.api.mds.MDSApiClient.bindRequest(MDSApiClient.java:96)
	at com.purbon.kafka.topology.roles.RBACProvider.createBindings(RBACProvider.java:29)
	at com.purbon.kafka.topology.actions.access.CreateBindings.execute(CreateBindings.java:28)
	at com.purbon.kafka.topology.actions.BaseAccessControlAction.run(BaseAccessControlAction.java:30)
	at com.purbon.kafka.topology.ExecutionPlan.execute(ExecutionPlan.java:112)
	at com.purbon.kafka.topology.ExecutionPlan.run(ExecutionPlan.java:90)
	at com.purbon.kafka.topology.ExecutionPlan.run(ExecutionPlan.java:84)
	at com.purbon.kafka.topology.integration.RBACPRoviderRbacIT.connectAclsCreation(RBACPRoviderRbacIT.java:217)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:54)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:99)
	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:105)
	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:40)
	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
	at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:53)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
Caused by: java.io.IOException: Something happened with the connection, response status code: 400 body: {"status_code":400,"message":"Cannot grant resource role bindings to a cluster scoped role.","type":"CLIENT_ERROR"}
	at com.purbon.kafka.topology.clients.JulieHttpClient.doRequest(JulieHttpClient.java:113)
	... 41 more

do you have any idea why this might be happening with your change? I guess we need a bit more debugging with the changes.

@purbon purbon self-requested a review September 11, 2021 09:29
@ludovic-boutros
Copy link
Contributor Author

Hi @purbon ,

Thank you for working on this.
Don't worry, I understand that open source projects are often written during free time.
I would be glad to help you :)
I will check why the integration test does not pass soon.

@purbon
Copy link
Collaborator

purbon commented Sep 17, 2021

Hi @ludovic-boutros not sure why, honestly but I don't see the error anymore when I run the test .....

https://c.tenor.com/bhFiUTubhiEAAAAC/poltergeist.gif

@purbon purbon linked an issue Sep 17, 2021 that may be closed by this pull request
Copy link
Collaborator

@purbon purbon left a comment

Choose a reason for hiding this comment

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

test failures does not show anymore in my local rbac environment. Merging and moving on! thanks a lo @ludovic-boutros for your contributino

@purbon purbon merged commit 011b0d0 into kafka-ops:master Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants