e2e tests CDAP-19055 - Override service account details#1002
e2e tests CDAP-19055 - Override service account details#1002itsankit-google merged 1 commit intodata-integrations:developfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). For more information, open the CLA check for this pull request. |
|
Conflicts resolved. |
| if (serviceAccountType.equalsIgnoreCase("FilePath")) { | ||
| String serviceAccountFilePath = System.getenv("SERVICE_ACCOUNT_FILE_PATH"); | ||
| if (!(serviceAccountFilePath == null) && !serviceAccountFilePath.equalsIgnoreCase("auto-detect")) { | ||
| PluginPropertyUtils.addPluginProp("serviceAccountFilePath", serviceAccountFilePath); |
There was a problem hiding this comment.
Why is this step needed if we are reading directly from env variables?
https://github.com/cdapio/cdap-e2e-tests/blob/develop/src/main/java/io/cdap/e2e/pages/actions/CdfPluginPropertiesActions.java#L591
There was a problem hiding this comment.
This is required for macro runtime arguments.
There was a problem hiding this comment.
I think the macro scenario needs to be modified, what if service account type is set as "JSON" instead of "filePath". It will fail, right?
There was a problem hiding this comment.
We have separate macro tests with Service Account Type as macro param
There was a problem hiding this comment.
What about the existing macro tests? They will fail if serviceAccountType is set as JSON in environment variable, right?
There was a problem hiding this comment.
Updated macro tests.
itsankit-google
left a comment
itsankit-google
left a comment
There was a problem hiding this comment.
The clients used to setup input data should also be created using the given credentials. Are we taking care of this as well?
|
Also there is one GCS sink test failure, please ensure |
Clients use projectId - which needs to be updated in pluginParameters.properties file as per config. |
If we don't set credentials while creating GCP API clients, they internally use the credentials from GCE VM which will be similar to auto-detect. If we want to create the clients in different projects you will need to set the service credentials which have permissions on the given project. See this for example, https://github.com/data-integrations/google-cloud/blob/develop/src/main/java/io/cdap/plugin/gcp/common/GCPUtils.java#L186 |
We can achieve this by setting env variable |
All tests passed in build |
itsankit-google
left a comment
itsankit-google
left a comment
There was a problem hiding this comment.
LGTM, please squash commits.
Squashed commits. |
2 participants
This PR depends on cdapio/cdap-e2e-tests#83
@rmstar @itsankit-google Please review.