Exclude expires field from federation definition when value is zero.

[niclic]

How to deal with default values, especially for federation definition attributes like expires and message-ttl?

This is a follow up to a question I buried in the Federation Runtime Parameters PR.

Turns out, that the go zero value of 0 for expires (int type) throws an error when the federation link is created. The docs state that it must be a positive integer. This is not captured by any current tests, since it only happens when you also create a policy that matches a federation upstream and then view the status of the link that is created.

I suspected this would become an issue but it only came to light when doing a complete end-to-end test using the terraform-provider-rabbitmq.

Here is an example of the error returned:

curl -i -u guest:guest \
http://localhost:15672/api/federation-links/%2f
[
  {
    "node": "rabbit@fb688184ee38",
    "exchange": "foo",
    "upstream_exchange": "",
    "type": "exchange",
    "vhost": "/",
    "upstream": "foo",
    "id": "14362399",
    "status": "shutdown",
    "error": "{server_initiated_close,406,\n                        <<\"PRECONDITION_FAILED - invalid arg 'x-expires' for queue 'federation:  -> rabbit@fb688184ee38:foo' in vhost '/': {value_zero,0}\">>}",
    "uri": "amqp://localhost",
    "timestamp": "2020-05-26 17:03:04"
  }
]

The easiest way to resolve this is to add the omitempty tag to the expires field (0 is a valid value for message_ttl). This resolves the error when creating an upstream, which I think is sufficient at this time.

---

Comments

The following comments are just some additional thoughts. I'm not suggesting any other changes to this PR.

1. Because of the way zero values work, the Expires:0 is there again when the FederationDefinition is read. I'm not sure if this needs to be changed, but it you wanted to, it may require changing the field type (to string) and introducing more robust checks for valid numeric values, etc.

2. Should other optional fields also include the omitempty flag? To ensure valid default values for prefetch-count or reconnect-delay, I set the recommended defaults in the terraform provider, otherwise the go zero value of 0 would be set instead of the recommended default value. However, only expires actually results in an error, which is why I changed it.

3. I was unsure how to test for this scenario. If the API included support for federation-links, perhaps it would be possible to check the status of the link in an end-to-end test (but using a single broker)? You could add this feature quite easily by marshalling to an interface{} instead of a predefined struct type, since the response for a federation link has numerous fields. Alternatively, you could select only the most important attributes as fields. If you think this feature is worth adding, then I can pursue it as part of a separate PR. See Added federation-links endpoints #155

[michaelklishin]

Travis failures have to do with Bintray host resolution by apt. Testing this locally.