fix(compute): prevent panic in google_compute_service_attachment target_service expansion#26765
Closed
raman1236 wants to merge 1 commit intohashicorp:mainfrom
Closed
fix(compute): prevent panic in google_compute_service_attachment target_service expansion#26765raman1236 wants to merge 1 commit intohashicorp:mainfrom
raman1236 wants to merge 1 commit intohashicorp:mainfrom
Conversation
…et_service expansion When target_service is specified using a short name rather than a full self_link URL, expandComputeServiceAttachmentTargetService would panic with 'index out of range' because it split by '/' and unconditionally accessed index 3. Remove the overly strict URL validation. The value is passed through directly, consistent with the DiffSuppressFunc that already accepts both short names and full self_link URLs. Fixes hashicorp#22206
raman1236
requested review from
a team,
NickElliot,
ScottSuarez,
SirGitsalot,
c2thorn,
hao-nan-li,
melinath,
rileykarson,
roaks3,
shuyama1,
slevenick,
trodge and
zli82016
as code owners
|
This repository is generated by https://github.com/GoogleCloudPlatform/magic-modules. Any changes made directly to this repository will likely be overwritten. If you have further questions, please feel free to ping your reviewer or, internal employees, reach out to one of the engineers. Thank you! |
Collaborator
|
Hi @raman1236 - thanks for working on this! This is a generated repository, so the change will need to be made in magic-modules. This resource is generated from mmv1/products/compute/ServiceAttachment.yaml. We have a contribution guide that has more details & guidance! In particular, you may need to add custom resource code. |
Author
|
Resubmitted upstream to magic-modules: GoogleCloudPlatform/magic-modules#17390 |
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
3 participants
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.
Summary
Fixes #22206
When
target_serviceis specified using a short resource name rather than a full self_link URL,expandComputeServiceAttachmentTargetServicepanics withindex out of rangebecause it splits the value by/and unconditionally accesses index 3.Root Cause
The function used
strings.Split(v, "/")and then accessedparts[3]without a bounds check. Short resource names that were passed via the.nameattribute had fewer than 4 segments, causing a runtime panic.Fix
Removed the overly strict URL validation that required 4+ URL segments. The value is now passed through directly, which is consistent with the existing
DiffSuppressFuncthat already accepts both short names and full self_link URLs.Files Changed
google/services/compute/resource_compute_service_attachment.go— ModifiedexpandComputeServiceAttachmentTargetServicefunctionTesting
go build)go vetpasses