From c636acffe61d69106fe8bcc939d273a6cd7c716f Mon Sep 17 00:00:00 2001 From: garfthoffman <109185460+garfthoffman@users.noreply.github.com> Date: Fri, 22 Nov 2024 18:30:29 +0000 Subject: [PATCH 1/4] fail initializing acl if the provided acl file is empty Signed-off-by: garfthoffman <109185460+garfthoffman@users.noreply.github.com> --- go/vt/vttablet/tabletserver/tabletserver.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index 847de25eb02..7d00c85704c 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -367,7 +367,8 @@ func (tsv *TabletServer) initACL(tableACLConfigFile string, enforceTableACLConfi tsv.ClearQueryPlanCache() }, ) - if err != nil { + // Log failure if either there was a problem loading the ACL, or if the ACL is empty + if err != nil || tableacl.GetCurrentConfig().String() == "" { 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.") From eca5d61c0e2157c18d969df6799be1d00396451c Mon Sep 17 00:00:00 2001 From: garfthoffman <109185460+garfthoffman@users.noreply.github.com> Date: Fri, 22 Nov 2024 19:05:58 +0000 Subject: [PATCH 2/4] validate instead that file data is not empty Signed-off-by: garfthoffman <109185460+garfthoffman@users.noreply.github.com> --- go/vt/tableacl/tableacl.go | 10 +++++++--- go/vt/vttablet/tabletserver/tabletserver.go | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/go/vt/tableacl/tableacl.go b/go/vt/tableacl/tableacl.go index 1b236cb1812..d68a0642ef0 100644 --- a/go/vt/tableacl/tableacl.go +++ b/go/vt/tableacl/tableacl.go @@ -96,11 +96,11 @@ var currentTableACL tableACL // } // ] // } -func Init(configFile string, aclCB func()) error { - return currentTableACL.init(configFile, aclCB) +func Init(configFile string, aclCB func(), enforceTableACLConfig string) error { + return currentTableACL.init(configFile, aclCB, enforceTableACLConfig string) } -func (tacl *tableACL) init(configFile string, aclCB func()) error { +func (tacl *tableACL) init(configFile string, aclCB func(), enforceTableACLConfig string) error { tacl.SetCallback(aclCB) if configFile == "" { return nil @@ -110,6 +110,10 @@ func (tacl *tableACL) init(configFile string, aclCB func()) error { log.Infof("unable to read tableACL config file: %v Error: %v", configFile, err) return err } + if len(data) == 0 { + return error.New("tableACL config file is empty") + } + config := &tableaclpb.Config{} if err := config.UnmarshalVT(data); err != nil { // try to parse tableacl as json file diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index 7d00c85704c..8fb167efc80 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -368,7 +368,7 @@ func (tsv *TabletServer) initACL(tableACLConfigFile string, enforceTableACLConfi }, ) // Log failure if either there was a problem loading the ACL, or if the ACL is empty - if err != nil || tableacl.GetCurrentConfig().String() == "" { + 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.") From d79d1c813156e02aebf9468a20f72e65d9424551 Mon Sep 17 00:00:00 2001 From: garfthoffman <109185460+garfthoffman@users.noreply.github.com> Date: Fri, 22 Nov 2024 19:06:56 +0000 Subject: [PATCH 3/4] clean up unused boolean flag Signed-off-by: garfthoffman <109185460+garfthoffman@users.noreply.github.com> --- go/vt/tableacl/tableacl.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/tableacl/tableacl.go b/go/vt/tableacl/tableacl.go index d68a0642ef0..dd6638e9786 100644 --- a/go/vt/tableacl/tableacl.go +++ b/go/vt/tableacl/tableacl.go @@ -96,11 +96,11 @@ var currentTableACL tableACL // } // ] // } -func Init(configFile string, aclCB func(), enforceTableACLConfig string) error { - return currentTableACL.init(configFile, aclCB, enforceTableACLConfig string) +func Init(configFile string, aclCB func()) error { + return currentTableACL.init(configFile, aclCB) } -func (tacl *tableACL) init(configFile string, aclCB func(), enforceTableACLConfig string) error { +func (tacl *tableACL) init(configFile string, aclCB func()) error { tacl.SetCallback(aclCB) if configFile == "" { return nil From d47a37b197d37fda6ceb62f21194ff65e719cf66 Mon Sep 17 00:00:00 2001 From: garfthoffman <109185460+garfthoffman@users.noreply.github.com> Date: Fri, 22 Nov 2024 19:12:11 +0000 Subject: [PATCH 4/4] add unit test, correct errors package Signed-off-by: garfthoffman <109185460+garfthoffman@users.noreply.github.com> --- go/vt/tableacl/tableacl.go | 2 +- go/vt/tableacl/tableacl_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/go/vt/tableacl/tableacl.go b/go/vt/tableacl/tableacl.go index dd6638e9786..2bb48ca28f2 100644 --- a/go/vt/tableacl/tableacl.go +++ b/go/vt/tableacl/tableacl.go @@ -111,7 +111,7 @@ func (tacl *tableACL) init(configFile string, aclCB func()) error { return err } if len(data) == 0 { - return error.New("tableACL config file is empty") + return errors.New("tableACL config file is empty") } config := &tableaclpb.Config{} diff --git a/go/vt/tableacl/tableacl_test.go b/go/vt/tableacl/tableacl_test.go index 388567b62e2..1f5e89a6a48 100644 --- a/go/vt/tableacl/tableacl_test.go +++ b/go/vt/tableacl/tableacl_test.go @@ -74,6 +74,21 @@ func TestInitWithValidConfig(t *testing.T) { } } +func TestInitWithEmptyConfig(t *testing.T) { + tacl := tableACL{factory: &simpleacl.Factory{}} + f, err := os.CreateTemp("", "tableacl") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + if err := f.Close(); err != nil { + t.Fatal(err) + } + if err := tacl.init(f.Name(), func() {}); err == nil { + t.Fatal("tableACL config file is empty") + } +} + func TestInitFromProto(t *testing.T) { tacl := tableACL{factory: &simpleacl.Factory{}} readerACL := tacl.Authorized("my_test_table", READER)