Allow DNSForwardingSettings to become partial#36
Conversation
Signed-off-by: 卜部 昌平 <s-urabe@sakura.ad.jp>
…g.yaml ./openapi/openapi.json Signed-off-by: 卜部 昌平 <s-urabe@sakura.ad.jp>
Signed-off-by: 卜部 昌平 <s-urabe@sakura.ad.jp>
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenAPI schema and regenerated Go types/JSON codecs so ModelsSettingsDNSForwardingSettings can be decoded from “partial” server responses (e.g., when DNS private hosted zone is disabled and only Enabled is returned).
Changes:
- Relax
Models.Settings.DNSForwardingSettingsrequired fields inopenapi.json(onlyEnabledremains required). - Regenerate Go models/codecs so
PrivateHostedZone/UpstreamDNS1/UpstreamDNS2become optional (OptString) and are conditionally encoded. - Update examples/tests to construct the optional string fields via
NewOptString.
Reviewed changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
seg_test.go |
Updates test input construction to use OptString fields. |
example_test.go |
Updates example code to use NewOptString for optional DNS forwarding fields. |
openapi/openapi.json |
Makes DNS forwarding fields optional in the schema (only Enabled required). |
apis/v1/oas_schemas_gen.go |
Regenerates model struct: DNS forwarding string fields become OptString (public API surface change). |
apis/v1/oas_json_gen.go |
Regenerates JSON encode/decode logic to omit unset optional fields and relax required-field validation. |
apis/v1/oas_faker_gen.go |
Updates faker to populate OptString fields via SetFake(). |
apis/v1/oas_test_examples_gen_test.go |
Regenerated tests; import ordering change needs goimports/gofmt compliance fix. |
Files not reviewed (4)
- apis/v1/oas_faker_gen.go: Generated file
- apis/v1/oas_json_gen.go: Generated file
- apis/v1/oas_schemas_gen.go: Generated file
- apis/v1/oas_test_examples_gen_test.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import ( | ||
| std "encoding/json" | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/go-faster/errors" | ||
| "github.com/go-faster/jx" |
There was a problem hiding this comment.
This file is auto generated. We're not going to touch it.
| "Models.Settings.DNSForwardingSettings": { | ||
| "type": "object", | ||
| "required": [ | ||
| "Enabled", | ||
| "PrivateHostedZone", | ||
| "UpstreamDNS1", | ||
| "UpstreamDNS2" | ||
| "Enabled" | ||
| ], |
There was a problem hiding this comment.
This is something optional that we don't plan to do.
| // ゾーン名. | ||
| PrivateHostedZone string `json:"PrivateHostedZone"` | ||
| PrivateHostedZone OptString `json:"PrivateHostedZone"` | ||
| // フォワード先のDNSサーバ(1). | ||
| UpstreamDNS1 string `json:"UpstreamDNS1"` | ||
| UpstreamDNS1 OptString `json:"UpstreamDNS1"` | ||
| // フォワード先のDNSサーバ(2). | ||
| UpstreamDNS2 string `json:"UpstreamDNS2"` | ||
| UpstreamDNS2 OptString `json:"UpstreamDNS2"` |
There was a problem hiding this comment.
This file is auto generated.
| return errors.Wrap(err, "decode field \"Enabled\"") | ||
| } | ||
| case "PrivateHostedZone": | ||
| requiredBitSet[0] |= 1 << 1 | ||
| if err := func() error { | ||
| v, err := d.Str() | ||
| s.PrivateHostedZone = string(v) | ||
| if err != nil { | ||
| s.PrivateHostedZone.Reset() | ||
| if err := s.PrivateHostedZone.Decode(d); err != nil { | ||
| return err |
There was a problem hiding this comment.
That is a legit criticism but the current test structure is not made as such; adding this test is too much of a pull request.
DNSプライベートホストゾーンが無効な場合に、実際のサーバからは
{...,"DNSForwarding":{"Enabled":"False"}}のような短い応答が返ってきうる模様ですので、そういったレスポンスを許容します。