Repository: supabase/cli
Pull request #374 (2022-08-17 06:35:34)

fix: escape db password from postgres url
## What kind of change does this PR introduce?

Bug fix: https://github.com/supabase/cli/issues/372

## What is the current behavior?

- db password is not escaped
- container name is fixed
- container removal is not auto

## What is the new behavior?

- fixes issues above and adds unit tests

## Additional context

Add any other context or screenshots.


coveralls: ## Pull Request Test Coverage Report for [Build 2873271077](https://coveralls.io/builds/51720934)

* **31** of **34**   **(91.18%)**  changed or added relevant lines in **4** files are covered.
* **1** unchanged line in **1** file lost coverage.
* Overall coverage increased (+**1.5%**) to **25.607%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [cmd/gen.go](https://coveralls.io/builds/51720934/source?filename=cmd%2Fgen.go#L30) | 2 | 5 | 40.0%
<!-- | **Total:** | **31** | **34** | **91.18%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [cmd/gen.go](https://coveralls.io/builds/51720934/source?filename=cmd%2Fgen.go#L32) | 1 | 64.29% |
<!-- | **Total:** | **1** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/51720934/badge)](https://coveralls.io/builds/51720934) |
| :-- | --: |
| Change from base [Build 2872403739](https://coveralls.io/builds/51717786): |  1.5% |
| Covered Lines: | 1265 |
| Relevant Lines: | 4940 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/cmd/gen.go b/cmd/gen.go
index 1ed6d28c..f207eefa 100644
--- a/cmd/gen.go
+++ b/cmd/gen.go
@@ -1,6 +1,9 @@
 package cmd
 
 import (
+	"errors"
+
+	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/supabase/cli/internal/gen/types/typescript"
 )
@@ -16,29 +19,27 @@ var (
 		Short: "Generate types from Postgres schema",
 	}
 
+	local bool
+	dbUrl string
+
 	genTypesTypescriptCmd = &cobra.Command{
 		Use:   "typescript",
 		Short: "Generate types for TypeScript",
 		Long:  "Generate types for TypeScript. Must specify either --local or --db-url",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			useLocal, err := cmd.Flags().GetBool("local")
-			if err != nil {
-				return err
-			}
-			dbUrl, err := cmd.Flags().GetString("db-url")
-			if err != nil {
-				return err
+			if !local && dbUrl == "" {
+				return errors.New("Must specify either --local or --db-url")
 			}
 
-			return typescript.Run(useLocal, dbUrl)
+			return typescript.Run(local, dbUrl, afero.NewOsFs())
 		},
 	}
 )
 
 func init() {
 	genFlags := genTypesTypescriptCmd.Flags()
-	genFlags.Bool("local", false, "Generate types from the local dev database.")
-	genFlags.String("db-url", "", "Generate types from a database url.")
+	genFlags.BoolVar(&local, "local", false, "Generate types from the local dev database.")
+	genFlags.StringVar(&dbUrl, "db-url", "", "Generate types from a database url.")
 	genTypesTypescriptCmd.MarkFlagsMutuallyExclusive("local", "db-url")
 	genTypesCmd.AddCommand(genTypesTypescriptCmd)
 	genCmd.AddCommand(genTypesCmd)
diff --git a/internal/db/remote/set/set.go b/internal/db/remote/set/set.go
index dcb61ab3..8c5ef775 100644
--- a/internal/db/remote/set/set.go
+++ b/internal/db/remote/set/set.go
@@ -37,10 +37,10 @@ func Run(url string, fsys afero.Fs, options ...func(*pgx.ConnConfig)) error {
 	}
 
 	matches := utils.PostgresUrlPattern.FindStringSubmatch(url)
-	if len(matches) != 4 {
+	if len(matches) != 3 {
 		return errors.New("URL is not a valid Supabase connection string.")
 	}
-	url = "postgresql://postgres:" + u.QueryEscape(matches[1]) + "@" + u.QueryEscape(matches[2]) + matches[3] + "/postgres"
+	url = "postgresql://postgres:" + u.QueryEscape(matches[1]) + "@" + matches[2] + "/postgres"
 
 	config, err := pgx.ParseConfig(url)
 	if err != nil {
diff --git a/internal/gen/types/typescript/typescript.go b/internal/gen/types/typescript/typescript.go
index 73cd1ff1..ff7a28e6 100644
--- a/internal/gen/types/typescript/typescript.go
+++ b/internal/gen/types/typescript/typescript.go
@@ -5,31 +5,24 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io"
-	"os"
+	"net/url"
 	"strings"
 
 	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/pkg/stdcopy"
+	"github.com/spf13/afero"
 	"github.com/supabase/cli/internal/utils"
 )
 
 var ctx = context.Background()
 
-func Run(useLocal bool, dbUrl string) error {
-	if useLocal && dbUrl != "" {
-		return errors.New("Cannot specify both --local and --db-url")
-	} else if !useLocal && dbUrl == "" {
-		return errors.New("Must specify either --local or --db-url")
-	}
-
-	if err := utils.LoadConfig(); err != nil {
+func Run(useLocal bool, dbUrl string, fsys afero.Fs) error {
+	if err := utils.LoadConfigFS(fsys); err != nil {
 		return err
 	}
 
 	if useLocal {
-		if err := utils.AssertSupabaseStartIsRunning(); err != nil {
+		if err := utils.AssertSupabaseDbIsRunning(); err != nil {
 			return err
 		}
 
@@ -69,54 +62,32 @@ func Run(useLocal bool, dbUrl string) error {
 
 	// run typegen on the dbUrl
 	{
-		if err := utils.AssertDockerIsRunning(); err != nil {
-			return err
-		}
-
-		defer utils.DockerRemoveAll()
-
-		metaImage := utils.GetRegistryImageUrl(utils.PgmetaImage)
-		if _, _, err := utils.Docker.ImageInspectWithRaw(ctx, metaImage); err != nil {
-			fmt.Fprintln(os.Stderr, "Downloading type generator...")
-			out, err := utils.Docker.ImagePull(ctx, metaImage, types.ImagePullOptions{})
-			if err != nil {
-				return err
-			}
-			if _, err := io.ReadAll(out); err != nil {
-				return err
-			}
-			if err := out.Close(); err != nil {
-				return err
-			}
-			fmt.Fprintln(os.Stderr, "Done downloading type generator")
+		matches := utils.PostgresUrlPattern.FindStringSubmatch(dbUrl)
+		if len(matches) != 3 {
+			return errors.New("URL is not a valid Supabase connection string.")
 		}
-
-		out, err := utils.DockerRun(
-			ctx,
-			"supabase_gen_types_typescript",
-			&container.Config{
-				Image: utils.PgmetaImage,
-				Env: []string{
-					"PG_META_DB_URL=" + dbUrl,
-				},
-				Cmd: []string{
-					"node", "bin/src/server/app.js", "gen", "types", "typescript", "--include-schemas", strings.Join(append([]string{"public"}, utils.Config.Api.Schemas...), ","),
-				},
-			},
-			&container.HostConfig{},
+		escaped := fmt.Sprintf(
+			"postgresql://postgres:%s@%s/postgres",
+			url.QueryEscape(matches[1]),
+			matches[2],
 		)
+
+		out, err := utils.DockerRunOnce(ctx, utils.PgmetaImage, []string{
+			"PG_META_DB_URL=" + escaped,
+		}, []string{
+			"node",
+			"bin/src/server/app.js",
+			"gen",
+			"types",
+			"typescript",
+			"--include-schemas",
+			strings.Join(append(utils.Config.Api.Schemas, "public"), ","),
+		})
 		if err != nil {
 			return err
 		}
-		var genBuf, errBuf bytes.Buffer
-		if _, err := stdcopy.StdCopy(&genBuf, &errBuf, out); err != nil {
-			return err
-		}
-		if errBuf.Len() > 0 {
-			return errors.New(errBuf.String())
-		}
 
-		fmt.Print(genBuf.String())
+		fmt.Print(out)
 	}
 
 	return nil
diff --git a/internal/utils/docker.go b/internal/utils/docker.go
index e6c68163..fe2aafc2 100644
--- a/internal/utils/docker.go
+++ b/internal/utils/docker.go
@@ -160,9 +160,11 @@ func GetRegistryImageUrl(imageName string) string {
 }
 
 func DockerPullImageIfNotCached(ctx context.Context, imageName string) error {
-	imageUrl := "docker.io/" + imageName
+	imageUrl := GetRegistryImageUrl(imageName)
 	if _, _, err := Docker.ImageInspectWithRaw(ctx, imageUrl); err == nil {
 		return nil
+	} else if !client.IsErrNotFound(err) {
+		return err
 	}
 	out, err := Docker.ImagePull(ctx, imageUrl, types.ImagePullOptions{})
 	if err != nil {
@@ -185,7 +187,7 @@ func DockerRunOnce(ctx context.Context, image string, env []string, cmd []string
 	}
 	// Create container from image
 	resp, err := Docker.ContainerCreate(ctx, &container.Config{
-		Image: image,
+		Image: GetRegistryImageUrl(image),
 		Env:   env,
 		Cmd:   cmd,
 		Labels: map[string]string{
diff --git a/internal/utils/misc.go b/internal/utils/misc.go
index 77ad6e78..9c771833 100644
--- a/internal/utils/misc.go
+++ b/internal/utils/misc.go
@@ -67,7 +67,7 @@ var (
 
 	AccessTokenPattern = regexp.MustCompile(`^sbp_[a-f0-9]{40}$`)
 	ProjectRefPattern  = regexp.MustCompile(`^[a-z]{20}$`)
-	PostgresUrlPattern = regexp.MustCompile(`^postgres(?:ql)?://postgres:(.+)@(.+?)(:\d+)?/postgres$`)
+	PostgresUrlPattern = regexp.MustCompile(`^postgres(?:ql)?:\/\/postgres:(.*)@(.+)\/postgres$`)
 	MigrateFilePattern = regexp.MustCompile(`([0-9]+)_.*\.sql`)
 	BranchNamePattern  = regexp.MustCompile(`[[:word:]-]+`)
 


Test patch:
diff --git a/internal/gen/types/typescript/typescript_test.go b/internal/gen/types/typescript/typescript_test.go
new file mode 100644
index 00000000..1271c8f2
--- /dev/null
+++ b/internal/gen/types/typescript/typescript_test.go
@@ -0,0 +1,162 @@
+package typescript
+
+import (
+	"bytes"
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/client"
+	"github.com/docker/docker/pkg/stdcopy"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/supabase/cli/internal/utils"
+	"gopkg.in/h2non/gock.v1"
+)
+
+func ListUnmatchedRequests() []string {
+	result := make([]string, len(gock.GetUnmatchedRequests()))
+	for i, r := range gock.GetUnmatchedRequests() {
+		result[i] = fmt.Sprintln(r.Method, r.URL.Path)
+	}
+	return result
+}
+
+func TestGenLocalCommand(t *testing.T) {
+	const version = "1.41"
+
+	t.Run("throws error on missing config", func(t *testing.T) {
+		assert.Error(t, Run(true, "", afero.NewMemMapFs()))
+	})
+
+	t.Run("throws error when db is not started", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		gock.New("http:///var/run/docker.sock").
+			Get("/v" + version + "/containers").
+			Reply(http.StatusServiceUnavailable)
+		// Run test
+		assert.Error(t, Run(true, "", fsys))
+		// Validate output
+		assert.False(t, gock.HasUnmatchedRequest(), ListUnmatchedRequests())
+	})
+
+	t.Run("throws error failure to exec", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		gock.New("http:///var/run/docker.sock").
+			Get("/v" + version + "/containers").
+			Reply(200).
+			JSON(types.ContainerJSON{})
+		gock.New("http:///var/run/docker.sock").
+			Post("/v" + version + "/containers").
+			Reply(http.StatusServiceUnavailable)
+		// Run test
+		assert.Error(t, Run(true, "", fsys))
+		// Validate output
+		assert.False(t, gock.HasUnmatchedRequest(), ListUnmatchedRequests())
+	})
+}
+
+func TestGenRemoteCommand(t *testing.T) {
+	const dbUrl = "postgres://postgres:@localhost:5432/postgres"
+	const version = "1.41"
+	const containerId = "test-container"
+
+	t.Run("generates type from remote db", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		gock.New("http:///var/run/docker.sock").
+			Get("/v" + version + "/images").
+			Reply(http.StatusOK).
+			JSON(types.ImageInspect{})
+		gock.New("http:///var/run/docker.sock").
+			Post("/v" + version + "/containers/create").
+			Reply(http.StatusOK).
+			JSON(container.ContainerCreateCreatedBody{ID: containerId})
+		gock.New("http:///var/run/docker.sock").
+			Post("/v" + version + "/containers/" + containerId + "/start").
+			Reply(http.StatusAccepted)
+		// Setup docker style logs
+		var body bytes.Buffer
+		writer := stdcopy.NewStdWriter(&body, stdcopy.Stdout)
+		_, err := writer.Write([]byte("hello world"))
+		require.NoError(t, err)
+		gock.New("http:///var/run/docker.sock").
+			Get("/v"+version+"/containers/"+containerId+"/logs").
+			Reply(http.StatusOK).
+			SetHeader("Content-Type", "application/vnd.docker.raw-stream").
+			Body(&body)
+		gock.New("http:///var/run/docker.sock").
+			Get("/v" + version + "/containers/" + containerId + "/json").
+			Reply(http.StatusOK).
+			JSON(types.ContainerJSONBase{State: &types.ContainerState{ExitCode: 0}})
+		// Run test
+		assert.NoError(t, Run(false, dbUrl, fsys))
+		// Validate output
+		assert.False(t, gock.HasUnmatchedRequest(), ListUnmatchedRequests())
+	})
+
+	t.Run("throws error on missing config", func(t *testing.T) {
+		assert.Error(t, Run(false, "", afero.NewMemMapFs()))
+	})
+
+	t.Run("throws error on malformed db url", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Run test
+		assert.Error(t, Run(false, "", fsys))
+	})
+
+	t.Run("throws error when docker is not started", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		gock.New("http:///var/run/docker.sock").
+			Get("/v" + version + "/images").
+			Reply(http.StatusServiceUnavailable)
+		// Run test
+		assert.Error(t, Run(false, dbUrl, fsys))
+		// Validate output
+		assert.False(t, gock.HasUnmatchedRequest(), ListUnmatchedRequests())
+	})
+}
diff --git a/internal/utils/docker_test.go b/internal/utils/docker_test.go
index ffcc3171..8f16d838 100644
--- a/internal/utils/docker_test.go
+++ b/internal/utils/docker_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/stdcopy"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/h2non/gock.v1"
@@ -23,6 +24,8 @@ const (
 )
 
 func TestPullImage(t *testing.T) {
+	viper.Set("INTERNAL_IMAGE_REGISTRY", "docker.io")
+
 	t.Run("pulls image if missing", func(t *testing.T) {
 		// Setup mock docker
 		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(Docker))
@@ -87,6 +90,8 @@ func TestPullImage(t *testing.T) {
 }
 
 func TestRunOnce(t *testing.T) {
+	viper.Set("INTERNAL_IMAGE_REGISTRY", "docker.io")
+
 	t.Run("runs once in container", func(t *testing.T) {
 		// Setup mock docker
 		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(Docker))
diff --git a/test/main_test.go b/test/main_test.go
index 402acda8..89d6bd4a 100644
--- a/test/main_test.go
+++ b/test/main_test.go
@@ -41,11 +41,7 @@ func TestMain(m *testing.M) {
 	if err != nil {
 		Logger.Fatal(err)
 	}
-	os.Setenv("SUPABASE_INTERNAL_API_HOST", "http://127.0.0.1"+SupabasePort)
-
-	// Configure viper env loader
-	viper.SetEnvPrefix("SUPABASE")
-	viper.AutomaticEnv()
+	viper.Set("INTERNAL_API_HOST", "http://127.0.0.1"+SupabasePort)
 
 	// run tests
 	exitVal := m.Run()