Repository navigation
feat(parametermanager): add templates, tags and checksum samples - #2337
suvidha-malaviya wants to merge 1 commit into
Conversation
Add global and regional samples for Parameter Manager templates, tags and checksum, with tests. Bump google/cloud-parametermanager to ^0.7.0 and add google/cloud-resource-manager for tag bindings.
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive PHP code samples and integration tests for Google Cloud Parameter Manager, covering both global and regional operations for parameters, templates, template versions, tag bindings, and checksum verification. The code review feedback is highly constructive, highlighting several opportunities for defensive programming. Specifically, the reviewer recommends adding null checks before calling getData() on payloads to prevent potential fatal errors, and converting format strings to uppercase to ensure robust enum value lookups.
| $payload = $parameterVersion->getPayload(); | ||
| if (hexdec(hash('crc32c', $payload->getData())) !== $payload->getDataCrc32C()) { | ||
| throw new \UnexpectedValueException(sprintf('Checksum mismatch for %s: data corrupted', $parameterVersion->getName())); | ||
| } |
There was a problem hiding this comment.
Defensive programming: The $payload variable can be null if the payload is not set or not returned. Accessing getData() on a null reference will cause a fatal error. A null check should be added before verifying the checksum.
$payload = $parameterVersion->getPayload();
if ($payload === null) {
throw new \UnexpectedValueException(sprintf('No payload found for %s', $parameterVersion->getName()));
}
if (hexdec(hash('crc32c', $payload->getData())) !== $payload->getDataCrc32C()) {
throw new \UnexpectedValueException(sprintf('Checksum mismatch for %s: data corrupted', $parameterVersion->getName()));
}| $parameterVersion = $client->getParameterVersion($request); | ||
|
|
||
| // Verify the payload checksum. | ||
| $payload = $parameterVersion->getPayload(); |
There was a problem hiding this comment.
Defensive programming: The $payload variable can be null if the payload is not set or not returned. Accessing getData() on a null reference will cause a fatal error. A null check should be added before verifying the checksum.
$payload = $parameterVersion->getPayload();
if ($payload === null) {
throw new \UnexpectedValueException(sprintf('No payload found for %s', $parameterVersion->getName()));
}
if (hexdec(hash('crc32c', $payload->getData())) !== $payload->getDataCrc32C()) {
throw new \UnexpectedValueException(sprintf('Checksum mismatch for %s: data corrupted', $parameterVersion->getName()));
}| printf('Got template version %s' . PHP_EOL, $templateVersion->getName()); | ||
| printf('Payload: %s' . PHP_EOL, $templateVersion->getPayload()->getData()); |
There was a problem hiding this comment.
Defensive programming: The $templateVersion->getPayload() call can return null if the payload is not set. Accessing getData() on a null reference will cause a fatal error. A null check should be added before accessing the payload data.
printf('Got template version %s' . PHP_EOL, $templateVersion->getName());
$payload = $templateVersion->getPayload();
printf('Payload: %s' . PHP_EOL, $payload ? $payload->getData() : '');| printf('Got regional template version %s' . PHP_EOL, $templateVersion->getName()); | ||
| printf('Payload: %s' . PHP_EOL, $templateVersion->getPayload()->getData()); |
There was a problem hiding this comment.
Defensive programming: The $templateVersion->getPayload() call can return null if the payload is not set. Accessing getData() on a null reference will cause a fatal error. A null check should be added before accessing the payload data.
printf('Got regional template version %s' . PHP_EOL, $templateVersion->getName());
$payload = $templateVersion->getPayload();
printf('Payload: %s' . PHP_EOL, $payload ? $payload->getData() : '');| printf('Template payload: %s' . PHP_EOL, $response->getPayload()->getData()); | ||
| printf('Rendered payload: %s' . PHP_EOL, $response->getRenderedPayload()); |
There was a problem hiding this comment.
Defensive programming: The $response->getPayload() call can return null if the payload is not set. Accessing getData() on a null reference will cause a fatal error. A null check should be added before accessing the payload data.
$payload = $response->getPayload();
printf('Template payload: %s' . PHP_EOL, $payload ? $payload->getData() : '');
printf('Rendered payload: %s' . PHP_EOL, $response->getRenderedPayload());| printf('Template payload: %s' . PHP_EOL, $response->getPayload()->getData()); | ||
| printf('Rendered payload: %s' . PHP_EOL, $response->getRenderedPayload()); |
There was a problem hiding this comment.
Defensive programming: The $response->getPayload() call can return null if the payload is not set. Accessing getData() on a null reference will cause a fatal error. A null check should be added before accessing the payload data.
$payload = $response->getPayload();
printf('Template payload: %s' . PHP_EOL, $payload ? $payload->getData() : '');
printf('Rendered payload: %s' . PHP_EOL, $response->getRenderedPayload());| $template = (new Template()) | ||
| ->setFormat(TemplateFormat::value('TEMPLATE_FORMAT_' . $format)); |
There was a problem hiding this comment.
Robustness: If $format is passed in lowercase or mixed case (e.g., 'json'), TemplateFormat::value() will return null or throw an exception because the enum values are uppercase. Converting $format to uppercase ensures the function is robust against different input casings.
$template = (new Template())
->setFormat(TemplateFormat::value('TEMPLATE_FORMAT_' . strtoupper($format)));| $template = (new Template()) | ||
| ->setFormat(TemplateFormat::value('TEMPLATE_FORMAT_' . $format)); |
There was a problem hiding this comment.
Robustness: If $format is passed in lowercase or mixed case (e.g., 'json'), TemplateFormat::value() will return null or throw an exception because the enum values are uppercase. Converting $format to uppercase ensures the function is robust against different input casings.
$template = (new Template())
->setFormat(TemplateFormat::value('TEMPLATE_FORMAT_' . strtoupper($format)));|
Here is the summary of changes. You are about to add 36 region tags.
This comment is generated by snippet-bot.
|
Templates (global and regional):
create_param_template/create_regional_param_template: creates a template in YAML or JSON formatcreate_param_template_version/create_regional_param_template_version: creates a template version with{{.variableName}}placeholderslist_param_templates/list_regional_param_templates: lists the templatesget_param_template/get_regional_param_template: gets a templatelist_param_template_versions/list_regional_param_template_versions: lists the versions of a templateget_param_template_version/get_regional_param_template_version: gets a template versionupdate_param_template_labels/update_regional_param_template_labels: adds or updates a label on a templatedisable_param_template_version/enable_param_template_version(and theregionalvariants): disables or enables a template versiondelete_param_template_version/delete_param_template(and theregionalvariants): deletes a template version or a templaterender_param_template_version/render_regional_param_template_version: renders a template version using the values of a parameter versiontemplate_quickstart/regional_template_quickstart: creates a template, a template version, a parameter and a parameter version, then renders the template versionTags (global and regional):
create_param_with_tags/create_regional_param_with_tags: creates a parameter with a tag (create-only)bind_tags_to_param/bind_tags_to_regional_param: creates a parameter, then binds an existing tag value to it through Resource Managerget_param_tags/get_regional_param_tags: gets the tag bindings of a parameterChecksum (global and regional):
create_param_version_with_checksum/create_regional_param_version_with_checksum: creates a parameter version with a client-computed CRC32C checksumget_param_version_verify_checksum/get_regional_param_version_verify_checksum: gets a parameter version and verifies its payload checksumBumps
google/cloud-parametermanagerto^0.7.0(the version with templates, tags and checksum support) and addsgoogle/cloud-resource-manager^1.2for tag bindings.Additional test-project requirements:
cloudresourcemanager.googleapis.com), in addition to Parameter ManagerPARAMETER_MANAGER_TAG_KEY/PARAMETER_MANAGER_TAG_VALUEenv vars (for exampletagKeys/1234567890andtagValues/1234567890). The tag tests are skipped when these are unset.