Fetch ga4 secret from GCP IDETECT-4189 #1045
Conversation
basimSynopsys
requested review from
gopannabd,
dterrysynopsys and
niravrsynopsys
There was a problem hiding this comment.
Current skeleton code looks good to me. I understand the actual GCP service call getGa4CredentialsFromGcp will change and will use the modified hash for fetching the secret - hence I haven't added an approval just yet since this is a draft. Added a few questions I have around the jar file path formatting.
src/main/java/com/synopsys/integration/detect/workflow/phonehome/PhoneHomeSecrets.java
Outdated
Show resolved
Hide resolved
| return gson.fromJson(responseBody, PhoneHomeSecrets.class); | ||
| } | ||
|
|
||
| public static String formatFilePath(String filePath) { |
There was a problem hiding this comment.
I'm a bit unclear on how/why this formatting is needed. I'm sure you have tested it but if possible, could you provide an example of the JAR file path seen at this point?
Also how are the windows paths getting resolved? Should this check be nested under the file:/ URI check or outside, considering the path can have backslash separators \?
There was a problem hiding this comment.
Formatting is needed because the getLocation().getPath() returns the uri as -
file:/C:/Users/<username>/synopsys-detect-9.4.0-SIGQA6-SNAPSHOT.jar!/BOOT-INF/classes!/
so, we need to get rid of the !/BOOT-INF/classes!/ part to read it as file.
Also, I have removed file: and windows checks. Because if we use URI (prefixed with file: scheme) instead of plain String, Java can take care of the rest (like \ separator for windows).
This pull request includes changes required to pull the GA4 credentials from GCP service.
Currently, the function for hash modification, GCP service, and BlackduckCommonHelper are not yet changed to reflect the new functionality. So, parts where they are needed are replaced with soe mock code. However, the changes should still give a clear idea of the new control flow after the changes. A short summary of the changes is as follows:
The Mockito dependency is added to mock the GCP http behavior. It will be reverted in the final version.
Also, Detect should not stop for any phone-home related failure. I tried to make sure all phone-home related exceptions are properly handled. Still, give it a look to see whether there are any case whether phone-home failure inhibits detect from proceeding.