Migrate ACL package to pflag (#11392)

* Move acl package to pflag

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Stragglers

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Gofmt after GH merge conflict resolution

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Merge in new vtbackup work

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Address linter errors:
context-as-argument: context.Context should be the first parameter of a function (revive)

Signed-off-by: Matt Lord <mattalord@gmail.com>

Signed-off-by: Matt Lord <mattalord@gmail.com>

go/acl/acl.go

@@ -36,11 +36,12 @@ limitations under the License.
 package acl
 
 import (
-	"flag"
 	"fmt"
 	"net/http"
 	"sync"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/vt/log"
 )
 
@@ -54,7 +55,7 @@ const (
 )
 
 var (
-	securityPolicy = flag.String("security_policy", "", "the name of a registered security policy to use for controlling access to URLs - empty means allow all for anyone (built-in policies: deny-all, read-only)")
+	securityPolicy string
 	policies       = make(map[string]Policy)
 	once           sync.Once
 	currentPolicy  Policy
@@ -71,6 +72,10 @@ type Policy interface {
 	CheckAccessHTTP(req *http.Request, role string) error
 }
 
+func RegisterFlags(fs *pflag.FlagSet) {
+	fs.StringVar(&securityPolicy, "security_policy", securityPolicy, "the name of a registered security policy to use for controlling access to URLs - empty means allow all for anyone (built-in policies: deny-all, read-only)")
+}
+
 // RegisterPolicy registers a security policy. This function must be called
 // before the first call to CheckAccess happens, preferably through an init.
 // This will ensure that the requested policy can be found by other acl
@@ -83,16 +88,16 @@ func RegisterPolicy(name string, policy Policy) {
 }
 
 func savePolicy() {
-	if *securityPolicy == "" {
+	if securityPolicy == "" {
 		// Setting the policy to nil means Allow All from Anyone.
 		currentPolicy = nil
 		return
 	}
-	if policy, ok := policies[*securityPolicy]; ok {
+	if policy, ok := policies[securityPolicy]; ok {
 		currentPolicy = policy
 		return
 	}
-	log.Warningf("security_policy %q not found; using fallback policy (deny-all)", *securityPolicy)
+	log.Warningf("security_policy %q not found; using fallback policy (deny-all)", securityPolicy)
 	currentPolicy = denyAllPolicy{}
 }
 

go/cmd/mysqlctl/mysqlctl.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/cmd"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/flagutil"
@@ -55,6 +56,8 @@ func init() {
 		fs.IntVar(&mysqlPort, "mysql_port", mysqlPort, "MySQL port")
 		fs.UintVar(&tabletUID, "tablet_uid", tabletUID, "Tablet UID")
 		fs.StringVar(&mysqlSocket, "mysql_socket", mysqlSocket, "Path to the mysqld socket file")
+
+		acl.RegisterFlags(fs)
 	})
 }
 

go/cmd/mysqlctld/mysqlctld.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/log"
@@ -63,6 +64,8 @@ func init() {
 		fs.StringVar(&mysqlSocket, "mysql_socket", mysqlSocket, "Path to the mysqld socket file")
 		fs.DurationVar(&waitTime, "wait_time", waitTime, "How long to wait for mysqld startup or shutdown")
 		fs.StringVar(&initDBSQLFile, "init_db_sql_file", initDBSQLFile, "Path to .sql file to run after mysqld initialization")
+
+		acl.RegisterFlags(fs)
 	})
 }
 

go/cmd/query_analyzer/query_analyzer.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -66,6 +67,7 @@ func main() {
 	fs := pflag.NewFlagSet("query_analyzer", pflag.ExitOnError)
 	log.RegisterFlags(fs)
 	logutil.RegisterFlags(fs)
+	acl.RegisterFlags(fs)
 	servenv.RegisterMySQLServerFlags(fs)
 	_flag.Parse(fs)
 	for _, filename := range _flag.Args() {

go/cmd/rulesctl/main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"log"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/cmd/rulesctl/cmd"
 	vtlog "vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -13,6 +14,7 @@ func main() {
 	rootCmd := cmd.Main()
 	vtlog.RegisterFlags(rootCmd.PersistentFlags())
 	logutil.RegisterFlags(rootCmd.PersistentFlags())
+	acl.RegisterFlags(rootCmd.PersistentFlags())
 	servenv.RegisterMySQLServerFlags(rootCmd.PersistentFlags())
 	if err := rootCmd.Execute(); err != nil {
 		log.Printf("%v", err)

go/cmd/topo2topo/topo2topo.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/grpccommon"
 	"vitess.io/vitess/go/vt/log"
@@ -61,6 +62,8 @@ func init() {
 		fs.BoolVar(&doShardReplications, "do-shard-replications", doShardReplications, "copies the shard replication information")
 		fs.BoolVar(&doTablets, "do-tablets", doTablets, "copies the tablet information")
 		fs.BoolVar(&doRoutingRules, "do-routing-rules", doRoutingRules, "copies the routing rules")
+
+		acl.RegisterFlags(fs)
 	})
 }
 

go/cmd/vtaclcheck/vtaclcheck.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -36,6 +37,8 @@ func init() {
 		fs.StringVar(&aclFile, "acl-file", aclFile, "The path of the JSON ACL file to check")
 		fs.StringVar(&staticAuthFile, "static-auth-file", staticAuthFile, "The path of the auth_server_static JSON file to check")
 
+		acl.RegisterFlags(fs)
+
 		fs.SetOutput(logutil.NewLoggerWriter(logger))
 	})
 }

go/cmd/vtbackup/vtbackup.go

@@ -71,6 +71,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/cmd"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/mysql"
@@ -137,6 +138,8 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.DurationVar(&mysqlTimeout, "mysql_timeout", mysqlTimeout, "how long to wait for mysqld startup")
 	fs.StringVar(&initDBSQLFile, "init_db_sql_file", initDBSQLFile, "path to .sql file to run after mysql_install_db")
 	fs.BoolVar(&detachedMode, "detach", detachedMode, "detached mode - run backups detached from the terminal")
+
+	acl.RegisterFlags(fs)
 }
 
 func init() {

go/cmd/vtbench/vtbench.go

@@ -24,10 +24,10 @@ import (
 
 	"github.com/spf13/pflag"
 
-	"vitess.io/vitess/go/vt/grpccommon"
-
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/dbconfigs"
+	"vitess.io/vitess/go/vt/grpccommon"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
 	"vitess.io/vitess/go/vt/servenv"
@@ -107,6 +107,7 @@ func initFlags(fs *pflag.FlagSet) {
 	grpccommon.RegisterFlags(fs)
 	log.RegisterFlags(fs)
 	logutil.RegisterFlags(fs)
+	acl.RegisterFlags(fs)
 	servenv.RegisterMySQLServerFlags(fs)
 }
 

go/cmd/vtclient/vtclient.go

@@ -33,6 +33,7 @@ import (
 	"github.com/olekukonko/tablewriter"
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/vt/concurrency"
 	"vitess.io/vitess/go/vt/grpccommon"
 	"vitess.io/vitess/go/vt/log"
@@ -102,6 +103,8 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&useRandom, "use_random_sequence", useRandom, "use random sequence for generating [min_sequence_id, max_sequence_id)")
 	fs.IntVar(&qps, "qps", qps, "queries per second to throttle each thread at.")
 
+	acl.RegisterFlags(fs)
+
 	bindVariables = newBindvars(fs, "bind_variables", "bind variables as a json list")
 }
 

go/cmd/vtcombo/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/spf13/pflag"
 	"google.golang.org/protobuf/proto"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/mysql"
 	"vitess.io/vitess/go/vt/dbconfigs"
@@ -132,6 +133,8 @@ func main() {
 		fs.AddFlagSet(flags)
 		// Save for later -- see comment directly after ParseFlags for why.
 		globalFlags = fs
+
+		acl.RegisterFlags(fs)
 	})
 
 	servenv.ParseFlags("vtcombo")

go/cmd/vtctl/vtctl.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/cmd"
 	"vitess.io/vitess/go/cmd/vtctldclient/command"
 	"vitess.io/vitess/go/exit"
@@ -65,6 +66,8 @@ func init() {
 
 		fs.DurationVar(&waitTime, "wait-time", waitTime, "time to wait on an action")
 		fs.BoolVar(&detachedMode, "detach", detachedMode, "detached mode - run vtcl detached from the terminal")
+
+		acl.RegisterFlags(fs)
 	})
 }
 

go/cmd/vtctlclient/main.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/trace"
 	"vitess.io/vitess/go/vt/log"
@@ -48,6 +49,8 @@ func init() {
 	servenv.OnParse(func(fs *pflag.FlagSet) {
 		fs.DurationVar(&actionTimeout, "action_timeout", actionTimeout, "timeout for the total command")
 		fs.StringVar(&server, "server", server, "server to use for connection")
+
+		acl.RegisterFlags(fs)
 	})
 }
 

go/cmd/vtctld/main.go

@@ -17,6 +17,9 @@ limitations under the License.
 package main
 
 import (
+	"github.com/spf13/pflag"
+
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/topo"
@@ -29,6 +32,10 @@ func init() {
 	servenv.RegisterGRPCServerFlags()
 	servenv.RegisterGRPCServerAuthFlags()
 	servenv.RegisterServiceMapFlag()
+
+	servenv.OnParse(func(fs *pflag.FlagSet) {
+		acl.RegisterFlags(fs)
+	})
 }
 
 // used at runtime by plug-ins

go/cmd/vtctldclient/main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"flag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/cmd/vtctldclient/command"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/grpcclient"
@@ -45,6 +46,7 @@ func main() {
 	grpcclientcommon.RegisterFlags(command.Root.PersistentFlags())
 	servenv.RegisterMySQLServerFlags(command.Root.PersistentFlags())
 	vtctlclient.RegisterFlags(command.Root.PersistentFlags())
+	acl.RegisterFlags(command.Root.PersistentFlags())
 
 	// hack to get rid of an "ERROR: logging before flag.Parse"
 	_flag.TrickGlog()

go/cmd/vtexplain/vtexplain.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -67,6 +68,8 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.IntVar(&numShards, "shards", numShards, "Number of shards per keyspace. Passing --ks-shard-map/--ks-shard-map-file causes this flag to be ignored.")
 	fs.StringVar(&executionMode, "execution-mode", executionMode, "The execution mode to simulate -- must be set to multi, legacy-autocommit, or twopc")
 	fs.StringVar(&outputMode, "output-mode", outputMode, "Output in human-friendly text or json")
+
+	acl.RegisterFlags(fs)
 }
 
 func init() {

go/cmd/vtgate/vtgate.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/discovery"
 	"vitess.io/vitess/go/vt/log"
@@ -48,6 +49,8 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&cell, "cell", cell, "cell to use")
 	fs.StringVar(&tabletTypesToWait, "tablet_types_to_wait", tabletTypesToWait, "wait till connected for specified tablet types during Gateway initialization")
 	fs.StringVar(&plannerName, "planner-version", plannerName, "Sets the default planner to use when the session has not changed it. Valid values are: V3, Gen4, Gen4Greedy and Gen4Fallback. Gen4Fallback tries the gen4 planner and falls back to the V3 planner if the gen4 fails.")
+
+	acl.RegisterFlags(fs)
 }
 
 var resilientServer *srvtopo.ResilientServer

go/cmd/vtgateclienttest/main.go

@@ -20,6 +20,9 @@ limitations under the License.
 package main
 
 import (
+	"github.com/spf13/pflag"
+
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/cmd/vtgateclienttest/services"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/servenv"
@@ -32,6 +35,10 @@ func init() {
 	servenv.RegisterGRPCServerFlags()
 	servenv.RegisterGRPCServerAuthFlags()
 	servenv.RegisterServiceMapFlag()
+
+	servenv.OnParse(func(fs *pflag.FlagSet) {
+		acl.RegisterFlags(fs)
+	})
 }
 
 func main() {

go/cmd/vtgr/main.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/vtgr"
 )
@@ -26,8 +27,9 @@ func main() {
 	var clustersToWatch []string
 	servenv.OnParseFor("vtgr", func(fs *pflag.FlagSet) {
 		fs.StringSliceVar(&clustersToWatch, "clusters_to_watch", nil, `Comma-separated list of keyspaces or keyspace/shards that this instance will monitor and repair. Defaults to all clusters in the topology. Example: "ks1,ks2/-80"`)
-	})
 
+		acl.RegisterFlags(fs)
+	})
 	servenv.ParseFlags("vtgr")
 
 	// openTabletDiscovery will open up a connection to topo server

go/cmd/vtorc/main.go

@@ -25,6 +25,7 @@ import (
 	_ "github.com/mattn/go-sqlite3"
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/vt/grpccommon"
 	"vitess.io/vitess/go/vt/log"
 	vtlog "vitess.io/vitess/go/vt/log"
@@ -108,6 +109,7 @@ func main() {
 	logutil.RegisterFlags(fs)
 	servenv.RegisterDefaultFlags()
 	servenv.RegisterFlags()
+	acl.RegisterFlags(fs)
 	servenv.OnParseFor("vtorc", func(flags *pflag.FlagSet) { flags.AddFlagSet(fs) })
 
 	args := append([]string{}, os.Args...)

go/cmd/vttablet/vttablet.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/vt/binlog"
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/log"
@@ -63,6 +64,7 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&tabletPath, "tablet-path", tabletPath, "tablet alias")
 	fs.StringVar(&tabletConfig, "tablet_config", tabletConfig, "YAML file config for tablet")
 
+	acl.RegisterFlags(fs)
 }
 
 func init() {

go/cmd/vttestserver/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/spf13/pflag"
 	"google.golang.org/protobuf/encoding/prototext"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/vttest"
@@ -163,6 +164,8 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&config.ExternalTopoImplementation, "external_topo_implementation", "", "the topology implementation to use for vtcombo process")
 	fs.StringVar(&config.ExternalTopoGlobalServerAddress, "external_topo_global_server_address", "", "the address of the global topology server for vtcombo process")
 	fs.StringVar(&config.ExternalTopoGlobalRoot, "external_topo_global_root", "", "the path of the global topology data in the global topology server for vtcombo process")
+
+	acl.RegisterFlags(fs)
 }
 
 func init() {

go/cmd/zk/zkcmd.go

@@ -37,6 +37,7 @@ import (
 	"github.com/z-division/go-zookeeper/zk"
 	"golang.org/x/term"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -144,6 +145,7 @@ func main() {
 	fs := pflag.NewFlagSet("zkcmd", pflag.ExitOnError)
 	log.RegisterFlags(fs)
 	logutil.RegisterFlags(fs)
+	acl.RegisterFlags(fs)
 	_flag.SetUsage(flag.CommandLine, _flag.UsageOptions{ // TODO: hmmm
 		Epilogue: func(w io.Writer) { fmt.Fprint(w, doc) },
 	})

go/cmd/zkctld/zkctld.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/spf13/pflag"
 
+	"vitess.io/vitess/go/acl"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -47,6 +48,8 @@ func registerFlags(fs *pflag.FlagSet) {
 		"zkid@server1:leaderPort1:electionPort1:clientPort1,...)")
 	fs.UintVar(&myID, "zk.myid", myID,
 		"which server do you want to be? only needed when running multiple instance on one box, otherwise myid is implied by hostname")
+
+	acl.RegisterFlags(fs)
 }
 
 func main() {

go/test/endtoend/encryption/encryptedtransport/encrypted_transport_test.go

@@ -193,13 +193,13 @@ func TestSecureTransport(t *testing.T) {
 	assert.Contains(t, err.Error(), "Select command denied to user")
 	assert.Contains(t, err.Error(), "for table 'vt_insert_test' (ACL check error)")
 
-	useEffectiveCallerID(t, ctx)
-	useEffectiveGroups(t, ctx)
+	useEffectiveCallerID(ctx, t)
+	useEffectiveGroups(ctx, t)
 
 	clusterInstance.Teardown()
 }
 
-func useEffectiveCallerID(t *testing.T, ctx context.Context) {
+func useEffectiveCallerID(ctx context.Context, t *testing.T) {
 	// now restart vtgate in the mode where we don't use SSL
 	// for client connections, but we copy effective caller id
 	// into immediate caller id.
@@ -248,7 +248,7 @@ func useEffectiveCallerID(t *testing.T, ctx context.Context) {
 	assert.Contains(t, err.Error(), "for table 'vt_insert_test' (ACL check error)")
 }
 
-func useEffectiveGroups(t *testing.T, ctx context.Context) {
+func useEffectiveGroups(ctx context.Context, t *testing.T) {
 	// now restart vtgate in the mode where we don't use SSL
 	// for client connections, but we copy effective caller's groups
 	// into immediate caller id.