From 82c9800677a8e940f81d19514bea883e27cc899a Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Mon, 15 Jun 2026 14:42:51 +0200 Subject: [PATCH] Drop unused access review source category Source category was never surfaced in the product and added noise to snapshots and APIs. Remove the enum, columns, and service fields so campaign sources track only the identity fields reviewers need. Signed-off-by: Bryan Frimin --- pkg/accessreview/source_service.go | 15 +--- pkg/coredata/access_review_campaign_source.go | 75 ++++++++++++++---- .../access_review_entry_upsert_test.go | 2 - pkg/coredata/access_review_source.go | 25 ++---- pkg/coredata/access_review_source_category.go | 76 ------------------ .../access_review_source_category_test.go | 78 ------------------- pkg/coredata/migrations/20260615T120000Z.sql | 23 ++++++ 7 files changed, 94 insertions(+), 200 deletions(-) delete mode 100644 pkg/coredata/access_review_source_category.go delete mode 100644 pkg/coredata/access_review_source_category_test.go create mode 100644 pkg/coredata/migrations/20260615T120000Z.sql diff --git a/pkg/accessreview/source_service.go b/pkg/accessreview/source_service.go index 4c17bfc9d..58845cdb1 100644 --- a/pkg/accessreview/source_service.go +++ b/pkg/accessreview/source_service.go @@ -37,14 +37,12 @@ type ( OrganizationID gid.GID ConnectorID *gid.GID Name string - Category coredata.AccessReviewSourceCategory CsvData *string } UpdateAccessReviewSourceRequest struct { AccessReviewSourceID gid.GID - Name *string - Category *coredata.AccessReviewSourceCategory + Name **string ConnectorID **gid.GID CsvData **string } @@ -60,7 +58,6 @@ func (r *CreateAccessReviewSourceRequest) Validate() error { v.Check(r.OrganizationID, "organization_id", validator.Required(), validator.GID(coredata.OrganizationEntityType)) v.Check(r.Name, "name", validator.SafeTextNoNewLine(NameMaxLength)) - v.Check(r.Category, "category", validator.OneOfSlice(coredata.AccessReviewSourceCategories())) return v.Error() } @@ -79,7 +76,6 @@ func (r *UpdateAccessReviewSourceRequest) Validate() error { v.Check(r.AccessReviewSourceID, "access_review_source_id", validator.Required(), validator.GID(coredata.AccessReviewSourceEntityType)) v.Check(r.Name, "name", validator.SafeTextNoNewLine(NameMaxLength)) - v.Check(r.Category, "category", validator.OneOfSlice(coredata.AccessReviewSourceCategories())) return v.Error() } @@ -99,7 +95,6 @@ func (s *Service) CreateSource( OrganizationID: req.OrganizationID, ConnectorID: req.ConnectorID, Name: req.Name, - Category: req.Category, CsvData: req.CsvData, CreatedAt: now, UpdatedAt: now, @@ -169,11 +164,9 @@ func (s *Service) UpdateSource( } if req.Name != nil { - source.Name = *req.Name - } - - if req.Category != nil { - source.Category = *req.Category + if *req.Name != nil { + source.Name = **req.Name + } } if req.ConnectorID != nil { diff --git a/pkg/coredata/access_review_campaign_source.go b/pkg/coredata/access_review_campaign_source.go index d9c377ec0..303c71738 100644 --- a/pkg/coredata/access_review_campaign_source.go +++ b/pkg/coredata/access_review_campaign_source.go @@ -24,29 +24,78 @@ import ( "github.com/jackc/pgx/v5" "go.gearno.de/kit/pg" "go.probo.inc/probo/pkg/gid" + "go.probo.inc/probo/pkg/iam/policy" ) type ( // AccessReviewCampaignSource is the per-campaign snapshot of an access - // source. It captures the source identity (name, category, connector) at + // source. It captures the source identity (name, connector) at // the time the source was scoped into the campaign so that the review's // data survives even if the live access source is later deleted. Access // entries and fetch attempts reference this snapshot, not the live source. AccessReviewCampaignSource struct { - ID gid.GID `db:"id"` - TenantID gid.TenantID `db:"tenant_id"` - AccessReviewCampaignID gid.GID `db:"access_review_campaign_id"` - AccessReviewSourceID *gid.GID `db:"access_review_source_id"` - Name string `db:"name"` - Category AccessReviewSourceCategory `db:"category"` - ConnectorID *gid.GID `db:"connector_id"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` + ID gid.GID `db:"id"` + TenantID gid.TenantID `db:"tenant_id"` + AccessReviewCampaignID gid.GID `db:"access_review_campaign_id"` + AccessReviewSourceID *gid.GID `db:"access_review_source_id"` + Name string `db:"name"` + ConnectorID *gid.GID `db:"connector_id"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` } AccessReviewCampaignSources []*AccessReviewCampaignSource ) +func (s *AccessReviewCampaignSource) AuthorizationAttributes( + ctx context.Context, + conn pg.Querier, + resourceIDs []gid.GID, +) (policy.AttributesByID, error) { + q := ` +SELECT + cs.id, + c.organization_id +FROM + access_review_campaign_sources cs +JOIN + access_review_campaigns c ON c.id = cs.access_review_campaign_id +WHERE + cs.id = ANY(@resource_ids::text[]) +` + + args := pgx.StrictNamedArgs{ + "resource_ids": resourceIDs, + } + + rows, err := conn.Query(ctx, q, args) + if err != nil { + return nil, fmt.Errorf("cannot query authorization attributes: %w", err) + } + + defer rows.Close() + + attrsByID := make(policy.AttributesByID) + + for rows.Next() { + var id, organizationID gid.GID + + if err := rows.Scan(&id, &organizationID); err != nil { + return nil, fmt.Errorf("cannot scan authorization attributes: %w", err) + } + + attrsByID[id] = policy.Attributes{ + "organization_id": organizationID.String(), + } + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("cannot iterate authorization attributes: %w", err) + } + + return attrsByID, nil +} + // Upsert inserts the snapshot or refreshes its denormalized identity from the // live source. The generated ID is preserved across upserts because it is not // part of the conflict target, so entries that already reference the snapshot @@ -63,7 +112,6 @@ INSERT INTO access_review_campaign_sources ( access_review_campaign_id, access_review_source_id, name, - category, connector_id, created_at, updated_at @@ -73,14 +121,12 @@ INSERT INTO access_review_campaign_sources ( @access_review_campaign_id, @access_review_source_id, @name, - @category, @connector_id, @created_at, @updated_at ) ON CONFLICT (access_review_campaign_id, access_review_source_id) DO UPDATE SET name = EXCLUDED.name, - category = EXCLUDED.category, connector_id = EXCLUDED.connector_id, updated_at = EXCLUDED.updated_at RETURNING id @@ -91,7 +137,6 @@ RETURNING id "access_review_campaign_id": s.AccessReviewCampaignID, "access_review_source_id": s.AccessReviewSourceID, "name": s.Name, - "category": s.Category, "connector_id": s.ConnectorID, "created_at": s.CreatedAt, "updated_at": s.UpdatedAt, @@ -117,7 +162,6 @@ SELECT access_review_campaign_id, access_review_source_id, name, - category, connector_id, created_at, updated_at @@ -193,7 +237,6 @@ SELECT access_review_campaign_id, access_review_source_id, name, - category, connector_id, created_at, updated_at diff --git a/pkg/coredata/access_review_entry_upsert_test.go b/pkg/coredata/access_review_entry_upsert_test.go index c6cd0b360..0be9d7d4f 100644 --- a/pkg/coredata/access_review_entry_upsert_test.go +++ b/pkg/coredata/access_review_entry_upsert_test.go @@ -66,7 +66,6 @@ func seedAccessReviewEntryFixture(t *testing.T, ctx context.Context, client *pg. ID: sourceID, OrganizationID: organizationID, Name: "Upsert Freeze Test Source", - Category: coredata.AccessReviewSourceCategorySaaS, CreatedAt: now, UpdatedAt: now, } @@ -92,7 +91,6 @@ func seedAccessReviewEntryFixture(t *testing.T, ctx context.Context, client *pg. AccessReviewCampaignID: campaignID, AccessReviewSourceID: &sourceID, Name: "Upsert Freeze Test Source", - Category: coredata.AccessReviewSourceCategorySaaS, CreatedAt: now, UpdatedAt: now, } diff --git a/pkg/coredata/access_review_source.go b/pkg/coredata/access_review_source.go index 2469e9e40..a8a7cc0ee 100644 --- a/pkg/coredata/access_review_source.go +++ b/pkg/coredata/access_review_source.go @@ -30,15 +30,14 @@ import ( type ( AccessReviewSource struct { - ID gid.GID `db:"id"` - OrganizationID gid.GID `db:"organization_id"` - ConnectorID *gid.GID `db:"connector_id"` - Name string `db:"name"` - Category AccessReviewSourceCategory `db:"category"` - CsvData *string `db:"csv_data"` - NameSyncedAt *time.Time `db:"name_synced_at"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` + ID gid.GID `db:"id"` + OrganizationID gid.GID `db:"organization_id"` + ConnectorID *gid.GID `db:"connector_id"` + Name string `db:"name"` + CsvData *string `db:"csv_data"` + NameSyncedAt *time.Time `db:"name_synced_at"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` } AccessReviewSources []*AccessReviewSource @@ -104,7 +103,6 @@ SELECT organization_id, connector_id, name, - category, csv_data, name_synced_at, created_at, @@ -153,7 +151,6 @@ INSERT INTO organization_id, connector_id, name, - category, csv_data, name_synced_at, created_at, @@ -165,7 +162,6 @@ VALUES ( @organization_id, @connector_id, @name, - @category, @csv_data, @name_synced_at, @created_at, @@ -179,7 +175,6 @@ VALUES ( "organization_id": as.OrganizationID, "connector_id": as.ConnectorID, "name": as.Name, - "category": as.Category, "csv_data": as.CsvData, "name_synced_at": as.NameSyncedAt, "created_at": as.CreatedAt, @@ -203,7 +198,6 @@ func (as *AccessReviewSource) Update( UPDATE access_review_sources SET name = @name, - category = @category, connector_id = @connector_id, csv_data = @csv_data, name_synced_at = @name_synced_at, @@ -217,7 +211,6 @@ WHERE args := pgx.StrictNamedArgs{ "id": as.ID, "name": as.Name, - "category": as.Category, "connector_id": as.ConnectorID, "csv_data": as.CsvData, "name_synced_at": as.NameSyncedAt, @@ -276,7 +269,6 @@ SELECT organization_id, connector_id, name, - category, csv_data, name_synced_at, created_at, @@ -378,7 +370,6 @@ SELECT organization_id, connector_id, name, - category, csv_data, name_synced_at, created_at, diff --git a/pkg/coredata/access_review_source_category.go b/pkg/coredata/access_review_source_category.go deleted file mode 100644 index 78b4a4537..000000000 --- a/pkg/coredata/access_review_source_category.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -package coredata - -import ( - "encoding" - "fmt" -) - -type AccessReviewSourceCategory string - -const ( - AccessReviewSourceCategorySaaS AccessReviewSourceCategory = "SAAS" - AccessReviewSourceCategoryCloudInfra AccessReviewSourceCategory = "CLOUD_INFRA" - AccessReviewSourceCategorySourceCode AccessReviewSourceCategory = "SOURCE_CODE" - AccessReviewSourceCategoryOther AccessReviewSourceCategory = "OTHER" -) - -var ( - _ fmt.Stringer = AccessReviewSourceCategory("") - _ encoding.TextMarshaler = AccessReviewSourceCategory("") - _ encoding.TextUnmarshaler = (*AccessReviewSourceCategory)(nil) -) - -func AccessReviewSourceCategories() []AccessReviewSourceCategory { - return []AccessReviewSourceCategory{ - AccessReviewSourceCategorySaaS, - AccessReviewSourceCategoryCloudInfra, - AccessReviewSourceCategorySourceCode, - AccessReviewSourceCategoryOther, - } -} - -func (v AccessReviewSourceCategory) IsValid() bool { - switch v { - case - AccessReviewSourceCategorySaaS, - AccessReviewSourceCategoryCloudInfra, - AccessReviewSourceCategorySourceCode, - AccessReviewSourceCategoryOther: - return true - } - - return false -} - -func (v AccessReviewSourceCategory) String() string { - return string(v) -} - -func (v AccessReviewSourceCategory) MarshalText() ([]byte, error) { - return []byte(v.String()), nil -} - -func (v *AccessReviewSourceCategory) UnmarshalText(text []byte) error { - val := AccessReviewSourceCategory(text) - if !val.IsValid() { - return fmt.Errorf("invalid AccessReviewSourceCategory value: %q", string(text)) - } - - *v = val - - return nil -} diff --git a/pkg/coredata/access_review_source_category_test.go b/pkg/coredata/access_review_source_category_test.go deleted file mode 100644 index cdcceb0a7..000000000 --- a/pkg/coredata/access_review_source_category_test.go +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -package coredata - -import "testing" - -func TestAccessReviewSourceCategoryIsValid(t *testing.T) { - t.Parallel() - - for _, value := range AccessReviewSourceCategories() { - if !value.IsValid() { - t.Fatalf("IsValid() = false for %q", value) - } - } - - if AccessReviewSourceCategory("BOGUS").IsValid() { - t.Fatal("IsValid() = true for invalid value") - } -} - -func TestAccessReviewSourceCategoryUnmarshalText(t *testing.T) { - t.Parallel() - - for _, value := range AccessReviewSourceCategories() { - t.Run(string(value), func(t *testing.T) { - t.Parallel() - - var got AccessReviewSourceCategory - if err := got.UnmarshalText([]byte(value)); err != nil { - t.Fatalf("UnmarshalText(%q) returned error: %v", value, err) - } - - if got != value { - t.Fatalf("UnmarshalText(%q) = %q, want %q", value, got, value) - } - }) - } - - t.Run("invalid", func(t *testing.T) { - t.Parallel() - - var got AccessReviewSourceCategory - if err := got.UnmarshalText([]byte("BOGUS")); err == nil { - t.Fatal("UnmarshalText(BOGUS) expected error") - } - }) -} - -func TestAccessReviewSourceCategoryMarshalText(t *testing.T) { - t.Parallel() - - for _, value := range AccessReviewSourceCategories() { - t.Run(string(value), func(t *testing.T) { - t.Parallel() - - got, err := value.MarshalText() - if err != nil { - t.Fatalf("MarshalText() returned error: %v", err) - } - - if string(got) != value.String() { - t.Fatalf("MarshalText() = %q, want %q", string(got), value.String()) - } - }) - } -} diff --git a/pkg/coredata/migrations/20260615T120000Z.sql b/pkg/coredata/migrations/20260615T120000Z.sql new file mode 100644 index 000000000..8d623462a --- /dev/null +++ b/pkg/coredata/migrations/20260615T120000Z.sql @@ -0,0 +1,23 @@ +-- Copyright (c) 2026 Probo Inc . +-- +-- Permission to use, copy, modify, and/or distribute this software for any +-- purpose with or without fee is hereby granted, provided that the above +-- copyright notice and this permission notice appear in all copies. +-- +-- THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +-- REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +-- AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +-- INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +-- LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +-- OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +-- PERFORMANCE OF THIS SOFTWARE. + +-- Access review: drop unused source category. + +ALTER TABLE access_review_sources + DROP COLUMN category; + +ALTER TABLE access_review_campaign_sources + DROP COLUMN category; + +DROP TYPE access_review_source_category;