From 8750e0dea32e5ce9f023b2b9ffddbc20aee12c2f Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Wed, 18 Oct 2023 22:21:44 -0700 Subject: [PATCH 1/2] feat: Improve messaging for common cloud config and rpc errors 1. Add a UnaryClientInterceptor for all gRPC client connections to catch and rewrite unauthenticated error messages 2. Put the sqlc auth token in the config struct, and add simple validation 3. Add more explanatory error messages in cases where users try to use cloud features without the proper configuration 4. Prevent sending the SQLC_AUTH_TOKEN env var to plugins Resolves https://github.com/sqlc-dev/sqlc/issues/2881 Resolves https://github.com/sqlc-dev/sqlc/issues/2881 --- internal/bundler/upload.go | 23 +++++++++++++++++------ internal/cmd/vet.go | 6 ++++-- internal/config/config.go | 33 ++++++++++++++++++++++++++++++--- internal/config/env.go | 17 +++++++++++++++++ internal/config/validate.go | 12 +++++++++--- internal/ext/process/gen.go | 3 +++ internal/quickdb/rpc.go | 6 +++--- internal/remote/rpc.go | 6 +++--- internal/rpc/interceptor.go | 27 +++++++++++++++++++++++++++ 9 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 internal/config/env.go create mode 100644 internal/rpc/interceptor.go diff --git a/internal/bundler/upload.go b/internal/bundler/upload.go index 1152925e02..e60d7dfe08 100644 --- a/internal/bundler/upload.go +++ b/internal/bundler/upload.go @@ -2,8 +2,8 @@ package bundler import ( "context" + "errors" "fmt" - "os" "google.golang.org/protobuf/encoding/protojson" @@ -13,8 +13,20 @@ import ( pb "github.com/sqlc-dev/sqlc/internal/quickdb/v1" ) +var ErrNoProject = errors.New(`project uploads require a cloud project + +If you don't have a project, you can create one from the sqlc Cloud +dashboard at https://dashboard.sqlc.dev/. If you have a project, ensure +you've set its id as the value of the "project" field within the "cloud" +section of your sqlc configuration. The id will look similar to +"01HA8TWGMYPHK0V2GGMB3R2TP9".`) +var ErrNoAuthToken = errors.New(`project uploads require an auth token + +If you don't have an auth token, you can create one from the sqlc Cloud +dashboard at https://dashboard.sqlc.dev/. If you have an auth token, ensure +you've set it as the value of the SQLC_AUTH_TOKEN environment variable.`) + type Uploader struct { - token string configPath string config *config.Config dir string @@ -23,7 +35,6 @@ type Uploader struct { func NewUploader(configPath, dir string, conf *config.Config) *Uploader { return &Uploader{ - token: os.Getenv("SQLC_AUTH_TOKEN"), configPath: configPath, config: conf, dir: dir, @@ -32,10 +43,10 @@ func NewUploader(configPath, dir string, conf *config.Config) *Uploader { func (up *Uploader) Validate() error { if up.config.Cloud.Project == "" { - return fmt.Errorf("cloud.project is not set") + return ErrNoProject } - if up.token == "" { - return fmt.Errorf("SQLC_AUTH_TOKEN environment variable is not set") + if up.config.Cloud.AuthToken == "" { + return ErrNoAuthToken } if up.client == nil { client, err := quickdb.NewClientFromConfig(up.config.Cloud) diff --git a/internal/cmd/vet.go b/internal/cmd/vet.go index 4f79fc8e8b..b567f17043 100644 --- a/internal/cmd/vet.go +++ b/internal/cmd/vet.go @@ -156,6 +156,7 @@ func Vet(ctx context.Context, dir, filename string, opts *Options) error { if err := c.checkSQL(ctx, sql); err != nil { if !errors.Is(err, ErrFailedChecks) { fmt.Fprintf(stderr, "%s\n", err) + continue } errored = true } @@ -400,6 +401,7 @@ func (c *checker) fetchDatabaseUri(ctx context.Context, s config.SQL) (string, f uri, err := c.DSN(s.Database.URI) return uri, cleanup, err } + if s.Engine != config.EnginePostgreSQL { return "", cleanup, fmt.Errorf("managed: only PostgreSQL currently") } @@ -418,8 +420,8 @@ func (c *checker) fetchDatabaseUri(ctx context.Context, s config.SQL) (string, f if err != nil { return "", cleanup, err } - for _, query := range files { - contents, err := os.ReadFile(query) + for _, schema := range files { + contents, err := os.ReadFile(schema) if err != nil { return "", cleanup, fmt.Errorf("read file: %w", err) } diff --git a/internal/config/config.go b/internal/config/config.go index abf264c8e1..26aefc7e70 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -72,6 +72,7 @@ type Cloud struct { Organization string `json:"organization" yaml:"organization"` Project string `json:"project" yaml:"project"` Hostname string `json:"hostname" yaml:"hostname"` + AuthToken string `json:"-" yaml:"-"` } type Plugin struct { @@ -186,9 +187,23 @@ var ErrPluginNoName = errors.New("missing plugin name") var ErrPluginExists = errors.New("a plugin with that name already exists") var ErrPluginNotFound = errors.New("no plugin found") var ErrPluginNoType = errors.New("plugin: field `process` or `wasm` required") -var ErrPluginBothTypes = errors.New("plugin: both `process` and `wasm` cannot both be defined") +var ErrPluginBothTypes = errors.New("plugin: `process` and `wasm` cannot both be defined") var ErrPluginProcessNoCmd = errors.New("plugin: missing process command") +var ErrInvalidDatabase = errors.New("database must be managed or have a non-empty URI") +var ErrManagedDatabaseNoProject = errors.New(`managed databases require a cloud project + +If you don't have a project, you can create one from the sqlc Cloud +dashboard at https://dashboard.sqlc.dev/. If you have a project, ensure +you've set its id as the value of the "project" field within the "cloud" +section of your sqlc configuration. The id will look similar to +"01HA8TWGMYPHK0V2GGMB3R2TP9".`) +var ErrManagedDatabaseNoAuthToken = errors.New(`managed databases require an auth token + +If you don't have an auth token, you can create one from the sqlc Cloud +dashboard at https://dashboard.sqlc.dev/. If you have an auth token, ensure +you've set it as the value of the SQLC_AUTH_TOKEN environment variable.`) + func ParseConfig(rd io.Reader) (Config, error) { var buf bytes.Buffer var config Config @@ -202,14 +217,26 @@ func ParseConfig(rd io.Reader) (Config, error) { if version.Number == "" { return config, ErrMissingVersion } + var err error switch version.Number { case "1": - return v1ParseConfig(&buf) + config, err = v1ParseConfig(&buf) + if err != nil { + return config, err + } case "2": - return v2ParseConfig(&buf) + config, err = v2ParseConfig(&buf) + if err != nil { + return config, err + } default: return config, ErrUnknownVersion } + err = config.addEnvVars() + if err != nil { + return config, err + } + return config, nil } type CombinedSettings struct { diff --git a/internal/config/env.go b/internal/config/env.go new file mode 100644 index 0000000000..0c608aa232 --- /dev/null +++ b/internal/config/env.go @@ -0,0 +1,17 @@ +package config + +import ( + "fmt" + "os" + "strings" +) + +func (c *Config) addEnvVars() error { + authToken := os.Getenv("SQLC_AUTH_TOKEN") + if authToken != "" && !strings.HasPrefix(authToken, "sqlc_") { + return fmt.Errorf("$SQLC_AUTH_TOKEN doesn't start with \"sqlc_\"") + } + c.Cloud.AuthToken = authToken + + return nil +} diff --git a/internal/config/validate.go b/internal/config/validate.go index b374a40c66..dd17b2aa3d 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -1,12 +1,18 @@ package config -import "fmt" - func Validate(c *Config) error { for _, sql := range c.SQL { if sql.Database != nil { if sql.Database.URI == "" && !sql.Database.Managed { - return fmt.Errorf("invalid config: database must be managed or have a non-empty URI") + return ErrInvalidDatabase + } + if sql.Database.Managed { + if c.Cloud.Project == "" { + return ErrManagedDatabaseNoProject + } + if c.Cloud.AuthToken == "" { + return ErrManagedDatabaseNoAuthToken + } } } } diff --git a/internal/ext/process/gen.go b/internal/ext/process/gen.go index 7a48038444..8022941ed4 100644 --- a/internal/ext/process/gen.go +++ b/internal/ext/process/gen.go @@ -37,6 +37,9 @@ func (r Runner) Generate(ctx context.Context, req *plugin.CodeGenRequest) (*plug fmt.Sprintf("SQLC_VERSION=%s", req.SqlcVersion), } for _, key := range r.Env { + if key == "SQLC_AUTH_TOKEN" { + continue + } cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, os.Getenv(key))) } diff --git a/internal/quickdb/rpc.go b/internal/quickdb/rpc.go index c1f9ffd4c2..9d9649d3dc 100644 --- a/internal/quickdb/rpc.go +++ b/internal/quickdb/rpc.go @@ -2,7 +2,6 @@ package quickdb import ( "crypto/tls" - "os" "github.com/riza-io/grpc-go/credentials/basic" "google.golang.org/grpc" @@ -10,14 +9,14 @@ import ( "github.com/sqlc-dev/sqlc/internal/config" pb "github.com/sqlc-dev/sqlc/internal/quickdb/v1" + "github.com/sqlc-dev/sqlc/internal/rpc" ) const defaultHostname = "grpc.sqlc.dev" func NewClientFromConfig(cloudConfig config.Cloud) (pb.QuickClient, error) { projectID := cloudConfig.Project - authToken := os.Getenv("SQLC_AUTH_TOKEN") - return NewClient(projectID, authToken, WithHost(cloudConfig.Hostname)) + return NewClient(projectID, cloudConfig.AuthToken, WithHost(cloudConfig.Hostname)) } type options struct { @@ -41,6 +40,7 @@ func NewClient(project, token string, opts ...Option) (pb.QuickClient, error) { dialOpts := []grpc.DialOption{ grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{})), grpc.WithPerRPCCredentials(basic.NewPerRPCCredentials(project, token)), + grpc.WithUnaryInterceptor(rpc.UnaryInterceptor), } hostname := o.hostname diff --git a/internal/remote/rpc.go b/internal/remote/rpc.go index d7b8556454..4adc9f8681 100644 --- a/internal/remote/rpc.go +++ b/internal/remote/rpc.go @@ -2,23 +2,23 @@ package remote import ( "crypto/tls" - "os" "github.com/riza-io/grpc-go/credentials/basic" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "github.com/sqlc-dev/sqlc/internal/config" + "github.com/sqlc-dev/sqlc/internal/rpc" ) const defaultHostname = "remote.sqlc.dev" func NewClient(cloudConfig config.Cloud) (GenClient, error) { authID := cloudConfig.Organization + "https://github.com/" + cloudConfig.Project - authToken := os.Getenv("SQLC_AUTH_TOKEN") opts := []grpc.DialOption{ grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{})), - grpc.WithPerRPCCredentials(basic.NewPerRPCCredentials(authID, authToken)), + grpc.WithPerRPCCredentials(basic.NewPerRPCCredentials(authID, cloudConfig.AuthToken)), + grpc.WithUnaryInterceptor(rpc.UnaryInterceptor), } hostname := cloudConfig.Hostname diff --git a/internal/rpc/interceptor.go b/internal/rpc/interceptor.go new file mode 100644 index 0000000000..b0262c09fc --- /dev/null +++ b/internal/rpc/interceptor.go @@ -0,0 +1,27 @@ +package rpc + +import ( + "context" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +const errMessageUnauthenticated = `rpc authentication failed + +You may be using a sqlc auth token that was created for a different project, +or your auth token may have expired.` + +func UnaryInterceptor(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + err := invoker(ctx, method, req, reply, cc, opts...) + + switch status.Convert(err).Code() { + case codes.OK: + return nil + case codes.Unauthenticated: + return status.New(codes.Unauthenticated, errMessageUnauthenticated).Err() + default: + return err + } +} From 52b3b253f5b2cfaaff29ca8f515a9ec4f6523141 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Wed, 18 Oct 2023 22:48:26 -0700 Subject: [PATCH 2/2] Don't continue --- internal/cmd/vet.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/cmd/vet.go b/internal/cmd/vet.go index b567f17043..6013c5d40d 100644 --- a/internal/cmd/vet.go +++ b/internal/cmd/vet.go @@ -156,7 +156,6 @@ func Vet(ctx context.Context, dir, filename string, opts *Options) error { if err := c.checkSQL(ctx, sql); err != nil { if !errors.Is(err, ErrFailedChecks) { fmt.Fprintf(stderr, "%s\n", err) - continue } errored = true }