[vtadmin] Add infrastructure for generating authz tests for vtadmin (#10397)

* Add infrastructure for generating authz tests for vtadmin

The lack of verifying authz checks are where they should be is one of the
most glaring issues in vtadmin (in my opinion; it's also my "fault" things
are this way). At the same time, writing all the code by hand to verify
every single endpoint would be a giant pain (which is the main reason
things are this way). So, let's codegen all the bits we don't care about!
The bonus here is that the config.json now can serve as authoritative on
what permissions are required for what endpoints.

The goal here is to have the config primarily specify the rules needed for
each endpoint, with as minimal "overhead" (currently specifying test cases
and mock data) as possible.

I want to separate the introduction of this setup from its complete
adoption, so I will submit a follow-up change that adds the rest of the
endpoint tests.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* add missing license headers

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Add make target and CI check

Signed-off-by: Andrew Mason <andrew@planetscale.com>

.github/workflows/check_make_vtadmin_authz_testgen.yml

@@ -0,0 +1,55 @@
+name: check_make_vtadmin_authz_testgen
+on: [push, pull_request]
+jobs:
+
+  build:
+    name: Check Make vtadmin_authz_testgen
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check out code
+      uses: actions/checkout@v2
+
+    - name: Check for changes in relevant files
+      uses: frouioui/paths-filter@main
+      id: changes
+      with:
+        token: ''
+        filters: |
+          vtadmin_changes:
+            - 'bootstrap.sh'
+            - 'tools/**'
+            - 'build.env'
+            - 'go.[sumod]'
+            - 'Makefile'
+            - 'go/vt/vtadmin/**'
+
+    - name: Set up Go
+      uses: actions/setup-go@v2
+      if: steps.changes.outputs.vtadmin_changes == 'true'
+      with:
+        go-version: 1.18.1
+
+    - name: Tune the OS
+      if: steps.changes.outputs.vtadmin_changes == 'true'
+      run: |
+        echo '1024 65535' | sudo tee -a /proc/sys/net/ipv4/ip_local_port_range
+
+
+    - name: Get dependencies
+      if: steps.changes.outputs.vtadmin_changes == 'true'
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y make unzip g++ etcd curl git wget
+        sudo service etcd stop
+        go mod download
+        go install golang.org/x/tools/cmd/goimports@latest
+
+    - name: Run make minimaltools
+      if: steps.changes.outputs.vtadmin_changes == 'true'
+      run: |
+        make minimaltools
+
+    - name: check_make_vtadmin_authz_testgen
+      if: steps.changes.outputs.vtadmin_changes == 'true'
+      run: |
+        tools/check_make_vtadmin_authz_testgen.sh

Makefile

@@ -444,6 +444,10 @@ vtadmin_web_install:
 vtadmin_web_proto_types: vtadmin_web_install
 	./web/vtadmin/bin/generate-proto-types.sh
 
+vtadmin_authz_testgen:
+	go generate ./go/vt/vtadmin/
+	go fmt ./go/vt/vtadmin/
+
 # Generate github CI actions workflow files for unit tests and cluster endtoend tests based on templates in the test/templates directory
 # Needs to be called if the templates change or if a new test "shard" is created. We do not need to rebuild tests if only the test/config.json
 # is changed by adding a new test to an existing shard. Any new or modified files need to be committed into git

go/vt/vtadmin/api_authz_test.go

@@ -0,0 +1,141 @@
+// Code generated by testutil/authztestgen. DO NOT EDIT.
+
+/*
+Copyright 2022 The Vitess Authors.
+
+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
+
+    http://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.
+*/
+
+package vtadmin_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/vt/vtadmin"
+	"vitess.io/vitess/go/vt/vtadmin/rbac"
+	"vitess.io/vitess/go/vt/vtadmin/testutil"
+	"vitess.io/vitess/go/vt/vtadmin/vtctldclient/fakevtctldclient"
+
+	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
+	vtadminpb "vitess.io/vitess/go/vt/proto/vtadmin"
+	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
+)
+
+func TestGetClusters(t *testing.T) {
+	opts := vtadmin.Options{
+		RBAC: &rbac.Config{
+			Rules: []*struct {
+				Resource string
+				Actions  []string
+				Subjects []string
+				Clusters []string
+			}{
+				{
+					Resource: "Cluster",
+					Actions:  []string{"get"},
+					Subjects: []string{"user:allowed"},
+					Clusters: []string{"*"},
+				},
+			},
+		},
+	}
+	err := opts.RBAC.Reify()
+	require.NoError(t, err, "failed to reify authorization rules: %+v", opts.RBAC.Rules)
+
+	api := vtadmin.NewAPI(
+		testutil.BuildClusters(t, testutil.TestClusterConfig{
+			Cluster: &vtadminpb.Cluster{
+				Id:   "test",
+				Name: "test",
+			},
+			VtctldClient: newVtctldClient(),
+			Tablets: []*vtadminpb.Tablet{
+				{
+					Tablet: &topodatapb.Tablet{
+						Alias: &topodatapb.TabletAlias{
+							Cell: "zone1",
+							Uid:  100,
+						},
+					},
+				},
+			},
+		}),
+		opts,
+	)
+
+	t.Cleanup(func() {
+		if err := api.Close(); err != nil {
+			t.Logf("api did not close cleanly: %s", err.Error())
+		}
+	})
+
+	t.Run("unauthenticated", func(t *testing.T) {
+		t.Parallel()
+		var actor *rbac.Actor
+
+		ctx := context.Background()
+		if actor != nil {
+			ctx = rbac.NewContext(ctx, actor)
+		}
+
+		resp, _ := api.GetClusters(ctx, &vtadminpb.GetClustersRequest{})
+		assert.Empty(t, resp.Clusters, "actor %+v should not be permitted to GetClusters", actor)
+	})
+
+	t.Run("unauthorized actor", func(t *testing.T) {
+		t.Parallel()
+		actor := &rbac.Actor{Name: "other"}
+
+		ctx := context.Background()
+		if actor != nil {
+			ctx = rbac.NewContext(ctx, actor)
+		}
+
+		resp, _ := api.GetClusters(ctx, &vtadminpb.GetClustersRequest{})
+		assert.Empty(t, resp.Clusters, "actor %+v should not be permitted to GetClusters", actor)
+	})
+
+	t.Run("authorized actor", func(t *testing.T) {
+		t.Parallel()
+		actor := &rbac.Actor{Name: "allowed"}
+
+		ctx := context.Background()
+		if actor != nil {
+			ctx = rbac.NewContext(ctx, actor)
+		}
+
+		resp, err := api.GetClusters(ctx, &vtadminpb.GetClustersRequest{})
+		require.NoError(t, err)
+		assert.NotEmpty(t, resp.Clusters, "actor %+v should be permitted to GetClusters", actor)
+	})
+}
+
+func newVtctldClient() *fakevtctldclient.VtctldClient {
+	return &fakevtctldclient.VtctldClient{
+		DeleteTabletsResults: map[string]error{
+			"zone1-0000000100": nil,
+		},
+		GetCellInfoNamesResults: &struct {
+			Response *vtctldatapb.GetCellInfoNamesResponse
+			Error    error
+		}{
+			Response: &vtctldatapb.GetCellInfoNamesResponse{
+				Names: []string{"zone1"},
+			},
+		},
+	}
+}

go/vt/vtadmin/api_test.go

@@ -5147,3 +5147,6 @@ func init() {
 	// attempts to read that value by way of grpc.NewServer().
 	grpccommon.EnableTracingOpt()
 }
+
+//go:generate -command authztestgen go run ./testutil/authztestgen
+//go:generate authztestgen -c ./testutil/authztestgen/config.json -o ./api_authz_test.go

go/vt/vtadmin/testutil/authztestgen/config.json

@@ -0,0 +1,62 @@
+{
+    "package": "vtadmin_test",
+    "vtctldclient_mock_data": [
+        {
+            "field": "DeleteTabletsResults",
+            "type": "map[string]error",
+            "value": "\"zone1-0000000100\": nil,"
+        },
+        {
+            "field": "GetCellInfoNamesResults",
+            "type": "&struct{\nResponse *vtctldatapb.GetCellInfoNamesResponse\nError error}",
+            "value": "Response: &vtctldatapb.GetCellInfoNamesResponse{\nNames: []string{\"zone1\"},\n},"
+        }
+    ],
+    "db_tablet_list": [
+        {
+            "tablet": {
+                "alias": {"cell": "zone1", "uid": 100}
+            }
+        }
+    ],
+    "tests": [
+        {
+            "method": "GetClusters",
+            "rules": [
+                {
+                    "resource": "Cluster",
+                    "actions": ["get"],
+                    "subjects": ["user:allowed"],
+                    "clusters": ["*"]
+                }
+            ],
+            "request": "&vtadminpb.GetClustersRequest{}",
+            "cases": [
+                {
+                    "name": "unauthenticated",
+                    "actor": null,
+                    "assertions": [
+                        "assert.Empty(t, resp.Clusters, $$)"
+                    ]
+                },
+                {
+                    "name": "unauthorized actor",
+                    "actor": {"name": "other"},
+                    "assertions": [
+                        "assert.Empty(t, resp.Clusters, $$)"
+                    ]
+                },
+                {
+                    "name": "authorized actor",
+                    "actor": {"name": "allowed"},
+                    "is_permitted": true,
+                    "include_error_var": true,
+                    "assertions": [
+                        "require.NoError(t, err)",
+                        "assert.NotEmpty(t, resp.Clusters, $$)"
+                    ]
+                }
+            ]
+        }
+    ]
+}

go/vt/vtadmin/testutil/authztestgen/functions.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2022 The Vitess Authors.
+
+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
+
+    http://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.
+*/
+
+package main
+
+import (
+	"strings"
+
+	"vitess.io/vitess/go/vt/vtadmin/rbac"
+)
+
+func getActor(actor *rbac.Actor) string {
+	var buf strings.Builder
+	switch actor {
+	case nil:
+		buf.WriteString("var actor *rbac.Actor")
+	default:
+		buf.WriteString("actor := ")
+		_inlineActor(&buf, *actor)
+	}
+
+	return buf.String()
+}
+
+func _inlineActor(buf *strings.Builder, actor rbac.Actor) {
+	buf.WriteString(`&rbac.Actor{Name: "`)
+	buf.WriteString(actor.Name)
+	buf.WriteString(`"`)
+	if actor.Roles != nil {
+		buf.WriteString(", Roles: []string{")
+		for i, role := range actor.Roles {
+			buf.WriteString(strings.Join([]string{`"`, role, `"`}, ""))
+			if i != len(actor.Roles)-1 {
+				buf.WriteString(", ")
+			}
+		}
+
+		buf.WriteString("}")
+	}
+
+	buf.WriteString("}")
+}
+
+func writeAssertion(line string, test *Test, testCase *TestCase) string {
+	if !strings.Contains(line, "$$") {
+		return line
+	}
+
+	var msg strings.Builder
+	msg.WriteString(`"actor %+v should `)
+	if !testCase.IsPermitted {
+		msg.WriteString("not ")
+	}
+
+	msg.WriteString("be permitted to ")
+	msg.WriteString(test.Method)
+	msg.WriteString(`", actor`)
+
+	return strings.ReplaceAll(line, "$$", msg.String())
+}

go/vt/vtadmin/testutil/authztestgen/main.go

@@ -0,0 +1,104 @@
+/*
+Copyright 2022 The Vitess Authors.
+
+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
+
+    http://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.
+*/
+
+package main
+
+import (
+	"encoding/json"
+	"io"
+	"os"
+	"text/template"
+
+	"github.com/spf13/pflag"
+
+	"vitess.io/vitess/go/vt/vtadmin/rbac"
+
+	vtadminpb "vitess.io/vitess/go/vt/proto/vtadmin"
+)
+
+type Config struct {
+	Package                 string                    `json:"package"`
+	Tests                   []*Test                   `json:"tests"`
+	FakeVtctldClientResults []*FakeVtctldClientResult `json:"vtctldclient_mock_data"`
+	DBTablets               []*vtadminpb.Tablet       `json:"db_tablet_list"`
+}
+
+type Test struct {
+	Method  string        `json:"method"`
+	Rules   []*AuthzRules `json:"rules"`
+	Request string        `json:"request"`
+	Cases   []*TestCase   `json:"cases"`
+}
+
+type TestCase struct {
+	Name            string      `json:"name"`
+	Actor           *rbac.Actor `json:"actor"`
+	IsPermitted     bool        `json:"is_permitted"`
+	IncludeErrorVar bool        `json:"include_error_var"`
+	Assertions      []string    `json:"assertions"`
+}
+
+type AuthzRules struct {
+	Resource string   `json:"resource"`
+	Actions  []string `json:"actions"`
+	Subjects []string `json:"subjects"`
+	Clusters []string `json:"clusters"`
+}
+
+type FakeVtctldClientResult struct {
+	FieldName string `json:"field"`
+	Type      string `json:"type"`
+	Value     string `json:"value"`
+}
+
+func panicIf(err error) {
+	if err != nil {
+		panic(err)
+	}
+}
+
+func main() {
+	path := pflag.StringP("config", "c", "config.json", "authztest configuration (see the Config type in this package for the spec)")
+	pflag.StringVarP(path, "config-path", "p", "config.json", "alias for --config")
+	outputPath := pflag.StringP("output", "o", "", "destination to write generated code. if empty, defaults to os.Stdout")
+
+	pflag.Parse()
+
+	data, err := os.ReadFile(*path)
+	panicIf(err)
+
+	var cfg Config
+	err = json.Unmarshal(data, &cfg)
+	panicIf(err)
+
+	tmpl, err := template.New("tests").Funcs(map[string]any{
+		"getActor":       getActor,
+		"writeAssertion": writeAssertion,
+	}).Parse(_t)
+	panicIf(err)
+
+	var output io.Writer = os.Stdout
+	if *outputPath != "" {
+		f, err := os.Create(*outputPath)
+		panicIf(err)
+
+		defer f.Close()
+		output = f
+	}
+
+	err = tmpl.Execute(output, &cfg)
+	panicIf(err)
+}

go/vt/vtadmin/testutil/authztestgen/template.go

@@ -0,0 +1,143 @@
+/*
+Copyright 2022 The Vitess Authors.
+
+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
+
+    http://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.
+*/
+
+package main
+
+const _t = `// Code generated by testutil/authztestgen. DO NOT EDIT.
+
+/*
+Copyright 2022 The Vitess Authors.
+
+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
+
+    http://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.
+*/
+
+package {{ .Package }}
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/vt/vtadmin"
+	"vitess.io/vitess/go/vt/vtadmin/rbac"
+	"vitess.io/vitess/go/vt/vtadmin/testutil"
+	"vitess.io/vitess/go/vt/vtadmin/vtctldclient/fakevtctldclient"
+
+	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
+	vtadminpb "vitess.io/vitess/go/vt/proto/vtadmin"
+	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
+)
+
+{{ range .Tests }}
+func Test{{ .Method }}(t *testing.T) {
+	opts := vtadmin.Options{
+		RBAC: &rbac.Config{
+			Rules: []*struct{
+				Resource string
+				Actions  []string
+				Subjects []string
+				Clusters []string
+			}{
+				{{- range .Rules }}
+				{
+					Resource: "{{ .Resource }}",
+					Actions:  []string{ {{ range .Actions }}"{{ . }}",{{ end }} },
+					Subjects: []string{ {{ range .Subjects }}"{{ . }}",{{ end }} },
+					Clusters: []string{ {{ range .Clusters }}"{{ . }}",{{ end }} },
+				},
+				{{- end }}
+			},
+		},
+	}
+	err := opts.RBAC.Reify()
+	require.NoError(t, err, "failed to reify authorization rules: %+v", opts.RBAC.Rules)
+
+	api := vtadmin.NewAPI(
+		testutil.BuildClusters(t, testutil.TestClusterConfig{
+			Cluster: &vtadminpb.Cluster{
+				Id:   "test",
+				Name: "test",
+			},
+			VtctldClient: newVtctldClient(),
+			Tablets: []*vtadminpb.Tablet{
+				{{ range $.DBTablets -}}
+				{
+					Tablet: &topodatapb.Tablet{
+						Alias: &topodatapb.TabletAlias{
+							Cell: "{{ .Tablet.Alias.Cell }}",
+							Uid: {{ .Tablet.Alias.Uid }},
+						},
+					},
+				},
+				{{- end }}
+			},
+		}),
+		opts,
+	)
+
+	t.Cleanup(func() {
+		if err := api.Close(); err != nil {
+			t.Logf("api did not close cleanly: %s", err.Error())
+		}
+	})
+	{{ with $test := . -}}
+	{{ range .Cases }}
+	t.Run("{{ .Name }}", func(t *testing.T) {
+		t.Parallel()
+		{{ getActor .Actor }}
+
+		ctx := context.Background()
+		if actor != nil {
+			ctx = rbac.NewContext(ctx, actor)
+		}
+		{{ if .IncludeErrorVar }}
+		resp, err := api.{{ $test.Method }}(ctx, {{ $test.Request }})
+		{{ else }}
+		resp, _ := api.{{ $test.Method }}(ctx, {{ $test.Request }})
+		{{ end }}
+		{{- with $case := . -}}
+		{{ range .Assertions }}
+		{{- writeAssertion . $test $case }}
+		{{ end }}
+		{{- end -}}
+	})
+	{{ end }}
+	{{- end -}}
+}
+{{- end }}
+
+func newVtctldClient() *fakevtctldclient.VtctldClient {
+	return &fakevtctldclient.VtctldClient{
+		{{- range .FakeVtctldClientResults }}
+		{{ .FieldName }}: {{ .Type }}{
+			{{ .Value }}
+		},
+		{{- end }}
+	}
+}
+`

tools/check_make_vtadmin_authz_testgen.sh

@@ -0,0 +1,17 @@
+#!/bin/bash
+
+source build.env
+
+first_output=$(git status --porcelain)
+
+make vtadmin_authz_testgen
+
+second_output=$(git status --porcelain)
+
+diff=$(diff <( echo "$first_output") <( echo "$second_output"))
+
+if [[ "$diff" != "" ]]; then
+  echo "ERROR: Regenerated vtadmin_test files do not match the current version."
+  echo -e "List of files containing differences:\n$diff"
+  exit 1
+fi