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
fix: falsely omitted fields in firewall schema #396
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #396 +/- ##
==========================================
- Coverage 80.35% 80.35% -0.01%
==========================================
Files 32 32
Lines 5722 5731 +9
==========================================
+ Hits 4598 4605 +7
- Misses 717 718 +1
- Partials 407 408 +1 ☔ View full report in Codecov by Sentry. |
What is the rationale behind this change ? I am not sure if this is valid. As the |
Yes, they are, but the API returns empty arrays instead of omitting the property, see here: {
"id": 1216477,
"name": "test",
"labels": {},
"created": "2024-01-23T08:56:12+00:00",
"rules": [
{
"direction": "in",
"protocol": "tcp",
"port": "123",
"source_ips": [
"0.0.0.0/0",
"::/0"
],
"destination_ips": [],
"description": null
},
{
"direction": "out",
"protocol": "tcp",
"port": "123",
"source_ips": [],
"destination_ips": [
"0.0.0.0/0",
"::/0"
],
"description": null
}
],
"applied_to": []
} We should reflect this so that our marshaled schema is consistent with the API responses. |
56988bc
to
15a9265
Compare
15a9265
to
2a90e2d
Compare
🤖 I have created a release *beep* *boop* --- ## [2.7.0](v2.6.0...v2.7.0) (2024-03-27) ### Features * add volume format property ([#397](#397)) ([c0940af](c0940af)) * **error:** handle wrapped errors in IsError() ([#374](#374)) ([83df108](83df108)) * require Go >= 1.20 ([#392](#392)) ([299f181](299f181)) * **server:** deprecate ServerRescueTypeLinux32 ([#378](#378)) ([2f334c3](2f334c3)) * test with Go 1.22 ([#391](#391)) ([49be506](49be506)) ### Bug Fixes * falsely omitted fields in firewall schema ([#396](#396)) ([a3509b6](a3509b6)) * LoadBalancerUpdateServiceOpts not converted correctly ([#394](#394)) ([0f187ce](0f187ce)) * primary ip assignee id not mapped to nil ([#395](#395)) ([b5fea38](b5fea38)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
In the [API Docs](https://docs.hetzner.cloud/#firewalls-get-a-firewall) the `description`, `port`, `destination_ips` and `source_ips` properties are not marked as required (thus being optional) although in practice they are always set. This leads to inconsistencies between marshaled schemas and API responses. This PR modifies the schema so that it is consistent with the actual API output. (cherry picked from commit a3509b6)
In the API Docs the
description
,port
,destination_ips
andsource_ips
properties are not marked as required (thus being optional) although in practice they are always set. This leads to inconsistencies between marshaled schemas and API responses. This PR modifies the schema so that it is consistent with the actual API output.