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

fix: connections unique clause should have namespace #337

Merged
merged 5 commits into from
Nov 17, 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
44 changes: 28 additions & 16 deletions context/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,38 @@ var (
)

// extractConnectionNameType extracts the name and connection type from a connection
// string formatted as "connection://<type>/<name>".
func extractConnectionNameType(connectionString string) (name string, connectionType string, found bool) {
// string formatted as "connection://<type>/<namespace>/<name>".
func extractConnectionNameType(connectionString string) (name, namespace, connectionType string, found bool) {
prefix := "connection://"

if !strings.HasPrefix(connectionString, prefix) {
return
}

connectionString = strings.TrimPrefix(connectionString, prefix)
parts := strings.SplitN(connectionString, "/", 2)
if len(parts) != 2 {
parts := strings.Split(connectionString, "/")
if len(parts) > 3 || len(parts) < 2 {
return
}

if parts[0] == "" || parts[1] == "" {
return
}

return parts[1], parts[0], true
if len(parts) == 3 {
name, namespace, connectionType = parts[2], parts[1], parts[0]
return name, namespace, connectionType, true
} else if len(parts) == 2 {
name, connectionType = parts[1], parts[0]
return name, "", connectionType, true
}

return
}

// HydrateConnectionByURL retrieves a connection from the given connection string.
// The connection string is expected to be in one of the following forms:
// - connection://<type>/<name> or
// - connection://<type>/<name> or connection://<type>/<namespace>/<name>
// - the UUID of the connection.
func HydrateConnectionByURL(ctx Context, connectionString string) (*models.Connection, error) {
if connectionString == "" {
Expand Down Expand Up @@ -72,7 +80,7 @@ func IsValidConnectionURL(connectionString string) bool {
if _, err := uuid.Parse(connectionString); err == nil {
return true
}
_, _, found := extractConnectionNameType(connectionString)
_, _, _, found := extractConnectionNameType(connectionString)
return found
}

Expand All @@ -87,24 +95,28 @@ func FindConnectionByURL(ctx Context, connectionString string) (*models.Connecti
return &connection, nil
}

name, connectionType, found := extractConnectionNameType(connectionString)
name, namespace, connectionType, found := extractConnectionNameType(connectionString)
if !found {
return nil, nil
}

connection, err := FindConnection(ctx, connectionType, name)
connection, err := FindConnection(ctx, connectionType, name, namespace)
if err != nil {
return nil, fmt.Errorf("failed to find connection (type=%s, name=%s): %w", connectionType, name, err)
return nil, fmt.Errorf("failed to find connection (type=%s, name=%s, namespace=%s): %w", connectionType, name, namespace, err)
}

return connection, nil
}

// FindConnection returns the connection with the given type and name
func FindConnection(ctx Context, connectionType, name string) (*models.Connection, error) {
func FindConnection(ctx Context, connectionType, name, namespace string) (*models.Connection, error) {
var connection models.Connection

err := ctx.DB().Where("type = ? AND name = ?", connectionType, name).First(&connection).Error
if namespace == "" {
namespace = ctx.GetNamespace()
}

err := ctx.DB().Where("type = ? AND name = ? AND namespace = ?", connectionType, name, namespace).First(&connection).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, nil
Expand All @@ -116,12 +128,12 @@ func FindConnection(ctx Context, connectionType, name string) (*models.Connectio
return &connection, nil
}

func (ctx Context) GetConnection(connectionType string, name string) (*models.Connection, error) {
return GetConnection(ctx, connectionType, name)
func (ctx Context) GetConnection(connectionType, name, namespace string) (*models.Connection, error) {
return GetConnection(ctx, connectionType, name, namespace)
}

func GetConnection(ctx Context, connectionType string, name string) (*models.Connection, error) {
connection, err := FindConnection(ctx, connectionType, name)
func GetConnection(ctx Context, connectionType, name, namespace string) (*models.Connection, error) {
connection, err := FindConnection(ctx, connectionType, name, namespace)
if err != nil {
return nil, err
}
Expand Down
28 changes: 22 additions & 6 deletions context/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,45 +8,52 @@ func TestGetConnectionNameType(t *testing.T) {
connection string
Expect struct {
name string
namespace string
connectionType string
found bool
}
}{
{
name: "valid connection string",
connection: "connection://db/mission_control",
connection: "connection://db/default/mission_control",
Expect: struct {
name string
namespace string
connectionType string
found bool
}{
name: "mission_control",
namespace: "default",
connectionType: "db",
found: true,
},
},
{
name: "valid connection string | name has /",
connection: "connection://db/mission_control//",
connection: "connection://db/default/mission_control//",
Expect: struct {
name string
namespace string
connectionType string
found bool
}{
name: "mission_control//",
connectionType: "db",
found: true,
name: "",
namespace: "",
connectionType: "",
found: false,
},
},
{
name: "invalid | host only",
connection: "connection:///type-only",
Expect: struct {
name string
namespace string
connectionType string
found bool
}{
name: "",
namespace: "",
connectionType: "",
found: false,
},
Expand All @@ -56,10 +63,12 @@ func TestGetConnectionNameType(t *testing.T) {
connection: "invalid-connection-string",
Expect: struct {
name string
namespace string
connectionType string
found bool
}{
name: "",
namespace: "",
connectionType: "",
found: false,
},
Expand All @@ -69,10 +78,12 @@ func TestGetConnectionNameType(t *testing.T) {
connection: "",
Expect: struct {
name string
namespace string
connectionType string
found bool
}{
name: "",
namespace: "",
connectionType: "",
found: false,
},
Expand All @@ -82,10 +93,12 @@ func TestGetConnectionNameType(t *testing.T) {
connection: "connection://type-only",
Expect: struct {
name string
namespace string
connectionType string
found bool
}{
name: "",
namespace: "",
connectionType: "",
found: false,
},
Expand All @@ -94,10 +107,13 @@ func TestGetConnectionNameType(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
name, connectionType, found := extractConnectionNameType(tc.connection)
name, namespace, connectionType, found := extractConnectionNameType(tc.connection)
if name != tc.Expect.name {
t.Errorf("g.Expected name %q, but got %q", tc.Expect.name, name)
}
if namespace != tc.Expect.namespace {
t.Errorf("g.Expected namespace %q, but got %q", tc.Expect.namespace, namespace)
}
if connectionType != tc.Expect.connectionType {
t.Errorf("g.Expected connection type %q, but got %q", tc.Expect.connectionType, connectionType)
}
Expand Down
6 changes: 3 additions & 3 deletions migrate/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ func grantPostgrestRolesToCurrentUser(pool *sql.DB, connection string) error {
logger.Debugf("Granted postgrest_api to %s", user)

grantQuery := `
GRANT SELECT, UPDATE, DELETE, INSERT ON ALL TABLES IN SCHEMA public TO postgrest_api;
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT, UPDATE, DELETE, INSERT ON TABLES TO postgrest_api;
GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public TO postgrest_api;
GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO postgrest_api;
GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA public TO postgrest_api;
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL PRIVILEGES ON TABLES TO postgrest_api;
`
if _, err := pool.Exec(grantQuery); err != nil {
return err
Expand Down
7 changes: 4 additions & 3 deletions schema/connections.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ table "connections" {
type = text
}
column "namespace" {
null = true
null = false
type = text
default = "default"
}
column "type" {
null = false
Expand Down Expand Up @@ -68,9 +69,9 @@ table "connections" {
primary_key {
columns = [column.id]
}
index "connections_name_type_key" {
index "connections_type_name_namespace_key" {
unique = true
columns = [column.type, column.name]
columns = [column.type, column.name, column.namespace]
}
foreign_key "connections_created_by_fkey" {
columns = [column.created_by]
Expand Down
2 changes: 1 addition & 1 deletion tests/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var _ = Describe("Connection", Ordered, func() {
var connection *models.Connection
var err error
It("should be retrieved successfully", func() {
connection, err = testutils.DefaultContext.GetConnection("test", "test")
connection, err = testutils.DefaultContext.GetConnection("test", "test", "default")
Expect(err).ToNot(HaveOccurred())
})

Expand Down
3 changes: 1 addition & 2 deletions views/003_analysis_views.sql
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ ORDER BY
configs.name;

-- analysis_summary_by_component
DROP VIEW IF EXISTS analysis_summary_by_component;

DROP VIEW IF EXISTS analysis_summary_by_component CASCADE;
CREATE OR REPLACE VIEW
analysis_summary_by_component AS
WITH
Expand Down
Loading