Skip to content

Commit

Permalink
Fix unfiled bug; don't quote port numbers.
Browse files Browse the repository at this point in the history
  • Loading branch information
monopole committed Jan 17, 2021
1 parent 2a16af8 commit 065c2b8
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 31 deletions.
9 changes: 9 additions & 0 deletions api/filters/refvar/refvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,21 @@ func (f Filter) set(node *yaml.RNode) error {

func updateNodeValue(node *yaml.Node, newValue interface{}) {
switch newValue := newValue.(type) {
case int:
node.Value = strconv.FormatInt(int64(newValue), 10)
node.Tag = yaml.NodeTagInt
case int32:
node.Value = strconv.FormatInt(int64(newValue), 10)
node.Tag = yaml.NodeTagInt
case int64:
node.Value = strconv.FormatInt(newValue, 10)
node.Tag = yaml.NodeTagInt
case bool:
node.SetString(strconv.FormatBool(newValue))
node.Tag = yaml.NodeTagBool
case float32:
node.SetString(strconv.FormatFloat(float64(newValue), 'f', -1, 32))
node.Tag = yaml.NodeTagFloat
case float64:
node.SetString(strconv.FormatFloat(newValue, 'f', -1, 64))
node.Tag = yaml.NodeTagFloat
Expand Down
25 changes: 23 additions & 2 deletions api/internal/wrappy/wnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"log"
"regexp"
"strconv"
"strings"

"sigs.k8s.io/kustomize/api/ifc"
Expand Down Expand Up @@ -127,9 +128,29 @@ func (wn *WNode) GetFieldValue(path string) (interface{}, error) {
}
return result, nil
}
if yn.Kind != yaml.ScalarNode {
return nil, fmt.Errorf("expected ScalarNode, got Kind=%d", yn.Kind)
}

// Return value value directly for all other (ScalarNode) kinds
return yn.Value, nil
// TODO: When doing kustomize var replacement, which is likely a
// a primary use of this function and the reason it returns interface{}
// rather than string, we do conversion from Nodes to Go types and back
// to nodes. We should figure out how to do replacement using raw nodes,
// assuming we keep the var feature in kustomize.
// The other end of this is: refvar.go:updateNodeValue.
switch yn.Tag {
case yaml.NodeTagString:
return yn.Value, nil
case yaml.NodeTagInt:
return strconv.Atoi(yn.Value)
case yaml.NodeTagFloat:
return strconv.ParseFloat(yn.Value, 64)
case yaml.NodeTagBool:
return strconv.ParseBool(yn.Value)
default:
// Possibly this should be an error or log.
return yn.Value, nil
}
}

// GetGvk implements ifc.Kunstructured.
Expand Down
43 changes: 14 additions & 29 deletions api/krusty/variableref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package krusty_test

import (
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -404,18 +403,17 @@ spec:
- containerPort: 8888
name: http
`)
opts := th.MakeDefaultOptions()
m := th.Run(".", opts)
expFmt := `
m := th.Run(".", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: v1
kind: Service
metadata:
name: myService
spec:
ports:
- name: grpc
port: %s
targetPort: %s
port: 8888
targetPort: 8888
---
apiVersion: apps/v1beta1
kind: StatefulSet
Expand All @@ -432,13 +430,7 @@ spec:
name: grpc
- containerPort: 8888
name: http
`
th.AssertActualEqualsExpected(m,
// TODO(#3304): DECISION - quotes bad here, this is a bug.
opts.IfApiMachineryElseKyaml(
fmt.Sprintf(expFmt, `8888`, `8888`),
fmt.Sprintf(expFmt, `"8888"`, `"8888"`),
))
`)
}

// TODO(3449): Yield bare primitives in var replacements from configmaps.
Expand Down Expand Up @@ -551,7 +543,6 @@ metadata:

func TestVarRefBig(t *testing.T) {
th := kusttest_test.MakeHarness(t)
opts := th.MakeDefaultOptions()
th.WriteK("/app/base", `
namePrefix: base-
resources:
Expand Down Expand Up @@ -868,8 +859,8 @@ namePrefix: dev-
resources:
- ../../base
`)
m := th.Run("/app/overlay/staging", opts)
expFmt := `
m := th.Run("/app/overlay/staging", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: v1
kind: ServiceAccount
metadata:
Expand Down Expand Up @@ -956,8 +947,8 @@ spec:
port: 26257
targetPort: 26257
- name: http
port: %s
targetPort: %s
port: 8080
targetPort: 8080
selector:
app: cockroachdb
---
Expand All @@ -973,8 +964,8 @@ spec:
port: 26257
targetPort: 26257
- name: http
port: %s
targetPort: %s
port: 8080
targetPort: 8080
selector:
app: cockroachdb
---
Expand Down Expand Up @@ -1011,8 +1002,8 @@ spec:
- --host $(hostname -f)
- --http-host 0.0.0.0
- --join dev-base-cockroachdb-0.dev-base-cockroachdb,dev-base-cockroachdb-1.dev-base-cockroachdb,dev-base-cockroachdb-2.dev-base-cockroachdb
- --cache 25%%
- --max-sql-memory 25%%
- --cache 25%
- --max-sql-memory 25%
image: cockroachdb/cockroach:v1.1.5
imagePullPolicy: IfNotPresent
name: cockroachdb
Expand Down Expand Up @@ -1112,13 +1103,7 @@ data:
kind: ConfigMap
metadata:
name: dev-base-test-config-map-6b85g79g7g
`
th.AssertActualEqualsExpected(m,
// TODO(#3304): DECISION - quotes bad here, this still a bug.
opts.IfApiMachineryElseKyaml(
fmt.Sprintf(expFmt, `8080`, `8080`, `8080`, `8080`),
fmt.Sprintf(expFmt, `"8080"`, `"8080"`, `"8080"`, `"8080"`),
))
`)
}

func TestVariableRefIngressBasic(t *testing.T) {
Expand Down

0 comments on commit 065c2b8

Please sign in to comment.