-
Notifications
You must be signed in to change notification settings - Fork 94
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
Bump Infoblox go client to v2 (2.1.2-0.20220727224704-88a5ba602dbf) #931
Conversation
I've also opened infobloxopen/infoblox-go-client#177 on the client's repo side so that we can later switch to properly released version. |
The latest released version of this go client doesn't have the full support for CRUDING the TXT records (get is missing) so we need to depend on a git sha. It was a major version upgrade so some API was changed and some method signatures were modified. Namely: - inicialization of the client is different, it now requires the auth config to be passed as a separate struct - creating TXT record requires a view argument - `uint -> uint32` for ttl in method signatures - `*ibcl.ObjectManager -> ibcl.IBObjectManager` Signed-off-by: Jirka Kremser <[email protected]>
699f226
to
bcbc8fa
Compare
Signed-off-by: Jirka Kremser <[email protected]>
832e42c
to
0be653c
Compare
@jkremser It would be good to test the change with Infoblox instance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming tested end-to-end
@ytsarev it's not, also it pins non officially released version. |
agreed on parking
Yep, for changes like this the mock tests that we currently have are pretty much useless. They changed the contract of some methods and I had to change the mocking, because they were making assumptions about internal implementations of the API.. like how many times some internal method is being called etc. It would be much more useful to have a way to spawn infoblox service and test it against it. However, if I get it right, there is no easy way to do that. I wasn't able to find any public container image for that.. In the official docs, they suggest to download the tar from their web ui and "docker load" it - wow. I am not a lawyer, but I think this altered image won't be freely usable in the public CI on GitHub. I can also check if the client library has some real tests against Infoblox. UPDATE: the infoblox-client also doesnt' have tests against the real instance - https://github.com/infobloxopen/infoblox-go-client/blob/master/object_manager_a-record_test.go#L31:L43 |
This one looks very stale, and I will close it for now. |
Fix #920
The latest released version of this go client doesn't have the full
support for CRUDING the TXT records (get is missing) so we need to depend on a git sha.
It was a major version upgrade so some API was changed and some method signatures were modified.
Namely:
uint -> uint32
for ttl in method signatures*ibcl.ObjectManager -> ibcl.IBObjectManager
Signed-off-by: Jirka Kremser [email protected]