Skip to content

Commit

Permalink
gotohelm: improve special case detection in zeroOf
Browse files Browse the repository at this point in the history
Prior to this commit gotohelm would incorrectly attempt to compute the zero
value of types that implemented `json.{Unm,M}arshaller`. In most cases, this
would lead to a transpiler crash all together. In some corner cases, it was
possible to silently generate incorrect values.

This commit improves `zeroOf` to detect and reject implementers of
`json.{Unm,M}arshaller` with a helpful error message and adds a special case
for Kubernetes' `resource.Quantity` type.
  • Loading branch information
chrisseto committed Dec 16, 2024
1 parent 66671ba commit e6cc8bc
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 4 deletions.
7 changes: 7 additions & 0 deletions pkg/gotohelm/testdata/src/example/k8s/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,16 @@ func quantity(dot *helmette.Dot) map[string]any {
value = append(value, q.Value())
}

// Intentionally generate zero values of resource.Quantity to assert that
// zeroOf handles it.
var varZero resource.Quantity
resources := corev1.ResourceList{}

return map[string]any{
"MustParse": quantities,
"Value": value,
"String": strs,
"dictZero": resources[corev1.ResourceCPU],
"varZero": varZero,
}
}
7 changes: 7 additions & 0 deletions pkg/gotohelm/testdata/src/example/k8s/k8s.rewritten.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,16 @@ func quantity(dot *helmette.Dot) map[string]any {
value = append(value, q.Value())
}

// Intentionally generate zero values of resource.Quantity to assert that
// zeroOf handles it.
var varZero resource.Quantity
resources := corev1.ResourceList{}

return map[string]any{
"MustParse": quantities,
"Value": value,
"String": strs,
"dictZero": resources[corev1.ResourceCPU],
"varZero": varZero,
}
}
4 changes: 3 additions & 1 deletion pkg/gotohelm/testdata/src/example/k8s/k8s.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@
{{- if $_is_returning -}}
{{- break -}}
{{- end -}}
{{- $varZero := "0" -}}
{{- $resources := (dict ) -}}
{{- $_is_returning = true -}}
{{- (dict "r" (dict "MustParse" $quantities "Value" $value "String" $strs )) | toJson -}}
{{- (dict "r" (dict "MustParse" $quantities "Value" $value "String" $strs "dictZero" (ternary (index $resources "cpu") "0" (hasKey $resources "cpu")) "varZero" $varZero )) | toJson -}}
{{- break -}}
{{- end -}}
{{- end -}}
Expand Down
27 changes: 24 additions & 3 deletions pkg/gotohelm/transpiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -1459,9 +1459,6 @@ func (t *Transpiler) typeOf(expr ast.Expr) types.Type {
}

func (t *Transpiler) zeroOf(typ types.Type) Node {
// TODO need to detect and reject or special case implementors of
// json.Marshaler. Getting a handle to a that interface is... difficult.

// Special cases.
switch typ.String() {
case "k8s.io/apimachinery/pkg/apis/meta/v1.Time":
Expand All @@ -1470,6 +1467,30 @@ func (t *Transpiler) zeroOf(typ types.Type) Node {
// IntOrString's zero value appears to marshal to a 0 though it's
// unclear how correct this is.
return &Literal{Value: "0"}
case "k8s.io/apimachinery/pkg/api/resource.Quantity":
return &Literal{Value: `"0"`}
}

// If encoding/json is in the dependency chain for this package, we'll
// enable some additional checks (because it's other wise very difficult to
// check for implementation of {M,Unm}arshaller...)
if json, ok := t.packages["encoding/json"]; ok {
marshaller := json.Types.Scope().Lookup("Marshaler").Type().Underlying().(*types.Interface)
unmarshaller := json.Types.Scope().Lookup("Unmarshaler").Type().Underlying().(*types.Interface)

ptr := types.NewPointer(typ)

// If we can get a handle to Marshaler and Unmarshaler, we'll error out
// on any types that implement either and aren't special cased as their
// JSON representation likely won't match what we can infer from the go
// type.
switch {
case types.Implements(typ, unmarshaller),
types.Implements(typ, marshaller),
types.Implements(ptr, marshaller),
types.Implements(ptr, unmarshaller):
panic(fmt.Sprintf("unsupported type %q implemented json.Unmarshaler or json.Marshaller and needs to be special cased but isn't currently", typ.String()))
}
}

switch underlying := typ.Underlying().(type) {
Expand Down

0 comments on commit e6cc8bc

Please sign in to comment.