Skip to content

compute: don't flatten API-provided boot disk resource_policies into an unset field - #18894

Draft
pose wants to merge 3 commits into
GoogleCloudPlatform:mainfrom
pose:fix-compute-instance-resource-policies
Draft

compute: don't flatten API-provided boot disk resource_policies into an unset field#18894
pose wants to merge 3 commits into
GoogleCloudPlatform:mainfrom
pose:fix-compute-instance-resource-policies

Conversation

@pose

@pose pose commented Sep 4, 2026

Copy link
Copy Markdown

Picks up #16888, which was auto-closed for inactivity. The first two commits are @corymhall's, unchanged; the third fixes the acceptance test that failed in the RECORDING run.

flattenBootDisk writes the boot disk's resource policies into boot_disk.initialize_params.resource_policies whenever the API reports them. The instance GET does not return the boot disk's initializeParams, so the values come from a GET on the disk, where policies passed inline at create time and policies attached with google_compute_disk_resource_policy_attachment are indistinguishable. An instance whose disk has policies attached out of band therefore gets them written back into a field its config never set, and the disk can hold more of them than the field's MaxItems: 1 allows. The existing guard only compared against nil, which a TypeList read never returns, so it never fired.

The guard now uses GetOkExists + tpgresource.IsEmptyValue, so that a present-but-empty value counts as unset, and it sits in the branch where the disk lookup succeeded (assigning through the nil *compute.Disk on the error path is what panicked TestAccComputeInstance_bootDisk_storagePoolSpecified in the first CI run).

An import read starts from an empty state, so the field is left unset there as well: with no config and no prior state there is nothing to say whether the policies on the disk belong to initialize_params, and importing them can put more items in the list than the schema allows. The sibling fields in the same flatten (snapshot, source_image_encryption_key, resource_manager_tags) are already empty on import for the same reason. That is what step 6 of TestAccComputeInstance_diskResourcePolicies_attachmentDiff failed on in the RECORDING run, so that step now ignores the attribute; the two earlier import steps still assert that policies coming from an attachment do not leak into the field.

Internal tests cover the create, refresh and import shapes of the read plus the disk-lookup error path. All four unset-field cases fail against the old guard.

Context: pulumi/pulumi-gcp#3666

compute: fixed `google_compute_instance` boot disk `initialize_params.resource_policies` being populated from the API when the field is not set in the config

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 4, 2026
@google-cla

google-cla Bot commented Sep 4, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-approval Pull requests that need reviewer's approval to run presubmit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants