Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve messaging for common cloud config and rpc errors #2885

Merged
merged 2 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions internal/bundler/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package bundler

import (
"context"
"errors"
"fmt"
"os"

"google.golang.org/protobuf/encoding/protojson"

Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions internal/cmd/vet.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,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")
}
Expand All @@ -418,8 +419,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)
}
Expand Down
33 changes: 30 additions & 3 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions internal/config/env.go
Original file line number Diff line number Diff line change
@@ -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
}
Comment on lines +9 to +17
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to get away from calling os.Getenv() all over the place, but I'm not sure this (adding fields to Config struct and then populating after parse) is the best way to do it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think as long as we only do this in one place, here in the config package, it's okay.

12 changes: 9 additions & 3 deletions internal/config/validate.go
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions internal/ext/process/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment on lines +40 to +42
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really need this, it just seemed like a reasonable thing to do (prevent users from sending their auth tokens to plugins).

cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, os.Getenv(key)))
}

Expand Down
6 changes: 3 additions & 3 deletions internal/quickdb/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,21 @@ package quickdb

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"
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 {
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions internal/remote/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 + "/" + 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
Expand Down
27 changes: 27 additions & 0 deletions internal/rpc/interceptor.go
Original file line number Diff line number Diff line change
@@ -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
}
}
Loading