New Data Source: azurerm_network_interface #854
Conversation
There was a problem hiding this comment.
hey @dcaro
Thanks for this PR - I've taken a look through and this mostly LGTM; if we can fix up the highlighted issues (and the tests pass) this should be good to merge :)
Thanks!
|
|
||
| "location": { | ||
| Type: schema.TypeString, | ||
| Optional: true, |
There was a problem hiding this comment.
since this is a data source (and can't be specified), we can just make this Computed: true
|
|
||
| "network_security_group_id": { | ||
| Type: schema.TypeString, | ||
| Optional: true, |
There was a problem hiding this comment.
(as above) can we remove the optional field here?
|
|
||
| "mac_address": { | ||
| Type: schema.TypeString, | ||
| Optional: true, |
There was a problem hiding this comment.
(as above) can we remove the optional field here?
|
|
||
| "virtual_machine_id": { | ||
| Type: schema.TypeString, | ||
| Optional: true, |
There was a problem hiding this comment.
(as above) can we remove the optional field here?
| Schema: map[string]*schema.Schema{ | ||
| "name": { | ||
| Type: schema.TypeString, | ||
| Optional: true, |
There was a problem hiding this comment.
(as above) can we update this to be Computed: true since it can't be set by a user?
| "enable_accelerated_networking": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, | ||
| Default: false, |
There was a problem hiding this comment.
can we change this to be:
"enable_accelerated_networking": {
Type: schema.TypeBool,
Computed: true,
}
| "enable_ip_forwarding": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, | ||
| Default: false, |
| "resource_group_name": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, |
There was a problem hiding this comment.
can we switch over to using the helper for this:
"resource_group_name": resourceGroupNameForDataSourceSchema(),
| }, | ||
| }, | ||
|
|
||
| "tags": tagsSchema(), |
There was a problem hiding this comment.
can we switch over to using the helper here?
"tags": tagsForDataSourceSchema(),
| * `mac_address` - The MAC address used by the network interface. | ||
| * `private_ip_address` - The private ip address associated to the network interface. | ||
| * `virtual_machine_id` - The ID of the virtual machine this network interface is attached to. | ||
| * `network_security_group_id` - The ID of the network security group associated to the network interface |
There was a problem hiding this comment.
it'd be good to add descriptions for the other fields in here too (e.g. ip_configuration / location)
There was a problem hiding this comment.
sure, done and ordered them alphabetically.
| * `private_ip_addresses` - The list of private ip addresses associates to the specified network interface. | ||
| * `tags` - List the tags assocatied to the specified network interface. | ||
| * `virtual_machine_id` - The ID of the virtual machine that the specified network interface is attached to. | ||
|
|
There was a problem hiding this comment.
this is missing a link in the sidebar - I'll push a commit to fix this
|
Forgot to add - tests pass:
|
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
Adding a data source for network interface in Azure.