Include FirewallBackend in docker info output#6191
Conversation
Signed-off-by: Rob Murray <rob.murray@docker.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
| Info: [][2]string{ | ||
| {"ReloadedAt", "2025-07-16T16:59:14Z"}, | ||
| }, |
There was a problem hiding this comment.
I missed when we added this, but was there a specific reason for choosing [][2]string instead of map[string]string?
There was a problem hiding this comment.
This was the same format as we used for "driver info" (storage drivers); mostly to keep it consistent with that, and to provide a format that's not a formal contract for the API ("anything / any key/value pair goes!"), but in a format that can still be parsed for those wanting to.
There was a problem hiding this comment.
It followed a long discussion in Slack ... but the very short summary is we didn't want to use a map because, although it avoids duplicate keys, the ordering would change in CLI output when iterating, so @thaJeztah suggested it should be like this DriverStatus field.
There was a problem hiding this comment.
A map[string]string probably would've been fine as well, but this was consistent with the other one. (Besides the maps and randomising order - perhaps we should look at a sorted map for some of these)
There was a problem hiding this comment.
Ah, that makes sense now, thanks for the context!
5 participants
- What I did
If the daemon returns
FirewallBackendin itsInforesponse, include it in the output.- How I did it
- How to verify it
Updated unit tests, see golden output.
Also tested locally against a current daemon, with and without firewalld, and against a 27.5.1 daemon that doesn't report FirewallBackend.
- Human readable description for the release notes