Add missing CLI endpoints#113
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
JoshSEdwards
requested review from
Jay07GIT,
arpit4ever,
nspadaccino and
ssranjani06
JoshSEdwards
changed the title
| } | ||
|
|
||
| func zoneACLRuleCreate(c *cli.Context) error { | ||
| zoneID, err := getZoneID(client(c), c.String("zone-id"), c.String("zone-name")) |
There was a problem hiding this comment.
client() is being called twice in this function, once here and once more on line 126. That second call can be removed if the client is created once at the top of the function
There was a problem hiding this comment.
Fixed in 65b5fdf. We create the client once at the top of the function now and reuse it for both getZoneID and ZoneACLRuleCreate.
| } | ||
|
|
||
| func zoneACLRuleDelete(c *cli.Context) error { | ||
| zoneID, err := getZoneID(client(c), c.String("zone-id"), c.String("zone-name")) |
There was a problem hiding this comment.
Similar to previous comment, client() is being called twice in this function, once here and once more on line 156. That second call can be removed if the client is created once at the top of the function.
There was a problem hiding this comment.
Fixed in 65b5fdf. Same cleanup as the previous comment. The client is initialized once at the top of the function and reused in zoneACLRuleDelete too
| func zoneSync(c *cli.Context) error { | ||
| client := client(c) | ||
| id, err := getZoneID(client, c.String("zone-id"), c.String("zone-name")) | ||
| z, err := client.ZoneSync(id) |
There was a problem hiding this comment.
This is a preexisting issue so it could be fixed in a separate PR, but the error from getZoneID on line 255 is silently overwritten by the := on line 256. If the zone lookup fails, id will be empty and ZoneSync will be called with a blank ID, producing a confusing error. We could add an error check after line 255 to remedy this.
There was a problem hiding this comment.
Fixed in 65b5fdf. Added an explicit error check after getZoneID so we return the lookup error instead of calling ZoneSync with an empty ID.
2 participants
Summary
Dependency
github.com/vinyldns/go-vinyldnstov0.9.18, which includes the client methods needed by these commandsgo.modreplace now that the go-vinyldns changes have been releasedTest/CI Updates
0.21.6, since the old0.9.10test API does not expose several endpoints covered by this PRValidation
go test ./...VDNS-253