Skip to content

Commit

Permalink
Decouple entities from groups
Browse files Browse the repository at this point in the history
This removes the group information from entities in favor of
relying on providers for data integrity.
  • Loading branch information
JAORMX committed Sep 16, 2023
1 parent d8abb38 commit 84ce748
Show file tree
Hide file tree
Showing 32 changed files with 397 additions and 440 deletions.
12 changes: 6 additions & 6 deletions database/migrations/000001_init.up.sql
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ CREATE TABLE providers (
CREATE TABLE provider_access_tokens (
id SERIAL PRIMARY KEY,
provider_id UUID NOT NULL REFERENCES providers(id) ON DELETE CASCADE,
group_id INTEGER NOT NULL REFERENCES groups(id) ON DELETE CASCADE,
owner_filter TEXT,
encrypted_token TEXT NOT NULL,
expiration_time TIMESTAMP NOT NULL,
Expand All @@ -129,7 +128,6 @@ CREATE TABLE signing_keys (
CREATE TABLE repositories (
id SERIAL PRIMARY KEY,
provider UUID NOT NULL REFERENCES providers(id) ON DELETE CASCADE,
group_id INTEGER NOT NULL REFERENCES groups(id) ON DELETE CASCADE,
repo_owner TEXT NOT NULL,
repo_name TEXT NOT NULL,
repo_id INTEGER NOT NULL,
Expand Down Expand Up @@ -176,6 +174,9 @@ CREATE TABLE session_store (
created_at TIMESTAMP NOT NULL DEFAULT NOW()
);

-- table for storing rule types
-- Note we'll leave the group_id in here since we'll de-couple the provider
-- reference from this table in favor of referencing provider types.
CREATE TABLE rule_type (
id SERIAL PRIMARY KEY,
name TEXT NOT NULL,
Expand All @@ -192,7 +193,6 @@ CREATE TABLE policies (
id SERIAL PRIMARY KEY,
name TEXT NOT NULL,
provider UUID NOT NULL REFERENCES providers(id) ON DELETE CASCADE,
group_id INTEGER NOT NULL REFERENCES groups(id) ON DELETE CASCADE,
created_at TIMESTAMP NOT NULL DEFAULT NOW(),
updated_at TIMESTAMP NOT NULL DEFAULT NOW()
);
Expand Down Expand Up @@ -239,7 +239,7 @@ CREATE TABLE rule_evaluation_status (
ALTER TABLE projects ADD CONSTRAINT parent_child_not_equal CHECK (id != parent_id);

-- Unique constraint
ALTER TABLE provider_access_tokens ADD CONSTRAINT unique_group_id UNIQUE (group_id);
ALTER TABLE provider_access_tokens ADD CONSTRAINT unique_provider_id UNIQUE (provider_id);
ALTER TABLE repositories ADD CONSTRAINT unique_repo_id UNIQUE (repo_id);
ALTER TABLE signing_keys ADD CONSTRAINT unique_key_identifier UNIQUE (key_identifier);

Expand All @@ -250,11 +250,11 @@ CREATE INDEX idx_users_organization_id ON users(organization_id);
CREATE INDEX idx_groups_organization_id ON groups(organization_id);
CREATE INDEX idx_roles_group_id ON roles(group_id);
CREATE UNIQUE INDEX roles_organization_id_name_lower_idx ON roles (organization_id, LOWER(name));
CREATE INDEX idx_provider_access_tokens_group_id ON provider_access_tokens(group_id);
CREATE INDEX idx_provider_access_tokens_provider_id ON provider_access_tokens(provider_id);
CREATE UNIQUE INDEX users_organization_id_email_lower_idx ON users (organization_id, LOWER(email));
CREATE UNIQUE INDEX users_organization_id_username_lower_idx ON users (organization_id, LOWER(username));
CREATE UNIQUE INDEX repositories_repo_id_idx ON repositories(repo_id);
CREATE UNIQUE INDEX policies_group_id_policy_name_idx ON policies(provider, group_id, name);
CREATE UNIQUE INDEX policies_policy_name_idx ON policies(provider, name);
CREATE UNIQUE INDEX rule_type_idx ON rule_type(provider, group_id, name);
CREATE UNIQUE INDEX rule_evaluation_status_results_idx ON rule_evaluation_status(policy_id, repository_id, COALESCE(artifact_id, 0), entity, rule_type_id);
CREATE UNIQUE INDEX artifact_name_lower_idx ON artifacts (repository_id, LOWER(artifact_name));
Expand Down
149 changes: 82 additions & 67 deletions database/mock/store.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion database/query/artifacts.sql
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ RETURNING *;
-- name: GetArtifactByID :one
SELECT artifacts.id, artifacts.repository_id, artifacts.artifact_name, artifacts.artifact_type,
artifacts.artifact_visibility, artifacts.created_at,
repositories.provider, repositories.group_id, repositories.repo_owner, repositories.repo_name
repositories.provider, repositories.repo_owner, repositories.repo_name
FROM artifacts INNER JOIN repositories ON repositories.id = artifacts.repository_id
WHERE artifacts.id = $1;

Expand Down
15 changes: 7 additions & 8 deletions database/query/policies.sql
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
-- name: CreatePolicy :one
INSERT INTO policies (
provider,
group_id,
name) VALUES ($1, $2, $3) RETURNING *;
name) VALUES ($1, $2) RETURNING *;

-- name: CreatePolicyForEntity :one
INSERT INTO entity_policies (
entity,
policy_id,
contextual_rules) VALUES ($1, $2, sqlc.arg(contextual_rules)::jsonb) RETURNING *;

-- name: GetPolicyByGroupAndID :many
-- name: GetPolicyByProviderAndID :many
SELECT * FROM policies JOIN entity_policies ON policies.id = entity_policies.policy_id
WHERE policies.group_id = $1 AND policies.id = $2;
WHERE policies.provider = $1 AND policies.id = $2;

-- name: GetPolicyByID :one
SELECT * FROM policies WHERE id = $1;

-- name: GetPolicyByGroupAndName :many
-- name: GetPolicyByProviderAndName :many
SELECT * FROM policies JOIN entity_policies ON policies.id = entity_policies.policy_id
WHERE policies.group_id = $1 AND policies.name = $2;
WHERE policies.provider = $1 AND policies.name = $2;

-- name: ListPoliciesByGroupID :many
-- name: ListPoliciesByProvider :many
SELECT * FROM policies JOIN entity_policies ON policies.id = entity_policies.policy_id
WHERE policies.group_id = $1;
WHERE policies.provider = $1;

-- name: DeletePolicy :exec
DELETE FROM policies
Expand Down
6 changes: 4 additions & 2 deletions database/query/policy_status.sql
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ WHERE rule_evaluation_status.policy_id = $1
-- name: GetPolicyStatusByIdAndGroup :one
SELECT p.id, p.name, ps.policy_status, ps.last_updated FROM policy_status ps
INNER JOIN policies p ON p.id = ps.policy_id
WHERE p.id = $1 AND p.group_id = $2;
INNER JOIN providers pr ON pr.id = p.provider_id
WHERE p.id = $1 AND pr.group_id = $2;

-- name: GetPolicyStatusByGroup :many
SELECT p.id, p.name, ps.policy_status, ps.last_updated FROM policy_status ps
INNER JOIN policies p ON p.id = ps.policy_id
WHERE p.group_id = $1;
INNER JOIN providers pr ON pr.id = p.provider_id
WHERE pr.group_id = $1;

-- name: ListRuleEvaluationStatusByPolicyId :many
SELECT res.eval_status as eval_status, res.last_updated as last_updated, res.details as details, res.repository_id as repository_id, res.entity as entity, repo.repo_name as repo_name, repo.repo_owner as repo_owner, repo.provider as provider, rt.name as rule_type_name, rt.id as rule_type_id
Expand Down
12 changes: 6 additions & 6 deletions database/query/provider_access_tokens.sql
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
-- name: CreateAccessToken :one
INSERT INTO provider_access_tokens (group_id, provider_id, encrypted_token, expiration_time, owner_filter) VALUES ($1, $2, $3, $4, $5) RETURNING *;
INSERT INTO provider_access_tokens (provider_id, encrypted_token, expiration_time, owner_filter) VALUES ($1, $2, $3, $4) RETURNING *;

-- name: GetAccessTokenByGroupID :one
SELECT * FROM provider_access_tokens WHERE provider_id = $1 AND group_id = $2;
-- name: GetAccessTokenByProviderID :one
SELECT * FROM provider_access_tokens WHERE provider_id = $1;

-- name: UpdateAccessToken :one
UPDATE provider_access_tokens SET encrypted_token = $3, expiration_time = $4, owner_filter = $5, updated_at = NOW() WHERE provider_id = $1 AND group_id = $2 RETURNING *;
UPDATE provider_access_tokens SET encrypted_token = $2, expiration_time = $3, owner_filter = $4, updated_at = NOW() WHERE provider_id = $1 RETURNING *;

-- name: DeleteAccessToken :exec
DELETE FROM provider_access_tokens WHERE provider_id = $1 AND group_id = $2;
DELETE FROM provider_access_tokens WHERE provider_id = $1;

-- name: GetAccessTokenByProvider :many
SELECT * FROM provider_access_tokens WHERE provider_id = $1;

-- name: GetAccessTokenSinceDate :one
SELECT * FROM provider_access_tokens WHERE provider_id = $1 AND group_id = $2 AND created_at >= $3;
SELECT * FROM provider_access_tokens WHERE provider_id = $1 AND created_at >= $2;
3 changes: 3 additions & 0 deletions database/query/providers.sql
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ SELECT * FROM providers WHERE group_id = $1;
-- name: GlobalListProviders :many
SELECT * FROM providers;

-- name: GlobalGetProviderByID :one
SELECT * FROM providers WHERE id = $1;

-- name: DeleteProvider :exec
DELETE FROM providers WHERE id = $1 AND group_id = $2;
55 changes: 26 additions & 29 deletions database/query/repositories.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
-- name: CreateRepository :one
INSERT INTO repositories (
provider,
group_id,
repo_owner,
repo_name,
repo_id,
Expand All @@ -10,7 +9,7 @@ INSERT INTO repositories (
webhook_id,
webhook_url,
deploy_url,
clone_url) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING *;
clone_url) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING *;

-- name: GetRepositoryByID :one
SELECT * FROM repositories WHERE id = $1;
Expand All @@ -21,19 +20,19 @@ SELECT * FROM repositories WHERE repo_id = $1;
-- name: GetRepositoryByRepoName :one
SELECT * FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3;

-- name: GetRepositoryByIDAndGroup :one
SELECT * FROM repositories WHERE provider = $1 AND repo_id = $2 AND group_id = $3;
-- name: GetRepositoryByIDAndProvider :one
SELECT * FROM repositories WHERE provider = $1 AND repo_id = $2;

-- name: ListRepositoriesByGroupID :many
-- name: ListRepositoriesByProvider :many
SELECT * FROM repositories
WHERE provider = $1 AND group_id = $2
WHERE provider = $1
ORDER BY id
LIMIT $3
OFFSET $4;
LIMIT $2
OFFSET $3;

-- name: ListRegisteredRepositoriesByGroupIDAndProvider :many
-- name: ListRegisteredRepositoriesByProvider :many
SELECT * FROM repositories
WHERE provider = $1 AND group_id = $2 AND webhook_id IS NOT NULL
WHERE provider = $1 AND webhook_id IS NOT NULL
ORDER BY id;

-- name: ListRepositoriesByOwner :many
Expand All @@ -50,32 +49,30 @@ ORDER BY id;

-- name: UpdateRepository :one
UPDATE repositories
SET group_id = $2,
repo_owner = $3,
repo_name = $4,
repo_id = $5,
is_private = $6,
is_fork = $7,
webhook_id = $8,
webhook_url = $9,
deploy_url = $10,
provider = $11,
SET repo_owner = $2,
repo_name = $3,
repo_id = $4,
is_private = $5,
is_fork = $6,
webhook_id = $7,
webhook_url = $8,
deploy_url = $9,
provider = $10,
-- set clone_url if the value is not an empty string
clone_url = CASE WHEN sqlc.arg(clone_url)::text = '' THEN clone_url ELSE sqlc.arg(clone_url)::text END,
updated_at = NOW()
WHERE id = $1 RETURNING *;

-- name: UpdateRepositoryByID :one
UPDATE repositories
SET group_id = $2,
repo_owner = $3,
repo_name = $4,
is_private = $5,
is_fork = $6,
webhook_id = $7,
webhook_url = $8,
deploy_url = $9,
provider = $10,
SET repo_owner = $2,
repo_name = $3,
is_private = $4,
is_fork = $5,
webhook_id = $6,
webhook_url = $7,
deploy_url = $8,
provider = $9,
clone_url = CASE WHEN sqlc.arg(clone_url)::text = '' THEN clone_url ELSE sqlc.arg(clone_url)::text END,
updated_at = NOW()
WHERE repo_id = $1 RETURNING *;
Expand Down
10 changes: 7 additions & 3 deletions internal/controlplane/handlers_artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ func (s *Server) ListArtifacts(ctx context.Context, in *pb.ListArtifactsRequest)
}

// first read all the repositories for provider and group
repositories, err := s.store.ListRegisteredRepositoriesByGroupIDAndProvider(ctx,
db.ListRegisteredRepositoriesByGroupIDAndProviderParams{Provider: provider.ID, GroupID: in.GroupId})
repositories, err := s.store.ListRegisteredRepositoriesByProvider(ctx, provider.ID)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, status.Errorf(codes.NotFound, "repositories not found")
Expand Down Expand Up @@ -108,8 +107,13 @@ func (s *Server) GetArtifactById(ctx context.Context, in *pb.GetArtifactByIdRequ
return nil, status.Errorf(codes.Unknown, "failed to get artifact: %s", err)
}

prov, err := s.store.GlobalGetProviderByID(ctx, artifact.Provider)
if err != nil {
return nil, returnProviderError(err)
}

// check if user is authorized
if !IsRequestAuthorized(ctx, artifact.GroupID) {
if !IsRequestAuthorized(ctx, prov.GroupID) {
return nil, status.Errorf(codes.PermissionDenied, "user is not authorized to access this resource")
}

Expand Down
2 changes: 1 addition & 1 deletion internal/controlplane/handlers_authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (s *Server) IsProviderCallAuthorized(ctx context.Context, provider db.Provi
// check if token is expired
if encToken.Expiry.Unix() < time.Now().Unix() {
// remove from the database and deny the request
_ = s.store.DeleteAccessToken(ctx, db.DeleteAccessTokenParams{ProviderID: provider.ID, GroupID: groupId})
_ = s.store.DeleteAccessToken(ctx, provider.ID)

// remove from github
err := auth.DeleteAccessToken(ctx, provider.Name, encToken.AccessToken)
Expand Down
32 changes: 8 additions & 24 deletions internal/controlplane/handlers_githubwebhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,7 @@ func (s *Server) parseRepoEvent(
return err
}

provider, err := s.store.GetProviderByID(ctx, db.GetProviderByIDParams{
ID: dbrepo.Provider,
GroupID: dbrepo.GroupID,
})
provider, err := s.store.GlobalGetProviderByID(ctx, dbrepo.Provider)
if err != nil {
return fmt.Errorf("error getting provider: %w", err)
}
Expand All @@ -342,7 +339,7 @@ func (s *Server) parseRepoEvent(
eiw := engine.NewEntityInfoWrapper().
WithProvider(provider.Name).
WithRepository(repo).
WithGroupID(dbrepo.GroupID).
WithGroupID(provider.GroupID).
WithRepositoryID(dbrepo.ID)

return eiw.ToMessage(msg)
Expand All @@ -364,17 +361,13 @@ func (s *Server) parseArtifactPublishedEvent(
if err != nil {
return fmt.Errorf("error getting repo information from payload: %w", err)
}
g := dbrepo.GroupID

prov, err := s.store.GetProviderByID(ctx, db.GetProviderByIDParams{
ID: dbrepo.Provider,
GroupID: dbrepo.GroupID,
})
prov, err := s.store.GlobalGetProviderByID(ctx, dbrepo.Provider)
if err != nil {
return fmt.Errorf("error getting provider: %w", err)
}

cli, err := providers.BuildClient(ctx, dbrepo.Provider, g, s.store, s.cryptoEngine)
cli, err := providers.BuildClient(ctx, dbrepo.Provider, s.store, s.cryptoEngine)
if err != nil {
return fmt.Errorf("error building client: %w", err)
}
Expand All @@ -392,7 +385,7 @@ func (s *Server) parseArtifactPublishedEvent(
eiw := engine.NewEntityInfoWrapper().
WithVersionedArtifact(versionedArtifact).
WithProvider(prov.Name).
WithGroupID(dbrepo.GroupID).
WithGroupID(prov.GroupID).
WithRepositoryID(dbrepo.ID).
WithArtifactID(dbArtifact.ID)

Expand All @@ -409,17 +402,13 @@ func (s *Server) parsePullRequestModEvent(
if err != nil {
return fmt.Errorf("error getting repo information from payload: %w", err)
}
g := dbrepo.GroupID

prov, err := s.store.GetProviderByID(ctx, db.GetProviderByIDParams{
ID: dbrepo.Provider,
GroupID: dbrepo.GroupID,
})
prov, err := s.store.GlobalGetProviderByID(ctx, dbrepo.Provider)
if err != nil {
return fmt.Errorf("error getting provider: %w", err)
}

cli, err := providers.BuildClient(ctx, prov.ID, g, s.store, s.cryptoEngine)
cli, err := providers.BuildClient(ctx, prov.ID, s.store, s.cryptoEngine)
if err != nil {
return fmt.Errorf("error building client: %w", err)
}
Expand All @@ -440,7 +429,7 @@ func (s *Server) parsePullRequestModEvent(
WithPullRequest(prEvalInfo).
WithPullRequestID(prEvalInfo.Number).
WithProvider(prov.Name).
WithGroupID(dbrepo.GroupID).
WithGroupID(prov.GroupID).
WithRepositoryID(dbrepo.ID)

return eiw.ToMessage(msg)
Expand Down Expand Up @@ -886,11 +875,6 @@ func getRepoInformationFromPayload(
return db.Repository{}, fmt.Errorf("error getting repository: %w", err)
}

if dbrepo.GroupID == 0 {
return db.Repository{}, fmt.Errorf("no group found for repository %s/%s: %w",
dbrepo.RepoOwner, dbrepo.RepoName, ErrRepoNotFound)
}

return dbrepo, nil
}

Expand Down
1 change: 0 additions & 1 deletion internal/controlplane/handlers_githubwebhooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() {
GetRepositoryByRepoID(gomock.Any(), gomock.Any()).
Return(db.Repository{
ID: 1,
GroupID: 1,
RepoID: 12345,
Provider: providerUUID,
}, nil)
Expand Down
Loading

0 comments on commit 84ce748

Please sign in to comment.