chore: Use t.Setenv in tests (open-policy-agent#5321)

And enable the `tenv` linter for the future. Also, bump version of golangci-lint and fix some new warnings that came from that. Signed-off-by: Anders Eknert <[email protected]>
truera · Oct 27, 2022 · 50d4e31 · 50d4e31
1 parent 6ec07ca
commit 50d4e31
Show file tree

Hide file tree

Showing 24 changed files with 95 additions and 143 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -21,6 +21,7 @@ linters:
     - varcheck
     - deadcode
     - misspell
+    - tenv
     - typecheck
     - structcheck
     - staticcheck

diff --git a/Makefile b/Makefile
@@ -27,7 +27,7 @@ ifeq ($(WASM_ENABLED),1)
 GO_TAGS = -tags=opa_wasm
 endif
 
-GOLANGCI_LINT_VERSION := v1.46.2
+GOLANGCI_LINT_VERSION := v1.50.1
 
 DOCKER_RUNNING ?= $(shell docker ps >/dev/null 2>&1 && echo 1 || echo 0)
 

diff --git a/ast/doc.go b/ast/doc.go
@@ -8,29 +8,29 @@
 //
 // Rego policies are typically defined in text files and then parsed and compiled by the policy engine at runtime. The parsing stage takes the text or string representation of the policy and converts it into an abstract syntax tree (AST) that consists of the types mentioned above. The AST is organized as follows:
 //
-//  Module
-//   |
-//   +--- Package (Reference)
-//   |
-//   +--- Imports
-//   |     |
-//   |     +--- Import (Term)
-//   |
-//   +--- Rules
-//         |
-//         +--- Rule
-//               |
-//               +--- Head
-//               |     |
-//               |     +--- Name (Variable)
-//               |     |
-//               |     +--- Key (Term)
-//               |     |
-//               |     +--- Value (Term)
-//               |
-//               +--- Body
-//                     |
-//                     +--- Expression (Term | Terms | Variable Declaration)
+//	Module
+//	 |
+//	 +--- Package (Reference)
+//	 |
+//	 +--- Imports
+//	 |     |
+//	 |     +--- Import (Term)
+//	 |
+//	 +--- Rules
+//	       |
+//	       +--- Rule
+//	             |
+//	             +--- Head
+//	             |     |
+//	             |     +--- Name (Variable)
+//	             |     |
+//	             |     +--- Key (Term)
+//	             |     |
+//	             |     +--- Value (Term)
+//	             |
+//	             +--- Body
+//	                   |
+//	                   +--- Expression (Term | Terms | Variable Declaration)
 //
 // At query time, the policy engine expects policies to have been compiled. The compilation stage takes one or more modules and compiles them into a format that the policy engine supports.
 package ast
diff --git a/ast/term_bench_test.go b/ast/term_bench_test.go
@@ -152,9 +152,12 @@ var bs []byte
 
 // BenchmarkObjectString generates several objects of different sizes, and
 // marshals them to JSON via two ways:
-//   map[string]int -> ast.Value -> .String()
+//
+//	map[string]int -> ast.Value -> .String()
+//
 // and
-//   map[string]int -> json.Marshal()
+//
+//	map[string]int -> json.Marshal()
 //
 // The difference between these two is relevant for feeding input into the
 // wasm vm: when calling rego.New(...) with rego.Target("wasm"), it's up to

diff --git a/capabilities/capabilities.go b/capabilities/capabilities.go
@@ -14,5 +14,6 @@ import (
 // FS contains the embedded capabilities/ directory of the built version,
 // which has all the capabilities of previous versions:
 // "v0.18.0.json" contains the capabilities JSON of version v0.18.0, etc
+//
 //go:embed *.json
 var FS embed.FS
diff --git a/cmd/version_test.go b/cmd/version_test.go
@@ -9,7 +9,6 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"sort"
 	"strings"
 	"testing"
@@ -44,7 +43,7 @@ func TestGenerateCmdOutputWithCheckFlagNoError(t *testing.T) {
 	baseURL, teardown := getTestServer(exp, http.StatusOK)
 	defer teardown()
 
-	os.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
+	t.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
 
 	var stdout bytes.Buffer
 
@@ -66,7 +65,7 @@ func TestGenerateCmdOutputWithCheckFlagNoError(t *testing.T) {
 
 func TestCheckOPAUpdateBadURL(t *testing.T) {
 	url := "http://foo:8112"
-	os.Setenv("OPA_TELEMETRY_SERVICE_URL", url)
+	t.Setenv("OPA_TELEMETRY_SERVICE_URL", url)
 
 	err := checkOPAUpdate(nil)
 	if err == nil {

diff --git a/dependencies/deps.go b/dependencies/deps.go
@@ -200,9 +200,9 @@ func filter(rs []ast.Ref, pred func(ast.Ref, ast.Ref) bool) (filtered []ast.Ref)
 // complicated. It should be possible to compute all dependencies in two
 // passes:
 //
-// 1) perform syntactic unification on vars
-// 2) gather all refs rooted at data after plugging the head with substitution
-//    from (1)
+//  1. perform syntactic unification on vars
+//  2. gather all refs rooted at data after plugging the head with substitution
+//     from (1)
 func ruleDeps(rule *ast.Rule) (resolved []ast.Ref) {
 	vars, others := extractEq(rule.Body)
 	joined := joinVarRefs(vars)

diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -19,7 +19,7 @@ import (
 
 func setTestEnvVar(t *testing.T, name, value string) string {
 	envKey := fmt.Sprintf("%s_%s", t.Name(), name)
-	os.Setenv(envKey, value)
+	t.Setenv(envKey, value)
 	return envKey
 }
 

diff --git a/internal/gojsonschema/schema.go b/internal/gojsonschema/schema.go
@@ -73,7 +73,6 @@ func (d *Schema) SetRootSchemaName(name string) {
 // Pretty long function ( sorry :) )... but pretty straight forward, repetitive and boring
 // Not much magic involved here, most of the job is to validate the key names and their values,
 // then the values are copied into SubSchema struct
-//
 func (d *Schema) parseSchema(documentNode interface{}, currentSchema *SubSchema) error {
 
 	if currentSchema.Draft == nil {

diff --git a/internal/gojsonschema/schemaLoader.go b/internal/gojsonschema/schemaLoader.go
@@ -119,7 +119,7 @@ func (sl *SchemaLoader) AddSchemas(loaders ...JSONLoader) error {
 	return nil
 }
 
-//AddSchema adds a schema under the provided URL to the schema cache
+// AddSchema adds a schema under the provided URL to the schema cache
 func (sl *SchemaLoader) AddSchema(url string, loader JSONLoader) error {
 
 	ref, err := gojsonreference.NewJsonReference(url)

diff --git a/internal/jwx/jws/jws.go b/internal/jwx/jws/jws.go
@@ -4,8 +4,8 @@
 // If you do not care about the details, the only things that you
 // would need to use are the following functions:
 //
-//     jws.SignWithOption(Payload, algorithm, key)
-//     jws.Verify(encodedjws, algorithm, key)
+//	jws.SignWithOption(Payload, algorithm, key)
+//	jws.Verify(encodedjws, algorithm, key)
 //
 // To sign, simply use `jws.SignWithOption`. `Payload` is a []byte buffer that
 // contains whatever data you want to sign. `alg` is one of the
@@ -38,7 +38,6 @@ import (
 // SignLiteral generates a Signature for the given Payload and Headers, and serializes
 // it in compact serialization format. In this format you may NOT use
 // multiple signers.
-//
 func SignLiteral(payload []byte, alg jwa.SignatureAlgorithm, key interface{}, hdrBuf []byte, rnd io.Reader) ([]byte, error) {
 	encodedHdr := base64.RawURLEncoding.EncodeToString(hdrBuf)
 	encodedPayload := base64.RawURLEncoding.EncodeToString(payload)

diff --git a/internal/report/report.go b/internal/report/report.go
@@ -30,7 +30,7 @@ import (
 //
 // Override at build time via:
 //
-//    -ldflags "-X github.com/open-policy-agent/opa/internal/report.ExternalServiceURL=<url>"
+//	-ldflags "-X github.com/open-policy-agent/opa/internal/report.ExternalServiceURL=<url>"
 //
 // This will be overridden if the OPA_TELEMETRY_SERVICE_URL environment variable
 // is provided.

diff --git a/internal/report/report_test.go b/internal/report/report_test.go
@@ -9,13 +9,11 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"reflect"
 	"testing"
 )
 
 func TestNewReportDefaultURL(t *testing.T) {
-	os.Unsetenv("OPA_TELEMETRY_SERVICE_URL")
 
 	reporter, err := New("", Options{})
 	if err != nil {
@@ -34,7 +32,7 @@ func TestSendReportBadRespStatus(t *testing.T) {
 	baseURL, teardown := getTestServer(nil, http.StatusBadRequest)
 	defer teardown()
 
-	os.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
+	t.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
 
 	reporter, err := New("", Options{})
 	if err != nil {
@@ -59,7 +57,7 @@ func TestSendReportDecodeError(t *testing.T) {
 	baseURL, teardown := getTestServer("foo", http.StatusOK)
 	defer teardown()
 
-	os.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
+	t.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
 
 	reporter, err := New("", Options{})
 	if err != nil {
@@ -85,7 +83,7 @@ func TestSendReportWithOPAUpdate(t *testing.T) {
 	baseURL, teardown := getTestServer(exp, http.StatusOK)
 	defer teardown()
 
-	os.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
+	t.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
 
 	reporter, err := New("", Options{})
 	if err != nil {
@@ -108,7 +106,7 @@ func TestReportWithHeapStats(t *testing.T) {
 	baseURL, teardown := getTestServer(nil, http.StatusOK)
 	defer teardown()
 
-	os.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
+	t.Setenv("OPA_TELEMETRY_SERVICE_URL", baseURL)
 
 	reporter, err := New("", Options{})
 	if err != nil {

diff --git a/internal/strvals/doc.go b/internal/strvals/doc.go
@@ -13,7 +13,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-/*Package strvals provides tools for working with strval lines.
+/*
+Package strvals provides tools for working with strval lines.
 
 OPA runtime config supports a compressed format for YAML settings which we call strvals.
 The format is roughly like this:

diff --git a/plugins/plugins.go b/plugins/plugins.go
@@ -38,11 +38,11 @@ import (
 // configuration blob. If your plugin has not been configured, your
 // factory will not be invoked.
 //
-//   plugins:
-//     my_plugin1:
-//       some_key: foo
-//     # my_plugin2:
-//     #   some_key2: bar
+//	plugins:
+//	  my_plugin1:
+//	    some_key: foo
+//	  # my_plugin2:
+//	  #   some_key2: bar
 //
 // If OPA was started with the configuration above and received two
 // calls to runtime.RegisterPlugins (one with NAME "my_plugin1" and