r\shared_image: Add support for support_accelerated_network
#15562
Conversation
|
$ TF_ACC=1 go test -v ./internal/services/compute -run='TestAccSharedImage_' -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc" === RUN TestAccSharedImage_basic |
There was a problem hiding this comment.
hi @myc2h6o
Thanks for this PR.
I've taken a look through and left some comments inline, but if we can address those then we should be able to take another look.
Thanks!
| var features []compute.GalleryImageFeature | ||
| if d.Get("trusted_launch_enabled").(bool) { | ||
| features = append(features, compute.GalleryImageFeature{ | ||
| Name: utils.String("SecurityType"), | ||
| Value: utils.String("TrustedLaunch"), | ||
| }) | ||
| } |
There was a problem hiding this comment.
can we remove the expand function here and leave this as it was (adding the new field) - passing values out of d in functions makes it more likely we'll introduce bugs when refactoring etc
There was a problem hiding this comment.
Got it, moved to Create.
| if d.Get("trusted_launch_enabled").(bool) { | ||
| features = append(features, compute.GalleryImageFeature{ | ||
| Name: utils.String("SecurityType"), | ||
| Value: utils.String("TrustedLaunch"), | ||
| }) | ||
| } | ||
|
|
||
| if d.Get("support_accelerated_network").(bool) { |
There was a problem hiding this comment.
(as above) can we move this into the Create function?
| return &features | ||
| } | ||
|
|
||
| func flattenAndSetGalleryImageFeatures(d *pluginsdk.ResourceData, features *[]compute.GalleryImageFeature) { |
There was a problem hiding this comment.
(as above) can we move this into the read function - AndSet functions make it easier for bugs to be introduced
| featuresMap := make(map[string]string, len(*features)) | ||
| for _, feature := range *features { | ||
| if feature.Name != nil && feature.Value != nil { | ||
| featuresMap[*feature.Name] = *feature.Value | ||
| } | ||
| } | ||
|
|
||
| if featuresMap["SecurityType"] == "TrustedLaunch" { | ||
| trustedLaunchEnabled = true | ||
| } | ||
|
|
||
| if strings.EqualFold(featuresMap["IsAcceleratedNetworkSupported"], "true") { | ||
| supportAcceleratedNetwork = true | ||
| } |
There was a problem hiding this comment.
| featuresMap := make(map[string]string, len(*features)) | |
| for _, feature := range *features { | |
| if feature.Name != nil && feature.Value != nil { | |
| featuresMap[*feature.Name] = *feature.Value | |
| } | |
| } | |
| if featuresMap["SecurityType"] == "TrustedLaunch" { | |
| trustedLaunchEnabled = true | |
| } | |
| if strings.EqualFold(featuresMap["IsAcceleratedNetworkSupported"], "true") { | |
| supportAcceleratedNetwork = true | |
| } | |
| for _, feature := range *features { | |
| if feature.Name == nil || feature.Value == nil { | |
| continue | |
| } | |
| if strings.EqualFold(*feature.Name, "SecurityType") { | |
| trustedLaunchEnabled = strings.EqualFold(*feature.Value, "TrustedLaunch") | |
| } | |
| if strings.EqualFold(*feature.Name, "IsAcceleratedNetworkSupported") { | |
| supportAcceleratedNetwork = strings.EqualFold(*feature.Value, "true") | |
| } | |
| } |
There was a problem hiding this comment.
Updated. Looks more straightforward since we will probably not having too many features here
| @@ -160,6 +161,12 @@ func resourceSharedImage() *pluginsdk.Resource { | |||
| ForceNew: true, | |||
| }, | |||
|
|
|||
| "support_accelerated_network": { | |||
There was a problem hiding this comment.
can we document this field?
There was a problem hiding this comment.
oops, forgot the doc. Added
|
Hi @tombuildsstuff I've updated the pr, please take a look |
|
New test result after change: |
| @@ -82,6 +82,8 @@ The following arguments are supported: | |||
|
|
|||
| * `trusted_launch_enabled` - (Optional) Specifies if Trusted Launch has to be enabled for the Virtual Machine created from the Shared Image. Defaults to `false`. Changing this forces a new resource to be created. | |||
|
|
|||
| * `support_accelerated_network` - (Optional) Specifies if the Shared Image supports Accelerated Network. Defaults to `false`. Changing this forces a new resource to be created. | |||
There was a problem hiding this comment.
as this is a bool, could we change this to
| * `support_accelerated_network` - (Optional) Specifies if the Shared Image supports Accelerated Network. Defaults to `false`. Changing this forces a new resource to be created. | |
| * `accelerated_network_support_enabled` - (Optional) Specifies if the Shared Image supports Accelerated Network. Defaults to `false`. Changing this forces a new resource to be created. |
|
Test result after update |
|
Hi @katbyte I've updated the field name, could you take another look? |
|
This functionality has been released in v3.0.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fix #15441
support_accelerated_network