Skip to content
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

Add client logic to support read-only Mikrotik properties #143

Conversation

jdelnano
Copy link
Collaborator

Background

This PR adjusts logic in client.go related to Marshaling data before making API calls to a RouterOS device. While pairing with @DawoudMahmud on adding client support for creating Interface Wireguard resources (#140, which is still a WIP), we came across Mikrotik documentation that lists some Wireguard properties to be "Read Only".

This led to a test failure (see build here) since a read-only boolean property running was included in the interface wireguard add command:

=== RUN   TestAddFindDeleteInterfaceWireguard
2023/03/30 23:54:57 [INFO] Running the mikrotik command: `[/interface/wireguard/add =name=new_interface_wireguard =comment=new interface from test =disabled=no =listen-port=10000 =mtu=10001 =private-key=YOi0P0lTTiN8hAQvuRET23Srb+U7C52iOZokj0CCSkM= =running=no]`
Error:     interface_wireguard_test.go:35: expected no error, got from RouterOS device: unknown parameter running
--- FAIL: TestAddFindDeleteInterfaceWireguard (0.04s)

The issue here is that the unset boolean struct field defaulted to a value of false and then was subsequently included in marshaling.

What does this PR do?

  • Adjust client.go in regards to how the mikrotik struct tag is retrieved, doing so in a way that we now check for a new readonly tag value.
  • If readonly is specified as a struct tag value, that field will be excluded during marshaling.
  • A test has been updated to illustrate that a field will not be included in marshaling should the readonly struct tag value be present.

@jdelnano jdelnano marked this pull request as ready for review March 31, 2023 02:51
client/client.go Outdated Show resolved Hide resolved
@ddelnano
Copy link
Owner

ddelnano commented Apr 1, 2023

One minor comment about a misspelling I noticed, but otherwise lgtm. I'll wait for @maksym-nazarenko to give this a look as well.

On the topic of struct tags, I think it's best to avoid proliferating additional tags values when there are better alternatives (#131 as an example of this -- marshal/unmarshalling a custom type). In this case, I think a readonly tag is an appropriate though.

Co-authored-by: Dom Del Nano <ddelnano@gmail.com>
client/client.go Outdated Show resolved Hide resolved
Co-authored-by: Maksym <maks.nazarenko@gmail.com>
@jdelnano jdelnano merged commit 87c138a into ddelnano:master Apr 12, 2023
@jdelnano jdelnano deleted the jdelnano/enhance-marshaling-related-to-struct-tags branch April 12, 2023 03:55
@ddelnano ddelnano added the enhancement New feature or request label May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants