New Resource: azurerm_packet_capture
#1044
Conversation
tombuildsstuff
changed the title
azurerm_packet_captureazurerm_packet_capture
tombuildsstuff
force-pushed
the
packet-capture
branch
4 times, most recently
from
f7eb7a9 to
190cf57
Compare
tombuildsstuff
changed the title
azurerm_packet_captureazurerm_packet_capture
|
Tests pass:
|
tombuildsstuff
force-pushed
the
packet-capture
branch
from
190cf57 to
334941b
Compare
| resp, err := client.Get(ctx, resourceGroup, watcherName, name) | ||
| if err != nil { | ||
| if utils.ResponseWasNotFound(resp.Response) { | ||
| d.SetId("") |
There was a problem hiding this comment.
Minor nitpick: missing logging message, e.g. log.Printf("[WARN] Packet Capture (%s) not found - removing from state", id)
| Steps: []resource.TestStep{ | ||
| { | ||
| Config: testAzureRMPacketCapture_localDiskConfig(ri, location), | ||
| Check: resource.ComposeTestCheckFunc( |
There was a problem hiding this comment.
Minor nitpick: should we be ensuring any attributes are correctly in state? e.g.
resource.TestCheckResourceAttr(resourceName, "storage_location.#", "1"),
resource.TestCheckResourceAttr(resourceName, "storage_location.0.file_path", "/var/captures/packet.cap"),
There was a problem hiding this comment.
so we’ve been leaning on the Import tests to achieve that, since it’s comparing the config anyway - rather than duplicating these values; but I can change this if you want?
| } | ||
|
|
||
| resource "azurerm_virtual_network" "test" { | ||
| name = "production-network" |
There was a problem hiding this comment.
Minor nitpick: should terraform fmt the examples to make them easier to read
|
|
||
| * `local_port` - (Optional) The local port to be filtered on. Notation: "80" for single port entry."80-85" for range. "80;443;" for multiple entries. Multiple ranges not currently supported. Mixing ranges with multiple entries not currently supported. Changing this forces a new resource to be created. | ||
|
|
||
| * `protocol` - (Required) The Protocol to be filtered on. Possible values include `Any`, `TCP` and ``UDP`. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Typo: Double backticks before UDP
|
|
||
| A `filter` block contains: | ||
|
|
||
| * `local_ip_address` - (Optional) The local IP Address to be filtered on. Notation: "127.0.0.1" for single address entry. "127.0.0.1-127.0.0.255" for range. "127.0.0.1;127.0.0.5"? for multiple entries. Multiple ranges not currently supported. Mixing ranges with multiple entries not currently supported. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Extraneous ? before for multiple entries
There was a problem hiding this comment.
that's actually a copy and paste from the Microsoft Documentation
but yes, I'd agree let's remove it
|
|
||
| * `maximum_bytes_per_packet` - (Optional) The number of bytes captured per packet. The remaining bytes are truncated. Defaults to `0` (Entire Packet Captured). Changing this forces a new resource to be created. | ||
|
|
||
| * `maximum_bytes_per_session` - (Optional) Maximum size of the capture in Bytes. Defaults to `10737441824` (1GB). Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Typo: default does not match what is defined in the schema: 1073741824
There was a problem hiding this comment.
confirmed in the portal this should be 1073741824 - will fix
tombuildsstuff
force-pushed
the
packet-capture
branch
from
334941b to
8e3da9f
Compare
|
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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Adds support for Packet Capturing from Virtual Machines using a Network Watcher
Since this is reliant on the VM Extension and the Network Watcher (and we can only provision one per region at a time) - I've added this to the Network Watcher test case