compute: don't flatten API-provided boot disk resource_policies into an unset field - #18894
Draft
pose wants to merge 3 commits into
Draft
compute: don't flatten API-provided boot disk resource_policies into an unset field#18894pose wants to merge 3 commits into
pose wants to merge 3 commits into
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011aS2wyStqpBW8Rk2UMVLrh
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
flattenBootDiskwrites the boot disk's resource policies intoboot_disk.initialize_params.resource_policieswhenever the API reports them. The instance GET does not return the boot disk'sinitializeParams, so the values come from a GET on the disk, where policies passed inline at create time and policies attached withgoogle_compute_disk_resource_policy_attachmentare 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'sMaxItems: 1allows. The existing guard only compared againstnil, which aTypeListread 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.Diskon the error path is what panickedTestAccComputeInstance_bootDisk_storagePoolSpecifiedin 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 ofTestAccComputeInstance_diskResourcePolicies_attachmentDifffailed 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