Load azure secret from vault in JAVA code #254
Conversation
yishi-ttd
force-pushed
the
ysh-UID2-2123-load-azure-secret-from-vault
branch
from
37e0d81 to
2637d64
Compare
| @@ -58,7 +58,7 @@ public CloudSyncOptOutStore(Vertx vertx, ICloudStorage fsLocal, JsonObject jsonC | |||
| this.remoteApiPort = -1 == url.getPort() ? 80 : url.getPort(); | |||
| this.remoteApiHost = url.getHost(); | |||
| this.remoteApiPath = url.getPath(); | |||
| this.remoteApiBearerToken = "Bearer " + jsonConfig.getString(Const.Config.OptOutApiTokenProp); | |||
| this.remoteApiBearerToken = "Bearer " + operatorKey; | |||
There was a problem hiding this comment.
Why are we changing this? It is also used directly above on line 55
There was a problem hiding this comment.
L55 is a different config.
Currently we will set the same operator key for both "OptOutApiToken" and "CoreApiToken".
The value of "OptOutApiToken" should also be fetched from vault - actually it's just operator key.
There was a problem hiding this comment.
Ok, but this would mean we are actually getting rid of the config setting for optout_api_token. In that case, we need to remove it from the code, and from all the config. This could be another ticket, but I don't think we should leave unused config settings in the code / config files
There was a problem hiding this comment.
I will remove OptOutApiTokenProp config first in this PR.
This is the only place that will use it.
There was a problem hiding this comment.
As for the places to set this env, I will create a ticket to track.
|
|
||
| @Override | ||
| public String retrieve() { | ||
| return this.config.getString(Const.Config.CoreApiTokenProp); |
There was a problem hiding this comment.
The name of the property and the class name difference will cause confusion - the OperatorKeyRetriever reads the CoreApiTokenProp? They should both be called OperatorKey or CoreApiToken
There was a problem hiding this comment.
CoreApiTokenProp(core_api_token) is an existing config in shared and I don't want to change this to break existing logic.
E.g. currently AWS/GCP will set core_api_token and optout_api_token, they could still work after this change.
| @@ -134,6 +134,7 @@ jobs: | |||
|
|
|||
| - name: Test with Trivy vulnerability scanner | |||
| uses: aquasecurity/trivy-action@master | |||
| if: inputs.publish_vulnerabilities == 'true' | |||
There was a problem hiding this comment.
We should still scan and fail on critical. We can only publish if it is a public repo, so that is why the switch exists, but we should always scan.
As this is a public repo, i don't think we should even have the publish_vulerabilities switch - we should always scan and publish
There was a problem hiding this comment.
NVM, got your point. I thought this is a follow up step of the vulnerability check, and depends on previous step's input. But it actually does the same check as Step Generate Trivy vulnerability scan report
Will revert it back.
| @@ -58,7 +58,7 @@ public CloudSyncOptOutStore(Vertx vertx, ICloudStorage fsLocal, JsonObject jsonC | |||
| this.remoteApiPort = -1 == url.getPort() ? 80 : url.getPort(); | |||
| this.remoteApiHost = url.getHost(); | |||
| this.remoteApiPath = url.getPath(); | |||
| this.remoteApiBearerToken = "Bearer " + jsonConfig.getString(Const.Config.OptOutApiTokenProp); | |||
| this.remoteApiBearerToken = "Bearer " + operatorKey; | |||
There was a problem hiding this comment.
Ok, but this would mean we are actually getting rid of the config setting for optout_api_token. In that case, we need to remove it from the code, and from all the config. This could be another ticket, but I don't think we should leave unused config settings in the code / config files
Load operator key from JAVA code, instead of shell script.