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

fix: LoadBalancerUpdateServiceOpts not converted correctly #394

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

phm07
Copy link
Contributor

@phm07 phm07 commented Mar 5, 2024

In the conversion from LoadBalancerUpdateServiceOpts to schema.LoadBalancerActionUpdateServiceRequest, there are conversions from slices to slice pointers. Slice pointers exist in schemas to differentiate between absent and empty.

Goverter converts nil slices to &nil. This leads to the field being marshaled to null in JSON (even with the omitempty flag) instead of being not present (See hetznercloud/cli#702)

This PR fixes this issue by adding manual conversion methods and adds tests to account for this behavior.

In the conversion from LoadBalancerUpdateServiceOpts to schema.LoadBalancerActionUpdateServiceRequest, there are conversions from slices to slice pointers. Slice pointers exist in schemas to differentiate between absent and empty.

Goverter converts nil slices to &nil. This leads to the field being marshaled to `null` in JSON (even with the `omitempty` flag) instead of being not present (See hetznercloud/cli#702)

This PR fixes this issue by adding manual conversion methods
@phm07 phm07 added the Bug label Mar 5, 2024
@phm07 phm07 self-assigned this Mar 5, 2024
@phm07 phm07 requested a review from a team as a code owner March 5, 2024 10:37
Copy link

codecov bot commented Mar 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.34%. Comparing base (7511685) to head (8353c9b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #394      +/-   ##
==========================================
- Coverage   80.35%   80.34%   -0.02%     
==========================================
  Files          32       32              
  Lines        5722     5718       -4     
==========================================
- Hits         4598     4594       -4     
  Misses        717      717              
  Partials      407      407              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hcloud/schema_gen.go Outdated Show resolved Hide resolved
@phm07 phm07 force-pushed the fix-loadbalancer-update-service-opts-schema branch from 0e28a30 to 8353c9b Compare March 5, 2024 10:42
@phm07 phm07 merged commit 0f187ce into main Mar 5, 2024
4 checks passed
@phm07 phm07 deleted the fix-loadbalancer-update-service-opts-schema branch March 5, 2024 10:45
apricote pushed a commit that referenced this pull request Mar 27, 2024
🤖 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>
apricote pushed a commit to hetznercloud/fleeting-plugin-hetzner that referenced this pull request Sep 12, 2024
…etznercloud/fleeting-plugin-hetzner!38)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/hetznercloud/hcloud-go/v2](https://github.com/hetznercloud/hcloud-go) | require | minor | `v2.6.0` -> `v2.7.2` |

---

### Release Notes

<details>
<summary>hetznercloud/hcloud-go (github.com/hetznercloud/hcloud-go/v2)</summary>

### [`v2.7.2`](https://github.com/hetznercloud/hcloud-go/releases/tag/v2.7.2)

[Compare Source](hetznercloud/hcloud-go@v2.7.1...v2.7.2)

##### Bug Fixes

-   creating firewall with rules not working correctly ([#&#8203;412](hetznercloud/hcloud-go#412)) ([43b2bab](hetznercloud/hcloud-go@43b2bab))

### [`v2.7.1`](https://github.com/hetznercloud/hcloud-go/releases/tag/v2.7.1)

[Compare Source](hetznercloud/hcloud-go@v2.7.0...v2.7.1)

##### Bug Fixes

-   setting firewall rules not working correctly ([#&#8203;405](hetznercloud/hcloud-go#405)) ([16daea0](hetznercloud/hcloud-go@16daea0))

### [`v2.7.0`](https://github.com/hetznercloud/hcloud-go/releases/tag/v2.7.0)

[Compare Source](hetznercloud/hcloud-go@v2.6.0...v2.7.0)

##### Features

-   add volume format property ([#&#8203;397](hetznercloud/hcloud-go#397)) ([c0940af](hetznercloud/hcloud-go@c0940af))
-   **error:** handle wrapped errors in IsError() ([#&#8203;374](hetznercloud/hcloud-go#374)) ([83df108](hetznercloud/hcloud-go@83df108))
-   require Go >= 1.20 ([#&#8203;392](hetznercloud/hcloud-go#392)) ([299f181](hetznercloud/hcloud-go@299f181))
-   **server:** deprecate ServerRescueTypeLinux32 ([#&#8203;378](hetznercloud/hcloud-go#378)) ([2f334c3](hetznercloud/hcloud-go@2f334c3))
-   test with Go 1.22 ([#&#8203;391](hetznercloud/hcloud-go#391)) ([49be506](hetznercloud/hcloud-go@49be506))

##### Bug Fixes

-   falsely omitted fields in firewall schema ([#&#8203;396](hetznercloud/hcloud-go#396)) ([a3509b6](hetznercloud/hcloud-go@a3509b6))
-   LoadBalancerUpdateServiceOpts not converted correctly ([#&#8203;394](hetznercloud/hcloud-go#394)) ([0f187ce](hetznercloud/hcloud-go@0f187ce))
-   primary ip assignee id not mapped to nil ([#&#8203;395](hetznercloud/hcloud-go#395)) ([b5fea38](hetznercloud/hcloud-go@b5fea38))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMjQuMSIsInVwZGF0ZWRJblZlciI6IjM3LjMyNC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants