Merge branch 'main' into fuse-config

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,15 +683,15 @@ 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],
 			))
 	}
 
 }
 
 // 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) {

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 {

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