Repository: google/go-flow-levee
Pull request #163 (2020-10-30 17:52:23)

Allow use of YAML for config declaration.
- [x] Tests pass
- [x] Running against a large codebase such as [Kubernetes](https://github.com/kubernetes/kubernetes) does not error out.
- [x] Appropriate changes to README are included in PR

---

Using the `sig.k8s.io` fork of the `yaml` package allows us to avoid a great deal of manual field tagging of our config structs, e.g. `yaml:"Sanitizers,omitempty"`.  It retains the `json` unmarshallers and compatibility between JSON and YAML formats.  So that's nice.

PurelyApplied: > Why support both? I thought we were moving to YAML exclusively.

YAML is a superset of JSON.  Therre is no "YAML exclusively."  Valid JSON _is_ valid YAML.

Testing against both was more in the idea of a blue-green deployment.  I could drop the dual test if you think it reads better without the JSON version.
mlevesquedion: > Testing against both was more in the idea of a blue-green deployment. I could drop the dual test if you think it reads better without the JSON version.

I understand your approach, but to me it feels like we're testing the YAML package right now. I don't think we need explicit tests for the JSON version.
PurelyApplied: Comment for cross-referencing: This PR emerged for discussion in #157 

Patch:
diff --git a/.travis.yml b/.travis.yml
index 68d42ab..cf1de1f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,6 +5,7 @@ go:
 
 before_script:
 - go install golang.org/x/tools/cmd/goimports
+- sudo apt-get -y install yamllint
 
 script:
 - go test -v ./...
@@ -35,3 +36,4 @@ script:
     fi
   )
 - echo "Checking json..." && find . -name '*.json' -exec jq type {} + &>/dev/null
+- echo "Checking yaml..." && find . -name '*.yaml' -exec yamllint {} + &>/dev/null
diff --git a/configuration/README.md b/configuration/README.md
index 6483030..f033fda 100644
--- a/configuration/README.md
+++ b/configuration/README.md
@@ -11,21 +11,16 @@ Alternatively, build this repository from its root directory via `go build -o /i
 
 For design details concerning value and instruction classification, see [/design](../design/README.md).
 
-Configuration is provided to `go-flow-levee` via JSON.
+Configuration is provided to `go-flow-levee` via YAML.
 
 Objects of interest are identified primarily via regexp. An empty regexp will match any string.
 
 Sources are identified via regexp according to package, type, and field names.
-```json
-{
-  "Sources": [
-    {
-      "PackageRE": "<package path regexp>",
-      "TypeRE": "<type name regexp>",
-      "FieldRE": "<field name regexp>"
-    }
-  ]
-}
+```yaml
+Sources:
+- PackageRE: "<package path regexp>"
+  TypeRE: "<type name regexp>"
+  FieldRE: "<field name regexp>"
 ```
 
 Sources may also be identified via field tags:
@@ -36,36 +31,23 @@ type Example struct {
 ```
 
 The tag `levee:"source"` is built-in. Additional tags may be identified via explicit string literals (not regexps). The following example shows how the `levee:"source"` tag could be defined if it weren't built-in:
-```json
-{
-	"FieldTags": [
-		{
-			"Key": "levee",
-			"Val": "source"
-		}
-	]
-}
+```yaml
+FieldTags:
+- Key: levee
+  Val: source
 ```
 
 Sinks and sanitizers are identified via regexp according to package, method, and (optional) receiver name.
 
-```json
-{
-  "Sinks": [
-    {
-      "PackageRE": "<package path regexp>",
-      "ReceiverRE": "<type name regexp>",
-      "MethodRE": "<method name regexp>"
-    }
-  ],
-  "Sanitizers": [
-    {
-      "PackageRE": "<package path regexp>",
-      "ReceiverRE": "<type name regexp>",
-      "MethodRE": "<method name regexp>"
-    }
-  ]
-}
+```yaml
+Sinks:
+- PackageRE: <package path regexp>
+  ReceiverRE: <type name regexp>
+  MethodRE: <method name regexp>
+Sanitizers:
+- PackageRE: <package path regexp>
+  ReceiverRE: <type name regexp>
+  MethodRE: <method name regexp>
 ```
 
 Taint propagation is performed automatically and does not need to be explicitly configured.
@@ -77,15 +59,10 @@ To match only methods without any receiver (i.e., a top-level function), use the
 
 Functions can be explicitly excluded from analysis using regexps,
 constructed similarly to those used to identify sanitizers and sinks:
-```json
-{
-  "Exclude": [
-    {
-      "PathRE": "^myproject/mypackage$",
-      "MethodRE": "^myfunction$"
-    }
-  ]
-}
+```yaml
+Exclude:
+- PathRE: "^myproject/mypackage$"
+  MethodRE: "^myfunction$"
 ```
 
 The above will match the function `myfunction` from the `myproject/mypackage` package. It will also match a method named `myfunction` in the same package.
diff --git a/configuration/example-config.json b/configuration/example-config.json
deleted file mode 100644
index 65f419b..0000000
--- a/configuration/example-config.json
+++ /dev/null
@@ -1,37 +0,0 @@
-{
-  "Sources": [
-    {
-      "PackageRE": "",
-      "TypeRE": "^Secret$|Token",
-      "FieldRE": ""
-    },
-    {
-      "PackageRE": "k8s.io/client-go/tools/clientcmd/api(?:/v1)?",
-      "TypeRE": "^(?:Named)?AuthInfo$",
-      "FieldRE": ""
-    },
-    {
-      "PackageRE": "^k8s.io/client-go/rest$",
-      "TypeRE": "^TLSClientConfig$",
-      "FieldRE": "Password|BearerToken$|"
-    },
-    {
-      "PackageRE": "^k8s.io/client-go/rest$",
-      "TypeRE": "^Config$",
-      "FieldRE": "Password|BearerToken$|"
-    }
-  ],
-  "Sinks": [
-    {
-      "PackageRE": "log",
-      "ReceiverRE": "",
-      "MethodRE": "Info|Warning|Error|Fatal|Exit"
-    }
-  ],
-  "Sanitizers": [],
-  "Exclude": [
-    {
-      "PathRE": "^k8s.io/kubernetes/test"
-    }
-  ]
-}
diff --git a/configuration/example-config.yaml b/configuration/example-config.yaml
new file mode 100644
index 0000000..aedba01
--- /dev/null
+++ b/configuration/example-config.yaml
@@ -0,0 +1,33 @@
+# Copyright 2020 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+---
+
+Sources:
+- TypeRE: "^Secret$|Token"
+- PackageRE: "k8s.io/client-go/tools/clientcmd/api(?:/v1)?"
+  TypeRE: "^(?:Named)?AuthInfo$"
+  FieldRE: ""
+- PackageRE: "^k8s.io/client-go/rest$"
+  TypeRE: "^TLSClientConfig$"
+  FieldRE: "Password|BearerToken$|"
+- PackageRE: "^k8s.io/client-go/rest$"
+  TypeRE: "^Config$"
+  FieldRE: "Password|BearerToken$|"
+Sinks:
+- PackageRE: "log"
+  ReceiverRE: ""
+  MethodRE: "Info|Warning|Error|Fatal|Exit"
+Sanitizers: []
+Exclude:
+- PackageRE: "^k8s.io/kubernetes/test"
diff --git a/go.mod b/go.mod
index 787f951..ae8ac5f 100644
--- a/go.mod
+++ b/go.mod
@@ -5,4 +5,5 @@ go 1.14
 require (
 	github.com/google/go-cmp v0.5.2
 	golang.org/x/tools v0.0.0-20200416214402-fc959738d646
+	sigs.k8s.io/yaml v1.2.0
 )
diff --git a/go.sum b/go.sum
index 4d08442..269efb6 100644
--- a/go.sum
+++ b/go.sum
@@ -1,3 +1,5 @@
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
@@ -20,3 +22,9 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
+gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=
+sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 236040f..c188a39 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -15,7 +15,6 @@
 package config
 
 import (
-	"encoding/json"
 	"flag"
 	"fmt"
 	"go/types"
@@ -25,6 +24,8 @@ import (
 	"strings"
 	"sync"
 
+	"sigs.k8s.io/yaml"
+
 	"github.com/google/go-flow-levee/internal/pkg/config/regexp"
 	"github.com/google/go-flow-levee/internal/pkg/utils"
 	"golang.org/x/tools/go/ssa"
@@ -228,9 +229,7 @@ var readConfigCached *Config
 var readConfigCachedErr error
 
 func ReadConfig() (*Config, error) {
-	loadedFromCache := true
 	readFileOnce.Do(func() {
-		loadedFromCache = false
 		c := new(Config)
 		bytes, err := ioutil.ReadFile(configFile)
 		if err != nil {
@@ -238,12 +237,11 @@ func ReadConfig() (*Config, error) {
 			return
 		}
 
-		if err := json.Unmarshal(bytes, c); err != nil {
+		if err := yaml.UnmarshalStrict(bytes, c); err != nil {
 			readConfigCachedErr = err
 			return
 		}
 		readConfigCached = c
 	})
-	_ = loadedFromCache
 	return readConfigCached, readConfigCachedErr
 }


Test patch:
diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go
index a2d1cce..05b43c0 100644
--- a/internal/pkg/config/config_test.go
+++ b/internal/pkg/config/config_test.go
@@ -60,7 +60,7 @@ func runTest(pass *analysis.Pass) (interface{}, error) {
 
 func TestConfig(t *testing.T) {
 	testdata := analysistest.TestData()
-	if err := FlagSet.Set("config", filepath.Join(testdata, "test-config.json")); err != nil {
+	if err := FlagSet.Set("config", filepath.Join(testdata, "test-config.yaml")); err != nil {
 		t.Fatal(err)
 	}
 	for _, p := range []string{"core", "crosspkg", "exclusion", "notcore"} {
diff --git a/internal/pkg/config/testdata/test-config.json b/internal/pkg/config/testdata/test-config.json
deleted file mode 100644
index 72704b9..0000000
--- a/internal/pkg/config/testdata/test-config.json
+++ /dev/null
@@ -1,28 +0,0 @@
-{
-  "Sinks": [
-    {
-      "PackageRE": "^example.com/core$",
-      "MethodRE": "^Sink$"
-    },
-    {
-      "PackageRE": "^example.com/core$",
-      "MethodRE": "^Do$",
-      "ReceiverRE": "^Sinker$"
-    }
-  ],
-  "FieldTags": [
-  	{
-  		"Key": "example",
-  		"Val": "sensitive"
-  	}
-  ],
-  "Exclude": [
-    {
-      "PackageRE": "^example.com/exclusion$",
-      "MethodRE": "^Foo$"
-    },
-    {
-      "PackageRE": "^notexample.com/exclusion$"
-    }
-  ]
-}
diff --git a/internal/pkg/config/testdata/test-config.yaml b/internal/pkg/config/testdata/test-config.yaml
new file mode 100644
index 0000000..0340f10
--- /dev/null
+++ b/internal/pkg/config/testdata/test-config.yaml
@@ -0,0 +1,27 @@
+# Copyright 2020 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+---
+sinks:
+  - packageRE: "^example.com/core"
+    methodRE: "^Sink$"
+  - packageRE: "^example.com/core"
+    methodRE: "^Do$"
+    receiverRE: "^Sinker$"
+fieldTags:
+  - key: example
+    val: sensitive
+exclude:
+  - packageRE: "^example.com/exclusion$"
+    methodRE: "^Foo$"
+  - packageRE: "^notexample.com/exclusion$"