feat(parametermanager): Added samples for kms_key field in global parameter#10093
Conversation
|
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
There was a problem hiding this comment.
Hello @durgesh-ninave-crest, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request to help everyone get up to speed quickly.
This PR, authored by @durgesh-ninave-crest, focuses on enhancing the parametermanager samples by adding examples demonstrating how to interact with the kms_key field for global parameters. Specifically, it introduces samples for creating a parameter with a KMS key, updating a parameter's KMS key, and removing a parameter's KMS key. The changes include adding new sample classes and corresponding integration tests to ensure the functionality works as expected.
Highlights
- New Samples for KMS Key Management: Three new sample classes (
CreateParamWithKmsKey.java,UpdateParamKmsKey.java,RemoveParamKmsKey.java) have been added to demonstrate how to manage the KMS key associated with a global parameter using the Parameter Manager SDK. - Updated Dependencies: The
pom.xmlfile has been updated to use a newer version of thelibraries-bomand includes a new test dependency ongoogle-cloud-kms. - Integration Test Coverage: The integration test suite (
SnippetsIT.java) has been significantly updated to include tests for the new KMS key samples. This involves adding necessary imports, setting up KMS resources (key rings, HSM keys) in the test setup, cleaning them up afterwards, adding helper methods for KMS resource management, and implementing the actual test methods for creating, updating, and removing KMS keys on parameters.
Changelog
Click here to see the changelog
- parametermanager/pom.xml
- Updated
libraries-bomversion from 26.54.0 to 26.60.0. - Added
google-cloud-kmsas a test dependency.
- Updated
- parametermanager/src/main/java/parametermanager/CreateParamWithKmsKey.java
- Added new sample class
CreateParamWithKmsKey. - Provides a
mainmethod and acreateParameterWithKmsKeymethod to create a global parameter and associate it with a specified KMS key.
- Added new sample class
- parametermanager/src/main/java/parametermanager/RemoveParamKmsKey.java
- Added new sample class
RemoveParamKmsKey. - Provides a
mainmethod and aremoveParamKmsKeymethod to remove the KMS key association from an existing global parameter using a field mask.
- Added new sample class
- parametermanager/src/main/java/parametermanager/UpdateParamKmsKey.java
- Added new sample class
UpdateParamKmsKey. - Provides a
mainmethod and anupdateParamKmsKeymethod to update the KMS key associated with an existing global parameter using a field mask.
- Added new sample class
- parametermanager/src/test/java/parametermanager/SnippetsIT.java
- Added imports for KMS-related classes and utilities (
CryptoKey,KeyManagementServiceClient,FieldMask, etc.). - Added static variables to hold test parameter names and KMS resource IDs for the new tests.
- Modified the
beforeAllmethod to set up KMS key rings and HSM keys, and create initial parameters with KMS keys for the update and delete tests. - Modified the
afterAllmethod to clean up the parameters created for the new tests and also clean up the KMS key versions and keys. - Added helper methods
getKeyRingName,getLocationName,createParameterWithKms,createKeyRing, andcreateHsmKeyfor managing test resources. - Added three new test methods:
testCreateParamWithKmsKey,testUpdateParamKmsKey, andtestRemoveParamKmsKeyto verify the functionality of the new samples.
- Added imports for KMS-related classes and utilities (
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A parameter, a key,
Secrets kept for all to see.
With KMS they're bound,
Securely on the ground,
Safe from prying eyes, you agree?
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces new samples for managing KMS keys associated with global parameters in the Parameter Manager SDK. The additions include creating a parameter with a KMS key, updating a parameter's KMS key, and removing a KMS key from a parameter. Corresponding integration tests have also been added.
The code for the samples is clear and demonstrates the intended functionality well. The integration tests cover the new scenarios, and the setup/teardown logic for KMS resources is mostly robust.
I've identified a couple of medium-severity issues that should be addressed for improved code quality and test hygiene. Other minor points are noted in the summary but not as direct comments due to review settings.
Summary of Findings
- Copyright Year: The copyright year in new Java sample files (
CreateParamWithKmsKey.java,UpdateParamKmsKey.java,RemoveParamKmsKey.java) is set to2025and should be updated to the current year. (Severity: Medium) - KMS CryptoKey Resource Management in Tests: In
SnippetsIT.java,CryptoKeyresources are created with random IDs for each test run but are not deleted inafterAll(only their versions are destroyed). This leads to an accumulation ofCryptoKeyentities. Suggested fix: use fixed key IDs and modifycreateHsmKeyto handleAlreadyExistsException. (Severity: Medium) - Unused Imports: The files
parametermanager/src/main/java/parametermanager/RemoveParamKmsKey.javaandparametermanager/src/main/java/parametermanager/UpdateParamKmsKey.javacontain an unused import forcom.google.cloud.parametermanager.v1.ParameterManagerSettings. (Severity: Low - Not commented directly due to review settings) - Javadoc Clarity/Accuracy: Minor imprecisions in Javadoc comments within
CreateParamWithKmsKey.java(method Javadoc) andRemoveParamKmsKey.java(class and method Javadoc) could be refined for better accuracy. (Severity: Low - Not commented directly due to review settings) - Javadoc Typo: A small typo ("theParameter") exists in the class Javadoc of
parametermanager/src/main/java/parametermanager/UpdateParamKmsKey.java. (Severity: Low - Not commented directly due to review settings) - Trailing Space in Comment: A trailing space was found in a code comment in
parametermanager/src/main/java/parametermanager/RemoveParamKmsKey.javaat line 55. (Severity: Low - Not commented directly due to review settings)
Merge Readiness
The pull request adds valuable samples for KMS key management with Parameter Manager. However, there are a couple of medium-severity issues related to the copyright year in the new files and KMS resource management in the integration tests that should be addressed before merging. Addressing these will improve code consistency and test environment hygiene. I am not authorized to approve pull requests, so please ensure these changes are reviewed and approved by a maintainer.
| @@ -0,0 +1,66 @@ | |||
| /* | |||
| * Copyright 2025 Google LLC | |||
There was a problem hiding this comment.
The copyright year is set to 2025. Could you please update this to the current year of creation (e.g., 2024)? This should also be checked for the other new sample files (UpdateParamKmsKey.java, RemoveParamKmsKey.java).
| * Copyright 2025 Google LLC | |
| * Copyright 2024 Google LLC |
| private static CryptoKey createHsmKey(String keyId) throws IOException { | ||
| try (KeyManagementServiceClient client = KeyManagementServiceClient.create()) { | ||
| CryptoKey key = | ||
| CryptoKey.newBuilder() | ||
| .setPurpose(CryptoKey.CryptoKeyPurpose.ENCRYPT_DECRYPT) | ||
| .setVersionTemplate( | ||
| CryptoKeyVersionTemplate.newBuilder() | ||
| .setAlgorithm(CryptoKeyVersion | ||
| .CryptoKeyVersionAlgorithm | ||
| .GOOGLE_SYMMETRIC_ENCRYPTION) | ||
| .setProtectionLevel(ProtectionLevel.HSM) | ||
| .build()) | ||
| .putLabels("foo", "bar") | ||
| .putLabels("zip", "zap") | ||
| .build(); | ||
| CryptoKey createdKey = client.createCryptoKey(getKeyRingName(), keyId, key); | ||
| return createdKey; | ||
| } |
There was a problem hiding this comment.
The current approach of creating HSM keys using HSM_KEY_ID = randomId() (e.g., at lines 190, 197, 208) in beforeAll() and then calling createHsmKey(HSM_KEY_ID) results in new CryptoKey entities being created in KMS with each full test suite execution. While the afterAll() method cleans up CryptoKeyVersions, it doesn't remove the CryptoKey entities themselves. This can lead to an accumulation of CryptoKey resources in the test project's KMS key ring (test-parameter-manager-snippets) over time, potentially causing clutter or hitting quotas.
To improve test hygiene and resource management, consider the following:
- Use fixed, predefined string constants for your test
CryptoKeyIDs instead ofrandomId().
Example inbeforeAll:private static final String TEST_HSM_KEY_ID_1 = "pm-test-key-for-create"; private static final String TEST_HSM_KEY_ID_FOR_UPDATE_OLD = "pm-test-key-for-update-old"; private static final String TEST_HSM_KEY_ID_FOR_UPDATE_NEW = "pm-test-key-for-update-new"; // ... HSM_KEY_ID = TEST_HSM_KEY_ID_1; // for create test // ... HSM_KEY_ID = TEST_HSM_KEY_ID_FOR_UPDATE_OLD; // for update test initial key NEW_HSM_KEY_ID = TEST_HSM_KEY_ID_FOR_UPDATE_NEW; // for update test new key
- Modify this
createHsmKeymethod to handlecom.google.api.gax.rpc.AlreadyExistsException. If the key already exists (which it would on subsequent runs with fixed IDs), retrieve and return the existing key instead of trying to create it again.
This approach ensures that a fixed set of CryptoKey resources are reused across test runs, and the existing version cleanup logic in afterAll will apply to these persistent keys.
Description
Added samples for kms_key field in global parameter using Parameter manager SDK
Sample List (global):
Added required tests for the same.
Checklist
pom.xmlparent set to latestshared-configurationmvn clean verifyrequiredmvn -P lint checkstyle:checkrequiredmvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory only