-
Notifications
You must be signed in to change notification settings - Fork 296
ParameterManagerPrivateKeyStore implementation for KeyStore using GCP Parameter Manager #3030
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
Conversation
8eb78fb to
80c4496
Compare
| private static final String ZTS_SERVICE = "zts"; | ||
| private static final String MSD_SERVICE = "msd"; | ||
|
|
||
| private static final String ATHENZ_PROP_ZMS_KEY_NAME = "athenz.aws.zms.key_name"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the key names, we should make them cloud specific. It's not right to say I have a keystore implementation for gcp but my property name is based on aws. So it should be:
"athenz." + cloud + "." + service + ".key_name"
add add the cloud as an argument to the getPrivateKeyFromCloudParameter method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Updated the code.
libs/java/server_gcp_common/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>libraries-bom</artifactId> | ||
| <version>26.64.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not hard code the version here but use the version parameter from the main pom.xml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed it
| return ParameterManagerClient.create(); | ||
| } | ||
|
|
||
| String apiEndpoint = String.format("parametermanager.googleapis.com:443", location); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is location used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oversight. Matched the non-global location endpoint with the documentation here.
https://cloud.google.com/secret-manager/parameter-manager/docs/list-parameter-versions
Still working with Google Support for testing this with real values in us-central1.
| } | ||
|
|
||
| public static boolean isGlobalLocation(String location) { | ||
| return "global".equalsIgnoreCase(location); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use "global" in a couple of places so we should probably define as a static const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| <relativePath>../../../pom.xml</relativePath> | ||
| </parent> | ||
|
|
||
| <groupId>com.yahoo.athenz</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let' define a code coverage block as well - hopefully with 100% coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
… Parameter Manager Signed-off-by: Sasi Palaka <[email protected]>
Description
Providing KeyStore implementation for GCP using Parameter Manager
Moving common code among Cloud Parameter storage into a static methond in a Util class that can be reused
Contribution Checklist: