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

[codegen] Fix bug in provider schema where default int properties could not be int #13599

Merged
merged 1 commit into from Jul 27, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: sdkgen
description: Fix bug binding provider schema where type default int values could not take integers.
29 changes: 20 additions & 9 deletions pkg/codegen/schema/bind.go
Expand Up @@ -958,35 +958,46 @@ func bindConstValue(path, kind string, value interface{}, typ Type) (interface{}

switch typ = plainType(typ); typ {
case BoolType:
if _, ok := value.(bool); !ok {
v, ok := value.(bool)
if !ok {
return false, typeError("boolean")
}
return v, nil
case IntType:
v, ok := value.(float64)
v, ok := value.(int)
if !ok {
return 0, typeError("integer")
v, ok := value.(float64)
if !ok {
return 0, typeError("integer")
}
if math.Trunc(v) != v || v < math.MinInt32 || v > math.MaxInt32 {
return 0, typeError("integer")
}
return int32(v), nil
}
if math.Trunc(v) != v || v < math.MinInt32 || v > math.MaxInt32 {
if v < math.MinInt32 || v > math.MaxInt32 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check ever needed when v was correctly casted to int from value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. That check takes a float64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I've written a test case to catch overflows/underflows that failed without it.

return 0, typeError("integer")
}
value = int32(v)
return int32(v), nil
case NumberType:
if _, ok := value.(float64); !ok {
v, ok := value.(float64)
if !ok {
return 0.0, typeError("number")
}
return v, nil
case StringType:
if _, ok := value.(string); !ok {
v, ok := value.(string)
if !ok {
return 0.0, typeError("string")
}
return v, nil
default:
if _, isInvalid := typ.(*InvalidType); isInvalid {
return nil, nil
}
return nil, hcl.Diagnostics{errorf(path, "type %v cannot have a constant value; only booleans, integers, "+
"numbers and strings may have constant values", typ)}
}

return value, nil
}

func bindDefaultValue(path string, value interface{}, spec *DefaultSpec, typ Type) (*DefaultValue, hcl.Diagnostics) {
Expand Down
19 changes: 19 additions & 0 deletions pkg/codegen/schema/schema_test.go
Expand Up @@ -18,6 +18,7 @@ package schema
import (
"encoding/json"
"fmt"
"math"
"net/url"
"os"
"path/filepath"
Expand Down Expand Up @@ -957,3 +958,21 @@ func TestPackageIdentity(t *testing.T) {
})
}
}

func TestBindDefaultInt(t *testing.T) {
t.Parallel()
dv, diag := bindDefaultValue("fake-path", int(32), nil, IntType)
if diag.HasErrors() {
t.Fail()
}
assert.Equal(t, int32(32), dv.Value)

// Check that we error on overflow/underflow when casting int to int32.
if _, diag := bindDefaultValue("fake-path", int(math.MaxInt64), nil, IntType); !diag.HasErrors() {
assert.Fail(t, "did not catch oveflow")
t.Fail()
}
if _, diag := bindDefaultValue("fake-path", int(math.MinInt64), nil, IntType); !diag.HasErrors() {
assert.Fail(t, "did not catch underflow")
}
}