Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .agents/skills/fetch-spec/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Description

Use this skill when you need to look up details about the Oxide API, including:

- Finding endpoint definitions, paths, or HTTP methods
- Looking up schema/type definitions from the API spec
- Understanding request/response formats for API operations
Expand Down
296 changes: 296 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,296 @@
# Design Notes

## oneOf Type Generation

### Overview

OpenAPI `oneOf` types represent sum types, which are values that can be one of several variant
types. Go doesn't have a native sum type, so we have to decide how to represent these values in a
safe and ergonomic way. There are a few distinct patterns of `oneOf` types in the Oxide API, related
to different `serde` tagging strategies, and we handle each of them differently.

### Tagged union

When a `oneOf` has:

1. Exactly one discriminator property (a field with a single enum value per variant)
2. Exactly one multi-type property (a field whose type varies across variants)

We generate an **interface with variant wrapper types** pattern.

**Example: `PrivateIpStack`**

In Rust, `PrivateIpStack` is defined as:

```rust
#[serde(tag = "type", content = "value", rename_all = "snake_case")]
pub enum PrivateIpStack {
V4(PrivateIpv4Stack),
V6(PrivateIpv6Stack),
DualStack { v4: PrivateIpv4Stack, v6: PrivateIpv6Stack },
}
```

This generates the following OpenAPI spec:

```yaml
PrivateIpStack:
oneOf:
- type: object
properties:
type: { enum: ["v4"] }
value: { $ref: "#/components/schemas/PrivateIpv4Stack" }
required: [type, value]
- type: object
properties:
type: { enum: ["v6"] }
value: { $ref: "#/components/schemas/PrivateIpv6Stack" }
required: [type, value]
- type: object
properties:
type: { enum: ["dual_stack"] }
value:
type: object
properties:
v4: { $ref: "#/components/schemas/PrivateIpv4Stack" }
v6: { $ref: "#/components/schemas/PrivateIpv6Stack" }
required: [type, value]
```

This has:

- One discriminator: `type` (with values `v4`, `v6`, `dual_stack`)
- One multi-type property: `value` (which is `PrivateIpv4Stack`, `PrivateIpv6Stack`, or an inline
object depending on variant)

So it generates:

```go
// Interface with marker method that all variants implement
type privateIpStackVariant interface {
isPrivateIpStackVariant()
}

// Variant wrapper types (one per discriminator value)
type PrivateIpStackV4 struct {
Value PrivateIpv4Stack `json:"value"`
}
func (PrivateIpStackV4) isPrivateIpStackVariant() {}

type PrivateIpStackV6 struct {
Value PrivateIpv6Stack `json:"value"`
}
func (PrivateIpStackV6) isPrivateIpStackVariant() {}

type PrivateIpStackDualStack struct {
Value PrivateIpStackValue `json:"value"`
}
func (PrivateIpStackDualStack) isPrivateIpStackVariant() {}
Comment on lines +74 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably outside of the scope for this PR, but another challenge I faced while using the SDK is some confusion in the generated type names.

For example, PrivateIpStackValue is missing from this code block, but I'm assuming it looks something like this:

type PrivateIpStackValue struct {
  V4 PrivateIpv4Stack
  V6 PrivateIpv6Stack
}

But in isolation, PrivateIpStackValue is a very generic name that doesn't communicate a whole lot. Something like PrivateIpDualStack would probably be better, and match its sibling variants name pattern (PrivateIpv4Stack and PrivateIpv6Stack).

The other variants also have a naming issue, with PrivateIpStackV4 being way too close to PrivateIpv4Stack. Maybe in this case we could add a *Variant suffix to make the variant struct more distinct and add an additional signal that this is a variant out of many options?

Putting it all together, these changes would make the generated code look something like this:

// Variant wrapper types (one per discriminator value)
type PrivateIpStackV4Variant struct {
    Value PrivateIpv4Stack `json:"value"`
}
func (PrivateIpStackV4Variant) isPrivateIpStackVariant() {}

type PrivateIpStackV6Variant struct {
    Value PrivateIpv6Stack `json:"value"`
}
func (PrivateIpStackV6Variant) isPrivateIpStackVariant() {}

type PrivateIpStackDualStackVariant struct {
    Value PrivateIpDualStack `json:"value"`
}
func (PrivateIpStackDualStackVariant) isPrivateIpStackVariant() {}

type PrivateIpDualStack struct {
  V4 PrivateIpv4Stack
  V6 PrivateIpv6Stack
}

But I'm not sure how much of this would be possible to auto-generate based on the spec alone.


// Main type with only the value field
type PrivateIpStack struct {
Value privateIpStackVariant `json:"value,omitempty"`
}

// Type() method derives the discriminator from the concrete Value type
func (v PrivateIpStack) Type() PrivateIpStackType {
switch v.Value.(type) {
case *PrivateIpStackV4:
return PrivateIpStackTypeV4
case *PrivateIpStackV6:
return PrivateIpStackTypeV6
case *PrivateIpStackDualStack:
return PrivateIpStackTypeDualStack
default:
return ""
}
}
```

The discriminator field is not stored in the struct. We don't want users to be able to set its value
so that it doesn't match the type of the value field. Instead, we expose a public method named after
the discriminator that returns the discriminator value.

We also implement custom `MarshalJSON` and `UnmarshalJSON` methods for the main type. To unmarshal,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't technically need it for our APIs, but would we need the YAML equivalent for feature completeness?

we check the discriminator field in the JSON to determine which concrete type to use for
unmarshalling the value. To marshal, we call the `Type()` method to determine which discriminator to
emit.
Comment on lines +110 to +117
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not storing the discriminator field in the struct makes sense but the natural next question is how do we know what type the raw bytes are when unmarshalling. You answer that here but I think updating the wording to note that we explicitly read the discriminator from the JSON first before then deciding to read the rest of the JSON would be helpful. Perhaps this information is elsewhere already but I'm reading this linearly and making comments as I go.


**Usage examples:**

```go
// Reading a network interface's IP stack from the API
nic, _ := client.InstanceNetworkInterfaceView(ctx, params)
ipStack := nic.IpStack

switch v := ipStack.Value.(type) {
case *oxide.PrivateIpStackV4:
fmt.Printf("IPv4 only: %s\n", v.Value.Ip)
case *oxide.PrivateIpStackV6:
fmt.Printf("IPv6 only: %s\n", v.Value.Ip)
case *oxide.PrivateIpStackDualStack:
fmt.Printf("Dual stack: %s / %s\n", v.Value.V4.Ip, v.Value.V6.Ip)
}
```

```go
// Creating a network interface with an IPv4-only stack
params := oxide.InstanceNetworkInterfaceCreateParams{
Body: &oxide.InstanceNetworkInterfaceCreate{
Name: "my-nic",
SubnetName: "my-subnet",
VpcName: "my-vpc",
IpConfig: oxide.PrivateIpStackCreate{
Value: &oxide.PrivateIpStackCreateV4{
Value: oxide.PrivateIpv4StackCreate{
Ip: oxide.Ipv4Assignment{
Type: oxide.Ipv4AssignmentTypeExplicit,
Value: "10.0.0.5",
},
},
},
},
},
}
```

**Why wrapper structs?**

We represent each variant type using a wrapper struct, e.g.

```go
type PrivateIpStackV4 struct {
Value PrivateIpv4Stack `json:"value"`
}
```

The wrapper struct implements the marker method for the relevant interface:

```go
func (PrivateIpStackV4) isPrivateIpStackVariant() {}
```

Why use wrapper structs? For some variant types, we could omit the wrapper and implement the marker
method on the wrapped type instead:

```go
func (PrivateIpv4Stack) isPrivateIpStackVariant() {}
```

Primitive types can't implement methods, but we could use type definitions instead:

```go
type MyPrimitiveVariant string

func (MyPrimitiveVariant) isMyTypeVariant() {}
```

However, this presents a few problems:

- Some variant types are `interface{}` or `any`. We can't implement methods on `any`.
- Some variant types are represented by pointers to primitive types, like `*bool` or `*int`. We
can't implement methods on pointers to primitive types.

We could represent some variant types with wrapper structs and others as unwrapped structs or type
definitions. But the complexity of conditionally wrapping variant types is potentially more
confusing to end users than consistent use of wrappers.

Note: we can reconsider this choice if we're able to drop the use of `interface{}` types and
pointers to primitives for variants, and if we're confident that those cases won't emerge again.

### Discriminator with multiple value fields
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this our desired end goal for such types or just a stopgap to maintain the current behavior today before we have time to revisit this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a stopgap. I think it would be reasonable to change this to an interface type as well, but I didn't want to overcomplicate this initial PR. I'm down to explore that possibility here if you want, or maybe start a second PR branching off this one so we don't make this too hard to review.


When a `oneOf` has a discriminator field and _multiple_ value fields, we use a flat struct that
contains all properties from all variants. Properties that have different types across variants
become `any`.

**Example: `DiskSource`**

In Rust, `DiskSource` is defined as:

```rust
#[serde(tag = "type", rename_all = "snake_case")]
pub enum DiskSource {
Blank { block_size: BlockSize },
Snapshot { snapshot_id: Uuid },
Image { image_id: Uuid },
ImportingBlocks { block_size: BlockSize },
}
```

This generates the following OpenAPI spec:

```yaml
DiskSource:
oneOf:
- type: object
properties:
type: { enum: ["blank"] }
block_size: { $ref: "#/components/schemas/BlockSize" }
- type: object
properties:
type: { enum: ["snapshot"] }
snapshot_id: { type: string, format: uuid }
- type: object
properties:
type: { enum: ["image"] }
image_id: { type: string, format: uuid }
- type: object
properties:
type: { enum: ["importing_blocks"] }
block_size: { $ref: "#/components/schemas/BlockSize" }
```

This has a discriminator (`type`) but no multi-type property. Each variant has different fields
(`block_size`, `snapshot_id`, `image_id`), not different types for the same field. So we generate a
flat struct:

```go
type DiskSource struct {
BlockSize BlockSize `json:"block_size,omitempty"`
Type DiskSourceType `json:"type,omitempty"`
SnapshotId string `json:"snapshot_id,omitempty"`
ImageId string `json:"image_id,omitempty"`
}
```

If any property had different types across variants, it would become `any`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I understand this part, would PrivateIpStack be an example of this? Where the same field (value) can be one of many types?

If so, would we be able to provide at least some helper methods for now so users don't have to marshal and unmarshal bytes themselves?


### Untagged union

When a `oneOf` has no object properties (i.e., variants are primitive types or references wrapped in
`allOf`), the type becomes `interface{}`.

**Example: `IpNet`**

In Rust, `IpNet` is defined as:

```rust
#[serde(untagged)]
pub enum IpNet {
V4(Ipv4Net),
V6(Ipv6Net),
}
```

This generates the following OpenAPI spec:

```yaml
IpNet:
oneOf:
- title: v4
allOf:
- $ref: "#/components/schemas/Ipv4Net"
- title: v6
allOf:
- $ref: "#/components/schemas/Ipv6Net"
```

```go
type IpNet interface{}
```

Note: we may be able to handle these types better in the future. For example, we could detect that
all variants are effectively strings and represent `IpNet` as `string`. Alternatively, we could
represent `Ipv4Net` and `Ipv6Net` as distinct types with their own validation logic, and attempt to
unmarshal into each variant type until we find a match.
Loading