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.

Sign up for GitHub

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

Jump to bottom

#190-FetchInteger: Fix #492

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

#190-FetchInteger: Fix #492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf5fe1e
#190-FetchInteger: Fix
Aug 8, 2024
f22b744
#190-FetchInteger: TestCase
Aug 12, 2024
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
4 changes: 4 additions & 0 deletions vals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ func (r *Runtime) prepare() (*expansion.ExpandRegexMatch, error) {
for i, k := range keys {
newobj := map[string]interface{}{}
switch t := obj[k].(type) {
case int:
return fmt.Sprint(t), nil
case bool:
return fmt.Sprint(t), nil
case string:
if i != len(keys)-1 {
return "", fmt.Errorf("unexpected type of value for key at %d=%s in %v: expected map[string]interface{}, got %v(%T)", i, k, keys, t, t)
Expand Down
34 changes: 34 additions & 0 deletions vals_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,37 @@ kind: Secret

require.Equal(t, expected, buf.String())
}

func TestEvalIntegerBool(t *testing.T) {
var yamlDocs = `
---
Integer: 1
ResultInteger: ref+file://secrets.yaml#/Integer
Bool: true
ResultBool: ref+file://secrets.yaml#/Bool
`
var expected = `Bool: true
Integer: 1
ResultBool: "true"
ResultInteger: "1"
Comment on lines +130 to +131
Copy link
Collaborator

Choose a reason for hiding this comment

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

@artiodev Thanks a bunch for the pull request!
This looks well aligned with vals' design, where it wants any fetched values to be rendered as strings, so that the values before(ref+...) and after the vals rendering are both strings. It's great in that regard.

But, is this what you originally wanted in #190?

Here, I was wondering if you wanted:

Suggested change
ResultBool: "true"
ResultInteger: "1"
ResultBool: true
ResultInteger: 1

rather than the string representations of the original values.

If necessary, we could support both options, perhaps via feature toggling envvars and or ref URL params. Just wanted to confirm your goal and intention to start!

Thanks in advance for your cooperation 🙏

`

tmpFile, err := os.Create("secrets.yaml")
defer os.Remove(tmpFile.Name())
require.NoError(t, err)

_, err = tmpFile.WriteString(yamlDocs)
require.NoError(t, err)

input, err := Inputs(tmpFile.Name())
require.NoError(t, err)

nodes, err := EvalNodes(input, Options{})
require.NoError(t, err)
buf := new(strings.Builder)

err = Output(buf, "", nodes)
require.NoError(t, err)

require.Equal(t, expected, buf.String())
}
Loading