diff --git a/go/test/endtoend/cluster/vtgate_process.go b/go/test/endtoend/cluster/vtgate_process.go index c01f7c6e93b..1290156a1cd 100644 --- a/go/test/endtoend/cluster/vtgate_process.go +++ b/go/test/endtoend/cluster/vtgate_process.go @@ -132,6 +132,7 @@ func (vtgate *VtgateProcess) Setup() (err error) { return err } vtgate.proc.Stderr = errFile + vtgate.ErrorLog = errFile.Name() vtgate.proc.Env = append(vtgate.proc.Env, os.Environ()...) vtgate.proc.Env = append(vtgate.proc.Env, DefaultVttestEnv) diff --git a/go/test/endtoend/vtgate/vschema/vschema_test.go b/go/test/endtoend/vtgate/vschema/vschema_test.go index eec54f8f47f..d07a9f59092 100644 --- a/go/test/endtoend/vtgate/vschema/vschema_test.go +++ b/go/test/endtoend/vtgate/vschema/vschema_test.go @@ -18,21 +18,25 @@ package vschema import ( "context" + "encoding/json" "flag" "fmt" "os" + "path" "testing" + "time" - "vitess.io/vitess/go/test/endtoend/utils" - + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/test/endtoend/cluster" + "vitess.io/vitess/go/test/endtoend/utils" ) var ( clusterInstance *cluster.LocalProcessCluster + configFile string vtParams mysql.ConnParams hostname = "localhost" keyspaceName = "ks" @@ -53,7 +57,6 @@ var ( ) func TestMain(m *testing.M) { - defer cluster.PanicHandler(nil) flag.Parse() exitcode, err := func() (int, error) { @@ -66,7 +69,21 @@ func TestMain(m *testing.M) { } // List of users authorized to execute vschema ddl operations - clusterInstance.VtGateExtraArgs = []string{"--vschema_ddl_authorized_users=%", "--schema_change_signal=false"} + if utils.BinaryIsAtLeastAtVersion(22, "vtgate") { + timeNow := time.Now().Unix() + configFile = path.Join(os.TempDir(), fmt.Sprintf("vtgate-config-%d.json", timeNow)) + err := writeConfig(configFile, map[string]string{ + "vschema_ddl_authorized_users": "%", + }) + if err != nil { + return 1, err + } + defer os.Remove(configFile) + + clusterInstance.VtGateExtraArgs = []string{fmt.Sprintf("--config-file=%s", configFile), "--schema_change_signal=false"} + } else { + clusterInstance.VtGateExtraArgs = []string{"--vschema_ddl_authorized_users=%", "--schema_change_signal=false"} + } // Start keyspace keyspace := &cluster.Keyspace{ @@ -96,6 +113,15 @@ func TestMain(m *testing.M) { } +func writeConfig(path string, cfg map[string]string) error { + file, err := os.Create(path) + if err != nil { + return err + } + defer file.Close() + return json.NewEncoder(file).Encode(cfg) +} + func TestVSchema(t *testing.T) { defer cluster.PanicHandler(t) ctx := context.Background() @@ -138,4 +164,15 @@ func TestVSchema(t *testing.T) { utils.AssertMatches(t, conn, "delete from vt_user", `[]`) + if utils.BinaryIsAtLeastAtVersion(22, "vtgate") { + writeConfig(configFile, map[string]string{ + "vschema_ddl_authorized_users": "", + }) + + require.EventuallyWithT(t, func(t *assert.CollectT) { + _, err = conn.ExecuteFetch("ALTER VSCHEMA DROP TABLE main", 1000, false) + assert.Error(t, err) + assert.ErrorContains(t, err, "is not authorized to perform vschema operations") + }, 5*time.Second, 100*time.Millisecond) + } } diff --git a/go/viperutil/internal/sync/sync.go b/go/viperutil/internal/sync/sync.go index 6bee1a14e72..f69829f734d 100644 --- a/go/viperutil/internal/sync/sync.go +++ b/go/viperutil/internal/sync/sync.go @@ -86,7 +86,7 @@ func (v *Viper) Set(key string, value any) { v.m.Lock() defer v.m.Unlock() - // We must not update v.disk here; explicit calls to Set will supercede all + // We must not update v.disk here; explicit calls to Set will supersede all // future config reloads. v.live.Set(key, value) diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index e84ab7fbb21..0bb47361f55 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -187,7 +187,6 @@ func NewExecutor( // setting the vcursor config. e.initVConfig(warnOnShardedOnly, pv) - vschemaacl.Init() // we subscribe to update from the VSchemaManager e.vm = &VSchemaManager{ subscriber: e.SaveVSchema, diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index 62101639a11..f8ed0b558c3 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -401,9 +401,9 @@ func TestExecutorSetMetadata(t *testing.T) { }) t.Run("Session 2", func(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) @@ -411,11 +411,11 @@ func TestExecutorSetMetadata(t *testing.T) { set := "set @@vitess_metadata.app_keyspace_v1= '1'" _, err := executor.Execute(ctx, nil, "TestExecute", session, set, nil) - assert.NoError(t, err, "%s error: %v", set, err) + require.NoError(t, err, "%s error: %v", set, err) show := `show vitess_metadata variables like 'app\\_keyspace\\_v_'` result, err := executor.Execute(ctx, nil, "TestExecute", session, show, nil) - assert.NoError(t, err) + require.NoError(t, err) want := "1" got := result.Rows[0][1].ToString() @@ -424,11 +424,11 @@ func TestExecutorSetMetadata(t *testing.T) { // Update metadata set = "set @@vitess_metadata.app_keyspace_v2='2'" _, err = executor.Execute(ctx, nil, "TestExecute", session, set, nil) - assert.NoError(t, err, "%s error: %v", set, err) + require.NoError(t, err, "%s error: %v", set, err) show = `show vitess_metadata variables like 'app\\_keyspace\\_v%'` gotqr, err := executor.Execute(ctx, nil, "TestExecute", session, show, nil) - assert.NoError(t, err) + require.NoError(t, err) wantqr := &sqltypes.Result{ Fields: buildVarCharFields("Key", "Value"), diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go index 2b6d4710bce..d3ab28d6600 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -335,9 +335,9 @@ func TestExecutorTransactionsAutoCommitStreaming(t *testing.T) { } func TestExecutorDeleteMetadata(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) @@ -1318,9 +1318,9 @@ func TestExecutorDDLFk(t *testing.T) { } func TestExecutorAlterVSchemaKeyspace(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) @@ -1347,9 +1347,9 @@ func TestExecutorAlterVSchemaKeyspace(t *testing.T) { } func TestExecutorCreateVindexDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t) ks := "TestExecutor" @@ -1417,9 +1417,9 @@ func TestExecutorCreateVindexDDL(t *testing.T) { } func TestExecutorAddDropVschemaTableDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t) ks := KsTestUnsharded @@ -1486,8 +1486,7 @@ func TestExecutorVindexDDLACL(t *testing.T) { require.EqualError(t, err, `User 'blueUser' is not authorized to perform vschema operations`) // test when all users are enabled - vschemaacl.AuthorizedDDLUsers = "%" - vschemaacl.Init() + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) _, err = executor.Execute(ctxRedUser, nil, "TestExecute", session, stmt, nil) if err != nil { t.Errorf("unexpected error '%v'", err) @@ -1499,8 +1498,7 @@ func TestExecutorVindexDDLACL(t *testing.T) { } // test when only one user is enabled - vschemaacl.AuthorizedDDLUsers = "orangeUser, blueUser, greenUser" - vschemaacl.Init() + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("orangeUser, blueUser, greenUser")) _, err = executor.Execute(ctxRedUser, nil, "TestExecute", session, stmt, nil) require.EqualError(t, err, `User 'redUser' is not authorized to perform vschema operations`) @@ -1511,7 +1509,7 @@ func TestExecutorVindexDDLACL(t *testing.T) { } // restore the disallowed state - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) } func TestExecutorUnrecognized(t *testing.T) { diff --git a/go/vt/vtgate/executor_vschema_ddl_test.go b/go/vt/vtgate/executor_vschema_ddl_test.go index 825b65ab8f3..1acc1ba2362 100644 --- a/go/vt/vtgate/executor_vschema_ddl_test.go +++ b/go/vt/vtgate/executor_vschema_ddl_test.go @@ -135,9 +135,9 @@ func waitForColVindexes(t *testing.T, ks, table string, names []string, executor } func TestPlanExecutorAlterVSchemaKeyspace(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) session := econtext.NewSafeSession(&vtgatepb.Session{TargetString: "@primary", Autocommit: true}) @@ -163,9 +163,9 @@ func TestPlanExecutorAlterVSchemaKeyspace(t *testing.T) { } func TestPlanExecutorCreateVindexDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) ks := "TestExecutor" @@ -205,9 +205,9 @@ func TestPlanExecutorCreateVindexDDL(t *testing.T) { } func TestPlanExecutorDropVindexDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) ks := "TestExecutor" @@ -274,9 +274,9 @@ func TestPlanExecutorDropVindexDDL(t *testing.T) { } func TestPlanExecutorAddDropVschemaTableDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t) ks := KsTestUnsharded @@ -331,9 +331,9 @@ func TestPlanExecutorAddDropVschemaTableDDL(t *testing.T) { } func TestExecutorAddSequenceDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) ks := KsTestUnsharded @@ -391,9 +391,9 @@ func TestExecutorAddSequenceDDL(t *testing.T) { } func TestExecutorDropSequenceDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) ks := KsTestUnsharded @@ -442,9 +442,9 @@ func TestExecutorDropSequenceDDL(t *testing.T) { } func TestExecutorDropAutoIncDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, _, _, _, ctx := createExecutorEnv(t) ks := KsTestUnsharded @@ -484,9 +484,9 @@ func TestExecutorDropAutoIncDDL(t *testing.T) { } func TestExecutorAddDropVindexDDL(t *testing.T) { - vschemaacl.AuthorizedDDLUsers = "%" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) defer func() { - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) }() executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t) ks := "TestExecutor" @@ -747,8 +747,7 @@ func TestPlanExecutorVindexDDLACL(t *testing.T) { require.EqualError(t, err, `User 'blueUser' is not authorized to perform vschema operations`) // test when all users are enabled - vschemaacl.AuthorizedDDLUsers = "%" - vschemaacl.Init() + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("%")) _, err = executor.Execute(ctxRedUser, nil, "TestExecute", session, stmt, nil) if err != nil { t.Errorf("unexpected error '%v'", err) @@ -760,8 +759,7 @@ func TestPlanExecutorVindexDDLACL(t *testing.T) { } // test when only one user is enabled - vschemaacl.AuthorizedDDLUsers = "orangeUser, blueUser, greenUser" - vschemaacl.Init() + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("orangeUser, blueUser, greenUser")) _, err = executor.Execute(ctxRedUser, nil, "TestExecute", session, stmt, nil) require.EqualError(t, err, `User 'redUser' is not authorized to perform vschema operations`) @@ -772,5 +770,5 @@ func TestPlanExecutorVindexDDLACL(t *testing.T) { } // restore the disallowed state - vschemaacl.AuthorizedDDLUsers = "" + vschemaacl.AuthorizedDDLUsers.Set(vschemaacl.NewAuthorizedDDLUsers("")) } diff --git a/go/vt/vtgate/vschemaacl/vschemaacl.go b/go/vt/vtgate/vschemaacl/vschemaacl.go index 5345d1437fc..08f6c2b0cd4 100644 --- a/go/vt/vtgate/vschemaacl/vschemaacl.go +++ b/go/vt/vtgate/vschemaacl/vschemaacl.go @@ -18,26 +18,67 @@ package vschemaacl import ( "strings" - "sync" "github.com/spf13/pflag" + "github.com/spf13/viper" - "vitess.io/vitess/go/vt/servenv" - + "vitess.io/vitess/go/viperutil" querypb "vitess.io/vitess/go/vt/proto/query" + "vitess.io/vitess/go/vt/servenv" ) -var ( - // AuthorizedDDLUsers specifies the users that can perform ddl operations - AuthorizedDDLUsers string - - // ddlAllowAll is true if the special value of "*" was specified +type authorizedDDLUsers struct { allowAll bool + acl map[string]struct{} + source string +} + +func NewAuthorizedDDLUsers(users string) *authorizedDDLUsers { + acl := make(map[string]struct{}) + allowAll := false + + switch users { + case "": + case "%": + allowAll = true + default: + for _, user := range strings.Split(users, ",") { + user = strings.TrimSpace(user) + acl[user] = struct{}{} + } + } + + return &authorizedDDLUsers{ + allowAll: allowAll, + acl: acl, + source: users, + } +} - // ddlACL contains a set of allowed usernames - acl map[string]struct{} +func (a *authorizedDDLUsers) String() string { + return a.source +} - initMu sync.Mutex +var ( + // AuthorizedDDLUsers specifies the users that can perform ddl operations + AuthorizedDDLUsers = viperutil.Configure( + "vschema_ddl_authorized_users", + viperutil.Options[*authorizedDDLUsers]{ + FlagName: "vschema_ddl_authorized_users", + Default: &authorizedDDLUsers{}, + Dynamic: true, + GetFunc: func(v *viper.Viper) func(key string) *authorizedDDLUsers { + return func(key string) *authorizedDDLUsers { + newVal := v.GetString(key) + curVal, ok := v.Get(key).(*authorizedDDLUsers) + if ok && newVal == curVal.source { + return curVal + } + return NewAuthorizedDDLUsers(newVal) + } + }, + }, + ) ) // RegisterSchemaACLFlags installs log flags on the given FlagSet. @@ -46,7 +87,8 @@ var ( // calls this function, or call this function directly before parsing // command-line arguments. func RegisterSchemaACLFlags(fs *pflag.FlagSet) { - fs.StringVar(&AuthorizedDDLUsers, "vschema_ddl_authorized_users", AuthorizedDDLUsers, "List of users authorized to execute vschema ddl operations, or '%' to allow all users.") + fs.String("vschema_ddl_authorized_users", "", "List of users authorized to execute vschema ddl operations, or '%' to allow all users.") + viperutil.BindFlags(fs, AuthorizedDDLUsers) } func init() { @@ -55,33 +97,14 @@ func init() { } } -// Init parses the users option and sets allowAll / acl accordingly -func Init() { - initMu.Lock() - defer initMu.Unlock() - acl = make(map[string]struct{}) - allowAll = false - - if AuthorizedDDLUsers == "%" { - allowAll = true - return - } else if AuthorizedDDLUsers == "" { - return - } - - for _, user := range strings.Split(AuthorizedDDLUsers, ",") { - user = strings.TrimSpace(user) - acl[user] = struct{}{} - } -} - // Authorized returns true if the given caller is allowed to execute vschema operations func Authorized(caller *querypb.VTGateCallerID) bool { - if allowAll { + users := AuthorizedDDLUsers.Get() + if users.allowAll { return true } user := caller.GetUsername() - _, ok := acl[user] + _, ok := users.acl[user] return ok } diff --git a/go/vt/vtgate/vschemaacl/vschemaacl_test.go b/go/vt/vtgate/vschemaacl/vschemaacl_test.go index faa2dbfc294..cfd1de705af 100644 --- a/go/vt/vtgate/vschemaacl/vschemaacl_test.go +++ b/go/vt/vtgate/vschemaacl/vschemaacl_test.go @@ -35,8 +35,7 @@ func TestVschemaAcl(t *testing.T) { } // Test wildcard - AuthorizedDDLUsers = "%" - Init() + AuthorizedDDLUsers.Set(NewAuthorizedDDLUsers("%")) if !Authorized(&redUser) { t.Errorf("user should be authorized") @@ -46,8 +45,7 @@ func TestVschemaAcl(t *testing.T) { } // Test user list - AuthorizedDDLUsers = "oneUser, twoUser, redUser, blueUser" - Init() + AuthorizedDDLUsers.Set(NewAuthorizedDDLUsers("oneUser, twoUser, redUser, blueUser")) if !Authorized(&redUser) { t.Errorf("user should be authorized") @@ -57,8 +55,7 @@ func TestVschemaAcl(t *testing.T) { } // Revert to baseline state for other tests - AuthorizedDDLUsers = "" - Init() + AuthorizedDDLUsers.Set(NewAuthorizedDDLUsers("")) // By default no users are allowed in if Authorized(&redUser) {