Repository navigation
NFV-18603: add mgmtAclTemplateUuid in the device acl update request - #12
Conversation
| //DeviceACLTemplateRequest describes request for updating device ACL template | ||
| type DeviceACLTemplateRequest struct { | ||
| TemplateUUID *string `json:"aclTemplateUuid"` | ||
| TemplateUUID *string `json:"aclTemplateUuid"` |
There was a problem hiding this comment.
I think we'll want omitempty for both fields to prevent sending literal null on each request.
| reqBody := api.DeviceACLTemplateRequest{TemplateUUID: &templateID} | ||
| reqBody := api.DeviceACLTemplateRequest{} | ||
| if wanAclTemplateUuid != nil { | ||
| reqBody.TemplateUUID = wanAclTemplateUuid |
There was a problem hiding this comment.
The if conditions are not needed here. omitempty will give the desired behavior.
| type DeviceACLTemplateRequest struct { | ||
| TemplateUUID *string `json:"aclTemplateUuid"` | ||
| TemplateUUID *string `json:"aclTemplateUuid,omitempty"` | ||
| MgmtAclTemplateUuid *string `json:"mgmtAclTemplateUuid,omitempty"` |
There was a problem hiding this comment.
MgmtAclTemplateUuid should have UUID capitalized to match TemplateUUID.
I should have caught this before merging.
@rling-equinix can we follow up with a fix? Are there any other changes you forsee before we tag 1.6.0?
The only CHANGELOG item that I would report here is WithMgmtAclTemplate method added to DeviceUpdateRequest. Is another change that TemplateUUID also become optional (when MgmtAclTemplate is provided)?
There was a problem hiding this comment.
ID/UUID appears to be the convention we've been using in ne-go. We have many examples of ...UUID, ...ID, IP..., and MAC....
We have some exceptions like SshInterfaceId, ClusterId, and LicenseFileId. Because these are in internal/ we can change these freely.
It looks like we should update https://github.com/equinix/ne-go/pull/12/files#diff-79aab787cbc1ff2a0a825a43f479b275e28ea9cc070c72eeede7aeda30894c25R88 and https://github.com/equinix/ne-go/pull/12/files#diff-79aab787cbc1ff2a0a825a43f479b275e28ea9cc070c72eeede7aeda30894c25R69 to use the MgmtAclTemplateUUID too. These are inside of the internal path, so there is no breaking change.
We also have some public exceptions:
client.go:249: MgmtAclTemplateUuid *string
client.go:263: WanInterfaceId *string
client.go:435: ClusterId *string
client.go:446: LicenseFileId *string
Perhaps we can standardize those in a future breaking change.
As part of NE April release, we want to add mgmt acl in the device creation flow as well as device acl update flow. The mgmt acl field was introduced in Feb release in our NE API. Some vendors (e.g. Cisco FTD, PANW and Nginx) support that field.
More details could be found at the following link.
https://developer.equinix.com/docs/acl-template#get-acl-virtual-device
https://developer.equinix.com/docs/acl-template#update-acl-virtual-device
The given link shows the new API contract for acl and mgmt acl. However, the changes in this PR still use the original contract which looks like the following payload.
endpoint: POST /v1/devices/{virtualDeviceUUID}/acl
request body: