From 5660b4e58af7de7695b3b6c59b91568ad85259dc Mon Sep 17 00:00:00 2001 From: Scott Lanning Date: Wed, 29 Aug 2018 18:22:43 +0200 Subject: [PATCH] add a SIGHUP handler for reloading vttablet's table ACL file Implements the issue https://github.com/vitessio/vitess/issues/4160 The change is mainly factoring out what's now in initACL and adding also a the signal handler that calls that again. I moved it to vttabletserver, though, since that seemed like the best place for testing, though I'm not sure about that. Signed-off-by: Scott Lanning --- go/cmd/vttablet/vttablet.go | 16 +--- go/vt/vttablet/tabletserver/tabletserver.go | 36 ++++++++- .../tabletserver/tabletserver_test.go | 78 +++++++++++++++++++ 3 files changed, 115 insertions(+), 15 deletions(-) diff --git a/go/cmd/vttablet/vttablet.go b/go/cmd/vttablet/vttablet.go index f4ebedd9f5b..dcc9768daa1 100644 --- a/go/cmd/vttablet/vttablet.go +++ b/go/cmd/vttablet/vttablet.go @@ -36,7 +36,7 @@ import ( var ( enforceTableACLConfig = flag.Bool("enforce-tableacl-config", false, "if this flag is true, vttablet will fail to start if a valid tableacl config does not exist") - tableACLConfig = flag.String("table-acl-config", "", "path to table access checker config file") + tableACLConfig = flag.String("table-acl-config", "", "path to table access checker config file; send SIGHUP to reload this file") tabletPath = flag.String("tablet-path", "", "tablet alias") agent *tabletmanager.ActionAgent @@ -115,19 +115,7 @@ func main() { qsc.StopService() }) - // tabletacl.Init loads ACL from file if *tableACLConfig is not empty - err = tableacl.Init( - *tableACLConfig, - func() { - qsc.ClearQueryPlanCache() - }, - ) - if err != nil { - log.Errorf("Fail to initialize Table ACL: %v", err) - if *enforceTableACLConfig { - log.Exit("Need a valid initial Table ACL when enforce-tableacl-config is set, exiting.") - } - } + qsc.InitACL(*tableACLConfig, *enforceTableACLConfig) // Create mysqld and register the health reporter (needs to be done // before initializing the agent, so the initial health check diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index 1441cb0f981..2ae3ca6ac82 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -21,9 +21,12 @@ import ( "fmt" "io" "net/http" + "os" + "os/signal" "sort" "strings" "sync" + "syscall" "time" "golang.org/x/net/context" @@ -43,6 +46,7 @@ import ( "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/tableacl" "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vttablet/heartbeat" @@ -326,7 +330,7 @@ func (tsv *TabletServer) IsServing() bool { return tsv.GetState() == "SERVING" } -// InitDBConfig inititalizes the db config variables for TabletServer. You must call this function before +// InitDBConfig initializes the db config variables for TabletServer. You must call this function before // calling SetServingType. func (tsv *TabletServer) InitDBConfig(target querypb.Target, dbcfgs *dbconfigs.DBConfigs) error { tsv.mu.Lock() @@ -347,6 +351,36 @@ func (tsv *TabletServer) InitDBConfig(target querypb.Target, dbcfgs *dbconfigs.D return nil } +func (tsv *TabletServer) initACL(tableACLConfigFile string, enforceTableACLConfig bool) { + // tabletacl.Init loads ACL from file if *tableACLConfig is not empty + err := tableacl.Init( + tableACLConfigFile, + func() { + tsv.ClearQueryPlanCache() + }, + ) + if err != nil { + log.Errorf("Fail to initialize Table ACL: %v", err) + if enforceTableACLConfig { + log.Exit("Need a valid initial Table ACL when enforce-tableacl-config is set, exiting.") + } + } +} + +// InitACL loads the table ACL and sets up a SIGHUP handler for reloading it. +func (tsv *TabletServer) InitACL(tableACLConfigFile string, enforceTableACLConfig bool) { + tsv.initACL(tableACLConfigFile, enforceTableACLConfig) + + sigChan := make(chan os.Signal, 1) + signal.Notify(sigChan, syscall.SIGHUP) + go func() { + for { + <-sigChan + tsv.initACL(tableACLConfigFile, enforceTableACLConfig) + } + }() +} + // StartService is a convenience function for InitDBConfig->SetServingType // with serving=true. func (tsv *TabletServer) StartService(target querypb.Target, dbcfgs *dbconfigs.DBConfigs) (err error) { diff --git a/go/vt/vttablet/tabletserver/tabletserver_test.go b/go/vt/vttablet/tabletserver/tabletserver_test.go index 4e8085482ef..fa89040de35 100644 --- a/go/vt/vttablet/tabletserver/tabletserver_test.go +++ b/go/vt/vttablet/tabletserver/tabletserver_test.go @@ -20,11 +20,14 @@ import ( "expvar" "fmt" "io" + "io/ioutil" "math/rand" + "os" "reflect" "strconv" "strings" "sync" + "syscall" "testing" "time" @@ -36,6 +39,8 @@ import ( "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/tableacl" + "vitess.io/vitess/go/vt/tableacl/simpleacl" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" @@ -2719,6 +2724,79 @@ func TestTerseErrorsIgnoreFailoverInProgress(t *testing.T) { } } +var aclJSON1 = `{ + "table_groups": [ + { + "name": "group01", + "table_names_or_prefixes": ["test_table1"], + "readers": ["vt1"], + "writers": ["vt1"] + } + ] +}` +var aclJSON2 = `{ + "table_groups": [ + { + "name": "group02", + "table_names_or_prefixes": ["test_table2"], + "readers": ["vt2"], + "admins": ["vt2"] + } + ] +}` + +func TestACLHUP(t *testing.T) { + tableacl.Register("simpleacl", &simpleacl.Factory{}) + testUtils := newTestUtils() + config := testUtils.newQueryServiceConfig() + tsv := NewTabletServerWithNilTopoServer(config) + + f, err := ioutil.TempFile("", "tableacl") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + + if _, err := io.WriteString(f, aclJSON1); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + + tsv.InitACL(f.Name(), true) + + groups1 := tableacl.GetCurrentConfig().TableGroups + if name1 := groups1[0].GetName(); name1 != "group01" { + t.Fatalf("Expected name 'group01', got '%s'", name1) + } + + if f, err = os.Create(f.Name()); err != nil { + t.Fatal(err) + } + if _, err = io.WriteString(f, aclJSON2); err != nil { + t.Fatal(err) + } + + syscall.Kill(syscall.Getpid(), syscall.SIGHUP) + time.Sleep(25 * time.Millisecond) // wait for signal handler + + groups2 := tableacl.GetCurrentConfig().TableGroups + if len(groups2) != 1 { + t.Fatalf("Expected only one table group") + } + group2 := groups2[0] + if name2 := group2.GetName(); name2 != "group02" { + t.Fatalf("Expected name 'group02', got '%s'", name2) + } + if group2.GetAdmins() == nil { + t.Fatalf("Expected 'admins' to exist, but it didn't") + } + if group2.GetWriters() != nil { + t.Fatalf("Expected 'writers' to not exist, got '%s'", group2.GetWriters()) + } +} + func TestConfigChanges(t *testing.T) { db := setUpTabletServerTest(t) defer db.Close()