From 83c8a6444e9e1305922550bd5b6ac373babb0ffc Mon Sep 17 00:00:00 2001 From: Eno Compton Date: Tue, 11 Apr 2023 08:52:57 -0600 Subject: [PATCH] feat: add support for Auto IP (#1735) This commit restores a legacy behavior of the v1 Proxy, this time behind the flag --auto-ip. When the flag is enabled, the Proxy will first attempt to use a public IP. If no public IP is present, the Proxy will second attempt to use a private IP. If neither is present, the Proxy errors as expected. This new flag is primarily to support legacy behavior and should not be used in production otherwise. Co-authored-by: Jack Wotherspoon --- cmd/root.go | 28 +++++++++++++++++++--------- cmd/root_test.go | 29 +++++++++++++++++++++++++++++ internal/proxy/proxy.go | 17 ++++++++++++++--- 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index d025c7895..dfeaac0dd 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -427,6 +427,10 @@ https://cloud.google.com/storage/docs/requester-pays`) `Comma separated list of service accounts to impersonate. Last value is the target account.`) cmd.PersistentFlags().BoolVar(&c.conf.Quiet, "quiet", false, "Log error messages only") + pflags.BoolVar(&c.conf.AutoIP, "auto-ip", false, + `Supports legacy behavior of v1 and will try to connect to first IP +address returned by the SQL Admin API. In most cases, this flag should not be used. +Prefer default of public IP or use --private-ip instead.`) // Global and per instance flags pflags.StringVarP(&c.conf.Addr, "address", "a", "127.0.0.1", @@ -452,7 +456,7 @@ is the target account.`) // object as a single source of truth. if !f.Changed && v.IsSet(f.Name) { val := v.Get(f.Name) - pflags.Set(f.Name, fmt.Sprintf("%v", val)) + _ = pflags.Set(f.Name, fmt.Sprintf("%v", val)) } }) @@ -490,6 +494,9 @@ func parseConfig(cmd *Command, conf *proxy.Config, args []string) error { if ip := net.ParseIP(conf.Addr); ip == nil { return newBadCommandError(fmt.Sprintf("not a valid IP address: %q", conf.Addr)) } + if userHasSet("private-ip") && userHasSet("auto-ip") { + return newBadCommandError("cannot specify --private-ip and --auto-ip together") + } // If more than one auth method is set, error. if conf.Token != "" && conf.CredentialsFile != "" { @@ -639,6 +646,9 @@ func parseConfig(cmd *Command, conf *proxy.Config, args []string) error { if err != nil { return err } + if ic.PrivateIP != nil && *ic.PrivateIP && conf.AutoIP { + return newBadCommandError("cannot use --auto-ip with private-ip") + } } ics = append(ics, ic) @@ -653,16 +663,16 @@ func parseConfig(cmd *Command, conf *proxy.Config, args []string) error { // true if the value is "t" or "true" case-insensitive // false if the value is "f" or "false" case-insensitive func parseBoolOpt(q url.Values, name string) (*bool, error) { - iam, ok := q[name] + v, ok := q[name] if !ok { return nil, nil } - if len(iam) != 1 { - return nil, newBadCommandError(fmt.Sprintf("%v param should be only one value: %q", name, iam)) + if len(v) != 1 { + return nil, newBadCommandError(fmt.Sprintf("%v param should be only one value: %q", name, v)) } - switch strings.ToLower(iam[0]) { + switch strings.ToLower(v[0]) { case "true", "t", "": enable := true return &enable, nil @@ -673,7 +683,7 @@ func parseBoolOpt(q url.Values, name string) (*bool, error) { // value is not recognized return nil, newBadCommandError( fmt.Sprintf("%v query param should be true or false, got: %q", - name, iam[0], + name, v[0], )) } @@ -681,7 +691,7 @@ func parseBoolOpt(q url.Values, name string) (*bool, error) { // runSignalWrapper watches for SIGTERM and SIGINT and interupts execution if necessary. func runSignalWrapper(cmd *Command) (err error) { - defer cmd.cleanup() + defer func() { _ = cmd.cleanup() }() ctx, cancel := context.WithCancel(cmd.Context()) defer cancel() @@ -845,7 +855,7 @@ func runSignalWrapper(cmd *Command) (err error) { } func quitquitquit(quitOnce *sync.Once, shutdownCh chan<- error) http.HandlerFunc { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + return func(rw http.ResponseWriter, req *http.Request) { if req.Method != http.MethodPost { rw.WriteHeader(400) return @@ -858,7 +868,7 @@ func quitquitquit(quitOnce *sync.Once, shutdownCh chan<- error) http.HandlerFunc // the proxy is already exiting. } }) - }) + } } func startHTTPServer(ctx context.Context, l cloudsql.Logger, addr string, mux *http.ServeMux, shutdownCh chan<- error) { diff --git a/cmd/root_test.go b/cmd/root_test.go index 7b75c6442..a610a7019 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -405,6 +405,13 @@ func TestNewCommandArguments(t *testing.T) { LoginToken: "MYLOGINTOKEN", }), }, + { + desc: "using the auto-ip flag", + args: []string{"--auto-ip", "proj:region:inst"}, + want: withDefaults(&proxy.Config{ + AutoIP: true, + }), + }, } for _, tc := range tcs { @@ -744,6 +751,14 @@ func TestNewCommandWithEnvironmentConfig(t *testing.T) { QuitQuitQuit: true, }), }, + { + desc: "using the auto-ip envvar", + envName: "CSQL_PROXY_AUTO_IP", + envValue: "true", + want: withDefaults(&proxy.Config{ + AutoIP: true, + }), + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { @@ -1022,6 +1037,20 @@ func TestNewCommandWithErrors(t *testing.T) { "--token", "MYTOKEN", "--login-token", "MYLOGINTOKEN", "p:r:i"}, }, + { + desc: "using --private-ip with --auto-ip", + args: []string{ + "--private-ip", "--auto-ip", + "p:r:i", + }, + }, + { + desc: "using private-ip query param with --auto-ip", + args: []string{ + "--auto-ip", + "p:r:i?private-ip=true", + }, + }, } for _, tc := range tcs { diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index f6ee9b9db..6978e23db 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -168,6 +168,12 @@ type Config struct { // for all instances. PrivateIP bool + // AutoIP supports a legacy behavior where the Proxy will connect to + // the first IP address returned from the SQL ADmin API response. This + // setting should be avoided and used only to support legacy Proxy + // users. + AutoIP bool + // Instances are configuration for individual instances. Instance // configuration takes precedence over global configuration. Instances []InstanceConnConfig @@ -238,10 +244,15 @@ func dialOptions(c Config, i InstanceConnConfig) []cloudsqlconn.DialOption { opts = append(opts, cloudsqlconn.WithDialIAMAuthN(*i.IAMAuthN)) } - if i.PrivateIP != nil && *i.PrivateIP || i.PrivateIP == nil && c.PrivateIP { + switch { + // If private IP is enabled at the instance level, or private IP is enabled globally + // add the option. + case i.PrivateIP != nil && *i.PrivateIP || i.PrivateIP == nil && c.PrivateIP: opts = append(opts, cloudsqlconn.WithPrivateIP()) - } else { - opts = append(opts, cloudsqlconn.WithPublicIP()) + case c.AutoIP: + opts = append(opts, cloudsqlconn.WithAutoIP()) + default: + // assume public IP by default } return opts