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

[Functions] Provide an interface for functions worker service #8560

Merged
merged 25 commits into from
Dec 14, 2020

Conversation

sijie
Copy link
Member

@sijie sijie commented Nov 13, 2020

Motivation

Make the pulsar functions worker serve as an interface to allow plugin different functions of worker service implementations.

@sijie sijie added this to the 2.7.0 milestone Nov 13, 2020
@sijie sijie self-assigned this Nov 13, 2020
@sijie sijie marked this pull request as ready for review November 17, 2020 20:51
*Motivation*

The script was broken after we upgraded the `netty-tc-native` version to `2.0.33.Final`.

This change fixes the script to make sure it is able to run on MacOS.
*Motivation*

Use the same gson version as kubernetes client does to fix version conflict issue.

```
ERROR] testAuthProviderWrongInterface(org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactoryTest)  Time elapsed: 0.07 s  <<< FAILURE!
org.testng.TestException:

The exception was thrown with the wrong message: expected "Function authentication provider.*.must implement KubernetesFunctionAuthProvider" but got "com.google.gson.JsonParser.parseReader(Ljava/io/Reader;)Lcom/google/gson/JsonElement;"
	at org.testng.internal.ExpectedExceptionsHolder.wrongException(ExpectedExceptionsHolder.java:70)
	at org.testng.internal.TestInvoker.considerExceptions(TestInvoker.java:737)
	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:635)
	at org.testng.internal.TestInvoker.retryFailed(TestInvoker.java:214)
	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:58)
	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.testng.TestRunner.privateRun(TestRunner.java:764)
	at org.testng.TestRunner.run(TestRunner.java:585)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
	at org.testng.TestNG.runSuites(TestNG.java:1069)
	at org.testng.TestNG.run(TestNG.java:1037)
	at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:135)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeSingleClass(TestNGDirectoryTestSuite.java:112)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeLazy(TestNGDirectoryTestSuite.java:123)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:90)
	at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:146)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
Caused by: java.lang.NoSuchMethodError: com.google.gson.JsonParser.parseReader(Ljava/io/Reader;)Lcom/google/gson/JsonElement;
	at io.kubernetes.client.util.KubeConfig.runExec(KubeConfig.java:330)
	at io.kubernetes.client.util.KubeConfig.tokenViaExecCredential(KubeConfig.java:269)
	at io.kubernetes.client.util.KubeConfig.getAccessToken(KubeConfig.java:231)
	at io.kubernetes.client.util.credentials.KubeconfigAuthentication.<init>(KubeconfigAuthentication.java:46)
	at io.kubernetes.client.util.ClientBuilder.kubeconfig(ClientBuilder.java:276)
	at io.kubernetes.client.util.ClientBuilder.getClientBuilder(ClientBuilder.java:108)
	at io.kubernetes.client.util.ClientBuilder.standard(ClientBuilder.java:87)
	at io.kubernetes.client.util.ClientBuilder.standard(ClientBuilder.java:79)
	at io.kubernetes.client.util.Config.defaultClient(Config.java:113)
	at org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactory.setupClient(KubernetesRuntimeFactory.java:339)
	at org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactory.initialize(KubernetesRuntimeFactory.java:208)
	at org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactoryTest.createKubernetesRuntimeFactory(KubernetesRuntimeFactoryTest.java:184)
	at org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactoryTest.testAuthProvider(KubernetesRuntimeFactoryTest.java:232)
	at org.apache.pulsar.functions.runtime.kubernetes.KubernetesRuntimeFactoryTest.testAuthProviderWrongInterface(KubernetesRuntimeFactoryTest.java:243)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
	at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
	at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
	at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
```
@sijie
Copy link
Member Author

sijie commented Nov 18, 2020

/pulsarbot run-failure-checks

@sijie
Copy link
Member Author

sijie commented Nov 20, 2020

/pulsarbot run-failure-checks

@codelipenghui codelipenghui modified the milestones: 2.7.0, 2.8.0 Nov 21, 2020
@sijie
Copy link
Member Author

sijie commented Dec 10, 2020

/pulsarbot run-failure-checks

Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

LGTM +1

@sijie sijie merged commit 26749a8 into apache:master Dec 14, 2020
sijie pushed a commit that referenced this pull request May 4, 2021
---

Fixes #10332

*Motivation*

Sometimes, the superuser is not only needed the client
role but also needs some data in the authentication data
to check whether the role is the superuser.

The changed interface is introduced from #8560,
it's safe to change it since there is no release for that.
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request May 11, 2021
)

---

Fixes apache#10332

*Motivation*

Sometimes, the superuser is not only needed the client
role but also needs some data in the authentication data
to check whether the role is the superuser.

The changed interface is introduced from apache#8560,
it's safe to change it since there is no release for that.
zymap added a commit to zymap/pulsar that referenced this pull request Jul 22, 2021
---

*Motivation*

Port the fix apache#10364 to the branch 2.7.

Sometimes, the superuser is not only needed the client
role but also needs some data in the authentication data
to check whether the role is the superuser.

The changed interface is introduced from apache#8560,
it's safe to change it since there is no release for that.'

*Modifications*

Change the isSuperUser method to accept the authenticationData
and pass it to the authentication provider. So authenticatoin prodiver
can get the data saved in the authenticationDataSource.
zymap added a commit that referenced this pull request Jul 22, 2021
…er (#11418)

---

*Motivation*

Port the fix #10364 to the branch 2.7.

Sometimes, the superuser is not only needed the client
role but also needs some data in the authentication data
to check whether the role is the superuser.

The changed interface is introduced from #8560,
it's safe to change it since there is no release for that.'

*Modifications*

Change the isSuperUser method to accept the authenticationData
and pass it to the authentication provider. So authenticatoin prodiver
can get the data saved in the authenticationDataSource.
jerrypeng pushed a commit to jerrypeng/incubator-pulsar that referenced this pull request Nov 4, 2021
)

---

Fixes apache#10332

*Motivation*

Sometimes, the superuser is not only needed the client
role but also needs some data in the authentication data
to check whether the role is the superuser.

The changed interface is introduced from apache#8560,
it's safe to change it since there is no release for that.
michaeljmarshall added a commit that referenced this pull request Apr 7, 2023
…19975)

### Motivation

The current function worker interfaces introduced by #8560 are hard to extend. It is better to pass an object that we can add fields to as needed.

### Modifications

* Introduce a new `Authentication` class to wrap all of the relevant authentication "data". This class is somewhat unfortunate in that we already have many classes that hold authentication information. However, my goal with this class is to wrap all of the relevant auth info. This class is only currently used in the Function Worker API, but I plan to add it to the rest of the Admin HTTP endpoints.
* Deprecate all of the methods that are no longer needed. Add a default implementation to call the new methods that supersede those methods.
* Add `proxyRoles` setting to function worker.

### Verifying this change

There are tests to cover the changes.

### Does this pull request potentially affect one of the following parts:

This PR changes several interfaces in completely backwards compatible ways. It's possible we'll want to improve the abstraction for the `Authentication` class by making it an interface.

### Documentation

- [x] `doc`

This change has Javadoc updates to document the changes.

### Matching PR in forked repository

PR in forked repository: michaeljmarshall#37
michaeljmarshall added a commit that referenced this pull request Apr 8, 2023
…19975)

The current function worker interfaces introduced by #8560 are hard to extend. It is better to pass an object that we can add fields to as needed.

* Introduce a new `Authentication` class to wrap all of the relevant authentication "data". This class is somewhat unfortunate in that we already have many classes that hold authentication information. However, my goal with this class is to wrap all of the relevant auth info. This class is only currently used in the Function Worker API, but I plan to add it to the rest of the Admin HTTP endpoints.
* Deprecate all of the methods that are no longer needed. Add a default implementation to call the new methods that supersede those methods.
* Add `proxyRoles` setting to function worker.

There are tests to cover the changes.

This PR changes several interfaces in completely backwards compatible ways. It's possible we'll want to improve the abstraction for the `Authentication` class by making it an interface.

- [x] `doc`

This change has Javadoc updates to document the changes.

PR in forked repository: michaeljmarshall#37

(cherry picked from commit 55acbe6)
michaeljmarshall added a commit to michaeljmarshall/pulsar that referenced this pull request Apr 8, 2023
…pache#19975)

The current function worker interfaces introduced by apache#8560 are hard to extend. It is better to pass an object that we can add fields to as needed.

* Introduce a new `Authentication` class to wrap all of the relevant authentication "data". This class is somewhat unfortunate in that we already have many classes that hold authentication information. However, my goal with this class is to wrap all of the relevant auth info. This class is only currently used in the Function Worker API, but I plan to add it to the rest of the Admin HTTP endpoints.
* Deprecate all of the methods that are no longer needed. Add a default implementation to call the new methods that supersede those methods.
* Add `proxyRoles` setting to function worker.

There are tests to cover the changes.

This PR changes several interfaces in completely backwards compatible ways. It's possible we'll want to improve the abstraction for the `Authentication` class by making it an interface.

- [x] `doc`

This change has Javadoc updates to document the changes.

PR in forked repository: #37

(cherry picked from commit 55acbe6)
(cherry picked from commit 54c97ad)
michaeljmarshall added a commit to michaeljmarshall/pulsar that referenced this pull request Apr 8, 2023
…pache#19975)

The current function worker interfaces introduced by apache#8560 are hard to extend. It is better to pass an object that we can add fields to as needed.

* Introduce a new `Authentication` class to wrap all of the relevant authentication "data". This class is somewhat unfortunate in that we already have many classes that hold authentication information. However, my goal with this class is to wrap all of the relevant auth info. This class is only currently used in the Function Worker API, but I plan to add it to the rest of the Admin HTTP endpoints.
* Deprecate all of the methods that are no longer needed. Add a default implementation to call the new methods that supersede those methods.
* Add `proxyRoles` setting to function worker.

There are tests to cover the changes.

This PR changes several interfaces in completely backwards compatible ways. It's possible we'll want to improve the abstraction for the `Authentication` class by making it an interface.

- [x] `doc`

This change has Javadoc updates to document the changes.

PR in forked repository: #37

(cherry picked from commit 55acbe6)
(cherry picked from commit 54c97ad)
(cherry picked from commit cf40926)
michaeljmarshall added a commit to datastax/pulsar that referenced this pull request Apr 11, 2023
…pache#19975)

The current function worker interfaces introduced by apache#8560 are hard to extend. It is better to pass an object that we can add fields to as needed.

* Introduce a new `Authentication` class to wrap all of the relevant authentication "data". This class is somewhat unfortunate in that we already have many classes that hold authentication information. However, my goal with this class is to wrap all of the relevant auth info. This class is only currently used in the Function Worker API, but I plan to add it to the rest of the Admin HTTP endpoints.
* Deprecate all of the methods that are no longer needed. Add a default implementation to call the new methods that supersede those methods.
* Add `proxyRoles` setting to function worker.

There are tests to cover the changes.

This PR changes several interfaces in completely backwards compatible ways. It's possible we'll want to improve the abstraction for the `Authentication` class by making it an interface.

- [x] `doc`

This change has Javadoc updates to document the changes.

PR in forked repository: michaeljmarshall#37

(cherry picked from commit 55acbe6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants