add HTTP plugin support for service account authentication #60
Conversation
| @@ -82,7 +82,7 @@ | |||
| <gson.version>2.8.5</gson.version> | |||
| <hadoop.version>2.3.0</hadoop.version> | |||
| <httpcomponents.version>4.5.9</httpcomponents.version> | |||
| <hydrator.version>2.4.0-SNAPSHOT</hydrator.version> | |||
| <hydrator.version>2.3.0-SNAPSHOT</hydrator.version> | |||
There was a problem hiding this comment.
Why did we have to go to an older version?
| @@ -21,7 +21,7 @@ | |||
| <name>HTTP Plugins</name> | |||
| <groupId>io.cdap</groupId> | |||
| <artifactId>http-plugins</artifactId> | |||
| <version>1.4.0-SNAPSHOT</version> | |||
| <version>1.4.1-service-account</version> | |||
There was a problem hiding this comment.
When we publish in the hub we would change this, right?
| @@ -93,6 +93,16 @@ | |||
| </properties> | |||
|
|
|||
| <dependencies> | |||
| <dependency> | |||
| <groupId>com.google.guava</groupId> | |||
| <artifactId>guava</artifactId> | |||
There was a problem hiding this comment.
Do we know if this could break any other plugins due to conflicting dependencies?
| @@ -21,7 +21,7 @@ | |||
| <name>HTTP Plugins</name> | |||
| <groupId>io.cdap</groupId> | |||
| <artifactId>http-plugins</artifactId> | |||
| <version>1.4.0-SNAPSHOT</version> | |||
| <version>1.4.1-service-account</version> | |||
There was a problem hiding this comment.
Please revert the version change heree. We'll bump the version on the release branch after cherrypicking your PR.
| @@ -82,7 +82,7 @@ | |||
| <gson.version>2.8.5</gson.version> | |||
| <hadoop.version>2.3.0</hadoop.version> | |||
| <httpcomponents.version>4.5.9</httpcomponents.version> | |||
| <hydrator.version>2.4.0-SNAPSHOT</hydrator.version> | |||
| <hydrator.version>2.3.0-SNAPSHOT</hydrator.version> | |||
There was a problem hiding this comment.
Why did we change this. Also, we probably shouldn't be depending on a snapshot version here.
|
|
||
| /** | ||
| * A class which contains utilities to make OAuth2 specific calls. | ||
| */ | ||
| public class OAuthUtil { | ||
|
|
||
| public static PrivateKey readPKCS8Pem(String key) throws Exception { | ||
| key = key.replace("-----BEGIN PRIVATE KEY-----", ""); |
There was a problem hiding this comment.
Please fix the indentation. See https://wiki.cask.co/display/CE/Coding+Standard
|
I have proposed a more industrialized implementation of SA Authentication here. The main difference with this PR is that I used |
The issue here is that the oauth provided in the plugin doesn't have any support for service account users. It has a set of config parameters to enter, including a refresh token. That token is used to request a fresh access token when doing oauth for users. For a service account, there's no way to make a refresh token. Instead, a service account uses a private key and other details that are given in the service account json keyfile. The customer needs a way to use the http plugin using that data for a service account. It's taking more work than planned, but I have code working for a custom http plugin adding auth with a service account json key. Using this, I made a test fusion instance pipeline that calls the GCE API to list my compute engine instances and then writes that data to a GCS bucket. Authentication is done with a service account json keyfile given in the configuration for the http plugin. I'll send an update to the customer to find out if this is all they need to unblock. There are several things still to implement after this proof of concept change to the plugin. I've updated the batch source plugin, but that leave the streaming source plugin and both stream and batch for the sink output plugin. The changes there are the same as for the batch source code.
The code took a long time to finish and test because I ran into a lot of dependency conflicts with guava in the pipeline code and the current versions that the google oauth packages expect. Doing Base64 parsing and fetching web page results failed and needed local workarounds. If there's some way to clean up the code and avoid those parts of the code, it would be much simpler to read.