-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Data Source: Adding azurerm_vpn_gateway #12844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @catriona-m
Thanks for this PR - I've taken a look through and left a few comments inline but if we can fix those up then this otherwise LGTM 👍
Thanks!
Elem: &pluginsdk.Resource{ | ||
Schema: map[string]*pluginsdk.Schema{ | ||
"custom_ips": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a feeling this'll need to be a List given this is a data source (else they all get the same hash value)
}, | ||
|
||
"default_ips": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
}, | ||
|
||
"tunnel_ips": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
}, | ||
|
||
"default_ips": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Elem: &pluginsdk.Resource{ | ||
Schema: map[string]*pluginsdk.Schema{ | ||
"custom_ips": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
}, | ||
|
||
"tunnel_ips": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
name := d.Get("name").(string) | ||
resourceGroup := d.Get("resource_group_name").(string) | ||
|
||
resp, err := client.Get(ctx, resourceGroup, name) | ||
if err != nil { | ||
if utils.ResponseWasNotFound(resp.Response) { | ||
log.Printf("[DEBUG] VPN Gateway %q was not found in Resource Group %q - removing from state", name, resourceGroup) | ||
d.SetId("") | ||
return nil | ||
} | ||
|
||
return fmt.Errorf("Error retrieving VPN Gateway %q (Resource Group %q): %+v", name, resourceGroup, err) | ||
} | ||
|
||
if resp.ID != nil { | ||
d.SetId(*resp.ID) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than relying on the Azure ID, can we add a Resource ID Parser for this and use that value instead? (see this PR for an example #10128)
cdd9703
to
3e5d3a4
Compare
got an error while running the tests for this:
--- FAIL: TestAccVPNGatewayDataSource_complete (1481.37s)
|
This functionality has been released in v2.73.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. |
fixes #10703