From 0b6a6d9c86f1d223a57ea59bcbf959b7ef692963 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 28 Apr 2026 17:45:53 +0545 Subject: [PATCH 1/7] feat(config): add creator-scoped config properties Config item properties can be produced by scrapers and people, but previous writes had no ownership metadata or scoped replacement semantics. Add property creator provenance fields and a Postgres function that atomically replaces only the properties owned by a specific creator. The function preserves other creators and legacy unowned properties, returns the merged JSON for cache updates, and supports empty input as removal of the creator-owned slice. Expose the SQL function through a model helper and catalog RPC RBAC mapping so config-db can route scraper and manual property updates through the same path. --- functions/config_item_properties.sql | 65 +++++++ models/clone.go | 19 ++- models/config.go | 54 +++--- models/config_item_properties.go | 161 ++++++++++++++++++ query/template_test.go | 4 +- rbac/objects.go | 2 + tests/config_item_properties_function_test.go | 151 ++++++++++++++++ tests/fixtures/dummy/config.go | 8 +- tests/fixtures/dummy/config_demo_cluster.go | 4 +- types/resource_selector_test.go | 16 +- 10 files changed, 435 insertions(+), 49 deletions(-) create mode 100644 functions/config_item_properties.sql create mode 100644 models/config_item_properties.go create mode 100644 tests/config_item_properties_function_test.go diff --git a/functions/config_item_properties.sql b/functions/config_item_properties.sql new file mode 100644 index 000000000..091999cd7 --- /dev/null +++ b/functions/config_item_properties.sql @@ -0,0 +1,65 @@ +CREATE OR REPLACE FUNCTION update_config_item_properties_for_creator( + p_config_id uuid, + p_creator_type text, + p_created_by uuid, + p_properties jsonb +) RETURNS TABLE(changed boolean, properties jsonb) AS $$ +BEGIN + RETURN QUERY + WITH stamped AS ( + SELECT COALESCE( + jsonb_agg( + prop || jsonb_build_object( + 'creator_type', p_creator_type, + 'created_by', p_created_by::text + ) + ), + '[]'::jsonb + ) AS incoming + FROM jsonb_array_elements(COALESCE(p_properties, '[]'::jsonb)) AS incoming(prop) + ), updated AS ( + UPDATE config_items ci + SET properties = + ( + COALESCE( + ( + SELECT jsonb_agg(prop ORDER BY ord) + FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) + WITH ORDINALITY AS existing(prop, ord) + WHERE ( + prop->>'creator_type' = p_creator_type + AND prop->>'created_by' = p_created_by::text + ) IS NOT TRUE + ), + '[]'::jsonb + ) + || (SELECT incoming FROM stamped) + ) + WHERE ci.id = p_config_id + AND ci.properties IS DISTINCT FROM + ( + COALESCE( + ( + SELECT jsonb_agg(prop ORDER BY ord) + FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) + WITH ORDINALITY AS existing(prop, ord) + WHERE ( + prop->>'creator_type' = p_creator_type + AND prop->>'created_by' = p_created_by::text + ) IS NOT TRUE + ), + '[]'::jsonb + ) + || (SELECT incoming FROM stamped) + ) + RETURNING true AS changed, ci.properties + ) + SELECT updated.changed, updated.properties + FROM updated + UNION ALL + SELECT false AS changed, ci.properties + FROM config_items ci + WHERE ci.id = p_config_id + AND NOT EXISTS (SELECT 1 FROM updated); +END; +$$ LANGUAGE plpgsql; diff --git a/models/clone.go b/models/clone.go index 3773853b3..ac888f45c 100644 --- a/models/clone.go +++ b/models/clone.go @@ -38,7 +38,7 @@ func (config ConfigItem) Clone() ConfigItem { clone.ParentID = clonePtr(config.ParentID) clone.Labels = cloneJSONStringMapPtr(config.Labels) clone.Tags = maps.Clone(config.Tags) - clone.Properties = cloneTypePropertiesPtr(config.Properties) + clone.Properties = cloneConfigItemPropertiesPtr(config.Properties) clone.UpdatedAt = clonePtr(config.UpdatedAt) clone.DeletedAt = clonePtr(config.DeletedAt) clone.configJson = cloneAnyMap(config.configJson) @@ -128,21 +128,28 @@ func cloneModelProperties(in Properties) Properties { return out } -func cloneTypePropertiesPtr(in *types.Properties) *types.Properties { +func cloneConfigItemPropertiesPtr(in *ConfigItemProperties) *ConfigItemProperties { if in == nil { return nil } - out := cloneTypeProperties(*in) + out := cloneConfigItemProperties(*in) return &out } -func cloneTypeProperties(in types.Properties) types.Properties { +func cloneConfigItemProperties(in ConfigItemProperties) ConfigItemProperties { if in == nil { return nil } - out := make(types.Properties, len(in)) + out := make(ConfigItemProperties, len(in)) for i, property := range in { - out[i] = property.DeepCopy() + if property == nil { + continue + } + out[i] = &ConfigItemProperty{ + Property: *property.Property.DeepCopy(), + CreatedBy: property.CreatedBy, + CreatorType: property.CreatorType, + } } return out } diff --git a/models/config.go b/models/config.go index 89cfdd8e9..c73917bcf 100644 --- a/models/config.go +++ b/models/config.go @@ -133,33 +133,33 @@ type ConfigLocation struct { // ConfigItem represents the config item database table type ConfigItem struct { - ID uuid.UUID `json:"id" faker:"uuid_hyphenated" gorm:"default:generate_ulid()"` - ScraperID *string `json:"scraper_id,omitempty"` - AgentID uuid.UUID `json:"agent_id,omitempty"` - ConfigClass string `json:"config_class" faker:"oneof:File,EC2Instance,KubernetesPod" ` - ExternalID pq.StringArray `gorm:"type:[]text" json:"external_id,omitempty"` - Type *string `json:"type"` - Status *string `json:"status" gorm:"default:null"` - Ready bool `json:"ready"` - Health *Health `json:"health"` - Name *string `json:"name,omitempty" faker:"name"` - Description *string `json:"description"` - Config *string `json:"config"` - Source *string `json:"source,omitempty"` - ParentID *uuid.UUID `json:"parent_id,omitempty" faker:"-"` - Path string `json:"path,omitempty" faker:"-"` - CostPerMinute float64 `gorm:"column:cost_per_minute;default:null" json:"cost_per_minute,omitempty"` - CostTotal1d float64 `gorm:"column:cost_total_1d;default:null" json:"cost_total_1d,omitempty"` - CostTotal7d float64 `gorm:"column:cost_total_7d;default:null" json:"cost_total_7d,omitempty"` - CostTotal30d float64 `gorm:"column:cost_total_30d;default:null" json:"cost_total_30d,omitempty"` - Labels *types.JSONStringMap `json:"labels,omitempty" faker:"labels"` - Tags types.JSONStringMap `json:"tags,omitempty" faker:"tags"` - Properties *types.Properties `json:"properties,omitempty"` - CreatedAt time.Time `json:"created_at" gorm:"<-:create"` - InsertedAt time.Time `json:"inserted_at" gorm:"->;default:now()"` - UpdatedAt *time.Time `json:"updated_at" gorm:"autoUpdateTime:false"` - DeletedAt *time.Time `json:"deleted_at,omitempty"` - DeleteReason string `json:"delete_reason,omitempty"` + ID uuid.UUID `json:"id" faker:"uuid_hyphenated" gorm:"default:generate_ulid()"` + ScraperID *string `json:"scraper_id,omitempty"` + AgentID uuid.UUID `json:"agent_id,omitempty"` + ConfigClass string `json:"config_class" faker:"oneof:File,EC2Instance,KubernetesPod" ` + ExternalID pq.StringArray `gorm:"type:[]text" json:"external_id,omitempty"` + Type *string `json:"type"` + Status *string `json:"status" gorm:"default:null"` + Ready bool `json:"ready"` + Health *Health `json:"health"` + Name *string `json:"name,omitempty" faker:"name"` + Description *string `json:"description"` + Config *string `json:"config"` + Source *string `json:"source,omitempty"` + ParentID *uuid.UUID `json:"parent_id,omitempty" faker:"-"` + Path string `json:"path,omitempty" faker:"-"` + CostPerMinute float64 `gorm:"column:cost_per_minute;default:null" json:"cost_per_minute,omitempty"` + CostTotal1d float64 `gorm:"column:cost_total_1d;default:null" json:"cost_total_1d,omitempty"` + CostTotal7d float64 `gorm:"column:cost_total_7d;default:null" json:"cost_total_7d,omitempty"` + CostTotal30d float64 `gorm:"column:cost_total_30d;default:null" json:"cost_total_30d,omitempty"` + Labels *types.JSONStringMap `json:"labels,omitempty" faker:"labels"` + Tags types.JSONStringMap `json:"tags,omitempty" faker:"tags"` + Properties *ConfigItemProperties `json:"properties,omitempty"` + CreatedAt time.Time `json:"created_at" gorm:"<-:create"` + InsertedAt time.Time `json:"inserted_at" gorm:"->;default:now()"` + UpdatedAt *time.Time `json:"updated_at" gorm:"autoUpdateTime:false"` + DeletedAt *time.Time `json:"deleted_at,omitempty"` + DeleteReason string `json:"delete_reason,omitempty"` configJson map[string]any `json:"-" yaml:"-" gorm:"-"` } diff --git a/models/config_item_properties.go b/models/config_item_properties.go new file mode 100644 index 000000000..0616a65ff --- /dev/null +++ b/models/config_item_properties.go @@ -0,0 +1,161 @@ +package models + +import ( + "context" + stdsql "database/sql" + "database/sql/driver" + "encoding/json" + "errors" + "fmt" + + "github.com/flanksource/commons/logger" + "github.com/flanksource/duty/types" + "github.com/google/uuid" + "gorm.io/gorm" + "gorm.io/gorm/clause" + "gorm.io/gorm/schema" +) + +const ( + PropertyCreatorTypeScraper = "scraper" + PropertyCreatorTypePerson = "person" +) + +type ConfigItemProperty struct { + types.Property + + CreatedBy string `json:"created_by,omitempty"` + CreatorType string `json:"creator_type,omitempty"` +} + +type ConfigItemProperties []*ConfigItemProperty + +func NewConfigItemProperties(props types.Properties) ConfigItemProperties { + if props == nil { + return nil + } + + out := make(ConfigItemProperties, len(props)) + for i, prop := range props { + if prop == nil { + continue + } + out[i] = &ConfigItemProperty{Property: *prop.DeepCopy()} + } + return out +} + +func (p ConfigItemProperties) AsProperties() types.Properties { + if p == nil { + return nil + } + + out := make(types.Properties, len(p)) + for i, prop := range p { + if prop == nil { + continue + } + out[i] = prop.Property.DeepCopy() + } + return out +} + +func (m ConfigItemProperties) MarshalJSON() ([]byte, error) { + if m == nil { + return nil, nil + } + t := ([]*ConfigItemProperty)(m) + return json.Marshal(t) +} + +func (m *ConfigItemProperties) UnmarshalJSON(b []byte) error { + t := []*ConfigItemProperty{} + err := json.Unmarshal(b, &t) + *m = ConfigItemProperties(t) + return err +} + +func (p ConfigItemProperties) AsJSON() []byte { + if len(p) == 0 { + return []byte("[]") + } + data, err := json.Marshal(p) + if err != nil { + logger.Errorf("Error marshalling config item properties: %v", err) + } + return data +} + +func (p ConfigItemProperties) Value() (driver.Value, error) { + if len(p) == 0 { + return nil, nil + } + return p.AsJSON(), nil +} + +func (p *ConfigItemProperties) Scan(val interface{}) error { + if val == nil { + *p = make(ConfigItemProperties, 0) + return nil + } + var ba []byte + switch v := val.(type) { + case []byte: + ba = v + default: + return errors.New(fmt.Sprint("Failed to unmarshal config item properties value:", val)) + } + return json.Unmarshal(ba, p) +} + +func (ConfigItemProperties) GormDataType() string { + return "config_item_properties" +} + +func (ConfigItemProperties) GormDBDataType(db *gorm.DB, field *schema.Field) string { + switch db.Dialector.Name() { + case "sqlite": + return "TEXT" + case "postgres": + return "JSONB" + case "sqlserver": + return "NVARCHAR(MAX)" + } + return "" +} + +func (p ConfigItemProperties) GormValue(ctx context.Context, db *gorm.DB) clause.Expr { + data, _ := json.Marshal(p) + return gorm.Expr("?", data) +} + +type UpdateConfigItemPropertiesResult struct { + Changed bool + Properties ConfigItemProperties +} + +func UpdateConfigItemPropertiesForCreator(tx *gorm.DB, configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) (UpdateConfigItemPropertiesResult, error) { + incomingJSON := incoming.AsJSON() + + var result struct { + Changed bool + Properties string + } + if err := tx.Raw(`SELECT changed, properties FROM update_config_item_properties_for_creator(@configID, @creatorType, @createdBy, CAST(@incoming AS jsonb))`, + stdsql.Named("configID", configID), + stdsql.Named("creatorType", creatorType), + stdsql.Named("createdBy", createdBy), + stdsql.Named("incoming", string(incomingJSON)), + ).Scan(&result).Error; err != nil { + return UpdateConfigItemPropertiesResult{}, err + } + + var merged ConfigItemProperties + if result.Properties != "" { + if err := json.Unmarshal([]byte(result.Properties), &merged); err != nil { + return UpdateConfigItemPropertiesResult{}, fmt.Errorf("unmarshal merged properties: %w", err) + } + } + + return UpdateConfigItemPropertiesResult{Changed: result.Changed, Properties: merged}, nil +} diff --git a/query/template_test.go b/query/template_test.go index f9b80d591..52fc92687 100644 --- a/query/template_test.go +++ b/query/template_test.go @@ -43,12 +43,12 @@ func TestMatchQuery(t *testing.T) { "app.kubernetes.io/component": "backend", "deployment.kubernetes.io/revision": "42", }, - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "cpu", Text: "2000m"}, {Name: "memory", Text: "4Gi"}, {Name: "replicas", Value: lo.ToPtr(int64(3))}, {Name: "maxReplicas", Value: lo.ToPtr(int64(10))}, - }, + })), Config: lo.ToPtr(`{ "apiVersion": "apps/v1", "kind": "Deployment", diff --git a/rbac/objects.go b/rbac/objects.go index 29fadd1e6..9aac94452 100644 --- a/rbac/objects.go +++ b/rbac/objects.go @@ -151,6 +151,8 @@ var dbResourceObjMap = map[string]string{ "properties": policy.ObjectDatabaseSystem, "push_queue_summary": policy.ObjectMonitor, "responders": policy.ObjectIncident, + + "rpc/update_config_item_properties_for_creator": policy.ObjectCatalog, "rpc/lookup_component_config_id_related_components": policy.ObjectTopology, "rpc/_related_config_ids_recursive": policy.ObjectCatalog, "rpc/check_summary_for_component": policy.ObjectCanary, diff --git a/tests/config_item_properties_function_test.go b/tests/config_item_properties_function_test.go new file mode 100644 index 000000000..b3650eff5 --- /dev/null +++ b/tests/config_item_properties_function_test.go @@ -0,0 +1,151 @@ +package tests + +import ( + "encoding/json" + "sync" + + "github.com/flanksource/duty/models" + "github.com/flanksource/duty/types" + "github.com/google/uuid" + "github.com/lib/pq" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("update_config_item_properties_for_creator", func() { + It("preserves user, other scraper, and legacy properties", func() { + configID := uuid.New() + scraperA := uuid.New() + scraperB := uuid.New() + person := uuid.New() + seedConfigItemWithProperties(configID, models.ConfigItemProperties{ + {Property: types.Property{Name: "Owner", Text: "Team"}, CreatorType: models.PropertyCreatorTypePerson, CreatedBy: person.String()}, + {Property: types.Property{Name: "URL", Text: "old"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperA.String()}, + {Property: types.Property{Name: "Runbook", Text: "rb"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperB.String()}, + {Property: types.Property{Name: "Legacy", Text: "keep"}}, + }) + + result := callUpdateConfigItemPropertiesForCreator(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{ + {Name: "URL", Text: "new"}, + {Name: "Region", Text: "us-east-1"}, + }) + + Expect(result.Changed).To(BeTrue()) + props := propertyMaps(result.Properties) + Expect(props).To(HaveLen(5)) + Expect(findProperty(props, "Owner")).To(HaveKeyWithValue("created_by", person.String())) + Expect(findProperty(props, "Runbook")).To(HaveKeyWithValue("created_by", scraperB.String())) + Expect(findProperty(props, "Legacy")).To(HaveKeyWithValue("text", "keep")) + Expect(findProperty(props, "URL")).To(SatisfyAll(HaveKeyWithValue("text", "new"), HaveKeyWithValue("created_by", scraperA.String()))) + Expect(findProperty(props, "Region")).To(SatisfyAll(HaveKeyWithValue("text", "us-east-1"), HaveKeyWithValue("created_by", scraperA.String()))) + }) + + It("returns changed=false with merged properties when no update is needed", func() { + configID := uuid.New() + scraper := uuid.New() + seedConfigItemWithProperties(configID, models.ConfigItemProperties{ + {Property: types.Property{Name: "URL", Text: "new"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraper.String()}, + }) + + result := callUpdateConfigItemPropertiesForCreator(configID, models.PropertyCreatorTypeScraper, scraper, types.Properties{{Name: "URL", Text: "new"}}) + + Expect(result.Changed).To(BeFalse()) + Expect(findProperty(propertyMaps(result.Properties), "URL")).To(HaveKeyWithValue("created_by", scraper.String())) + }) + + It("removes creator-owned properties when incoming properties are empty", func() { + configID := uuid.New() + scraperA := uuid.New() + scraperB := uuid.New() + seedConfigItemWithProperties(configID, models.ConfigItemProperties{ + {Property: types.Property{Name: "URL", Text: "old"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperA.String()}, + {Property: types.Property{Name: "Runbook", Text: "rb"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperB.String()}, + {Property: types.Property{Name: "Legacy", Text: "keep"}}, + }) + + result := callUpdateConfigItemPropertiesForCreator(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{}) + + Expect(result.Changed).To(BeTrue()) + props := propertyMaps(result.Properties) + Expect(findProperty(props, "URL")).To(BeNil()) + Expect(findProperty(props, "Runbook")).To(HaveKeyWithValue("created_by", scraperB.String())) + Expect(findProperty(props, "Legacy")).To(HaveKeyWithValue("text", "keep")) + }) + + It("does not clobber concurrent updates from different scrapers", func() { + configID := uuid.New() + scraperA := uuid.New() + scraperB := uuid.New() + seedConfigItemWithProperties(configID, nil) + + var wg sync.WaitGroup + errs := make(chan error, 2) + wg.Add(2) + go func() { + defer wg.Done() + errs <- callUpdateConfigItemPropertiesForCreatorErr(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{{Name: "A", Text: "a"}}) + }() + go func() { + defer wg.Done() + errs <- callUpdateConfigItemPropertiesForCreatorErr(configID, models.PropertyCreatorTypeScraper, scraperB, types.Properties{{Name: "B", Text: "b"}}) + }() + wg.Wait() + close(errs) + for err := range errs { + Expect(err).ToNot(HaveOccurred()) + } + + maps := propertyMaps(getConfigItemProperties(configID)) + Expect(findProperty(maps, "A")).To(HaveKeyWithValue("created_by", scraperA.String())) + Expect(findProperty(maps, "B")).To(HaveKeyWithValue("created_by", scraperB.String())) + }) +}) + +func seedConfigItemWithProperties(id uuid.UUID, properties models.ConfigItemProperties) { + configType := "test" + config := "{}" + Expect(DefaultContext.DB().Create(&models.ConfigItem{ + ID: id, + Type: &configType, + ExternalID: pq.StringArray{id.String()}, + Config: &config, + Properties: &properties, + }).Error).To(Succeed()) +} + +func getConfigItemProperties(configID uuid.UUID) models.ConfigItemProperties { + var propertiesJSON string + Expect(DefaultContext.DB().Raw(`SELECT COALESCE(properties, '[]'::jsonb)::text FROM config_items WHERE id = ?`, configID).Scan(&propertiesJSON).Error).To(Succeed()) + + var props models.ConfigItemProperties + Expect(json.Unmarshal([]byte(propertiesJSON), &props)).To(Succeed()) + return props +} + +func callUpdateConfigItemPropertiesForCreator(configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) models.UpdateConfigItemPropertiesResult { + result, err := models.UpdateConfigItemPropertiesForCreator(DefaultContext.DB(), configID, creatorType, createdBy, incoming) + Expect(err).ToNot(HaveOccurred()) + return result +} + +func callUpdateConfigItemPropertiesForCreatorErr(configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) error { + _, err := models.UpdateConfigItemPropertiesForCreator(DefaultContext.DB(), configID, creatorType, createdBy, incoming) + return err +} + +func propertyMaps(props models.ConfigItemProperties) []map[string]any { + data, err := json.Marshal(props) + Expect(err).ToNot(HaveOccurred()) + var result []map[string]any + Expect(json.Unmarshal(data, &result)).To(Succeed()) + return result +} + +func findProperty(props []map[string]any, name string) map[string]any { + for _, prop := range props { + if prop["name"] == name { + return prop + } + } + return nil +} diff --git a/tests/fixtures/dummy/config.go b/tests/fixtures/dummy/config.go index 9a5596ac8..7f42da42e 100644 --- a/tests/fixtures/dummy/config.go +++ b/tests/fixtures/dummy/config.go @@ -127,10 +127,10 @@ var KubernetesNodeA = models.ConfigItem{ "role": "worker", "region": "us-east-1", }), - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(64))}, {Name: "region", Text: "us-east-1"}, - }, + })), CostTotal30d: 50, } @@ -155,11 +155,11 @@ var KubernetesNodeB = models.ConfigItem{ "region": "us-west-2", "storageprofile": "managed", }), - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(32))}, {Name: "region", Text: "us-west-2"}, {Name: "os", Text: "linux"}, - }, + })), CostTotal30d: 80, } diff --git a/tests/fixtures/dummy/config_demo_cluster.go b/tests/fixtures/dummy/config_demo_cluster.go index ecb7744e4..5164376c1 100644 --- a/tests/fixtures/dummy/config_demo_cluster.go +++ b/tests/fixtures/dummy/config_demo_cluster.go @@ -78,9 +78,9 @@ var KubernetesNodeAKSPool1 = models.ConfigItem{ "cluster": "demo", "subscription": "018fbd67-bb86-90e1-07c9-243eedc73892", }), - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(64))}, - }, + })), CostTotal30d: 100, } diff --git a/types/resource_selector_test.go b/types/resource_selector_test.go index 2b0b68f5f..77a9ceb4b 100644 --- a/types/resource_selector_test.go +++ b/types/resource_selector_test.go @@ -438,14 +438,14 @@ var _ = Describe("Resource Selector", func() { {FieldSelector: "properties.color=red"}, }, selectable: models.ConfigItem{ - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "color", Text: "red"}, - }, + })), }, unselectable: models.ConfigItem{ - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "color", Text: "green"}, - }, + })), }, }, { @@ -454,14 +454,14 @@ var _ = Describe("Resource Selector", func() { {FieldSelector: "properties.memory>50"}, }, selectable: models.ConfigItem{ - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(64))}, - }, + })), }, unselectable: models.ConfigItem{ - Properties: &types.Properties{ + Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(32))}, - }, + })), }, }, { From 84872bfd24899ef102def4734301719b3cead03f Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 28 Apr 2026 21:16:57 +0545 Subject: [PATCH 2/7] chore: Owned Properties --- functions/config_item_properties.sql | 6 ++- models/clone.go | 12 ++--- models/config.go | 54 +++++++++---------- ...g_item_properties.go => owned_property.go} | 49 +++++++++-------- query/template_test.go | 2 +- rbac/objects.go | 2 +- tests/config_item_properties_function_test.go | 36 ++++++------- tests/fixtures/dummy/config.go | 4 +- tests/fixtures/dummy/config_demo_cluster.go | 2 +- types/resource_selector_test.go | 8 +-- 10 files changed, 92 insertions(+), 83 deletions(-) rename models/{config_item_properties.go => owned_property.go} (60%) diff --git a/functions/config_item_properties.sql b/functions/config_item_properties.sql index 091999cd7..1022c6082 100644 --- a/functions/config_item_properties.sql +++ b/functions/config_item_properties.sql @@ -1,4 +1,8 @@ -CREATE OR REPLACE FUNCTION update_config_item_properties_for_creator( +-- Replaces the config item's properties owned by (p_creator_type, p_created_by) +-- with p_properties stamped with that ownership. Properties owned by other +-- creators, and legacy properties without ownership metadata, are preserved; +-- passing an empty/null p_properties removes this creator's owned properties. +CREATE OR REPLACE FUNCTION update_config_item_properties( p_config_id uuid, p_creator_type text, p_created_by uuid, diff --git a/models/clone.go b/models/clone.go index ac888f45c..f6911776d 100644 --- a/models/clone.go +++ b/models/clone.go @@ -38,7 +38,7 @@ func (config ConfigItem) Clone() ConfigItem { clone.ParentID = clonePtr(config.ParentID) clone.Labels = cloneJSONStringMapPtr(config.Labels) clone.Tags = maps.Clone(config.Tags) - clone.Properties = cloneConfigItemPropertiesPtr(config.Properties) + clone.Properties = cloneOwnedPropertiesPtr(config.Properties) clone.UpdatedAt = clonePtr(config.UpdatedAt) clone.DeletedAt = clonePtr(config.DeletedAt) clone.configJson = cloneAnyMap(config.configJson) @@ -128,24 +128,24 @@ func cloneModelProperties(in Properties) Properties { return out } -func cloneConfigItemPropertiesPtr(in *ConfigItemProperties) *ConfigItemProperties { +func cloneOwnedPropertiesPtr(in *OwnedProperties) *OwnedProperties { if in == nil { return nil } - out := cloneConfigItemProperties(*in) + out := cloneOwnedProperties(*in) return &out } -func cloneConfigItemProperties(in ConfigItemProperties) ConfigItemProperties { +func cloneOwnedProperties(in OwnedProperties) OwnedProperties { if in == nil { return nil } - out := make(ConfigItemProperties, len(in)) + out := make(OwnedProperties, len(in)) for i, property := range in { if property == nil { continue } - out[i] = &ConfigItemProperty{ + out[i] = &OwnedProperty{ Property: *property.Property.DeepCopy(), CreatedBy: property.CreatedBy, CreatorType: property.CreatorType, diff --git a/models/config.go b/models/config.go index c73917bcf..4af105024 100644 --- a/models/config.go +++ b/models/config.go @@ -133,33 +133,33 @@ type ConfigLocation struct { // ConfigItem represents the config item database table type ConfigItem struct { - ID uuid.UUID `json:"id" faker:"uuid_hyphenated" gorm:"default:generate_ulid()"` - ScraperID *string `json:"scraper_id,omitempty"` - AgentID uuid.UUID `json:"agent_id,omitempty"` - ConfigClass string `json:"config_class" faker:"oneof:File,EC2Instance,KubernetesPod" ` - ExternalID pq.StringArray `gorm:"type:[]text" json:"external_id,omitempty"` - Type *string `json:"type"` - Status *string `json:"status" gorm:"default:null"` - Ready bool `json:"ready"` - Health *Health `json:"health"` - Name *string `json:"name,omitempty" faker:"name"` - Description *string `json:"description"` - Config *string `json:"config"` - Source *string `json:"source,omitempty"` - ParentID *uuid.UUID `json:"parent_id,omitempty" faker:"-"` - Path string `json:"path,omitempty" faker:"-"` - CostPerMinute float64 `gorm:"column:cost_per_minute;default:null" json:"cost_per_minute,omitempty"` - CostTotal1d float64 `gorm:"column:cost_total_1d;default:null" json:"cost_total_1d,omitempty"` - CostTotal7d float64 `gorm:"column:cost_total_7d;default:null" json:"cost_total_7d,omitempty"` - CostTotal30d float64 `gorm:"column:cost_total_30d;default:null" json:"cost_total_30d,omitempty"` - Labels *types.JSONStringMap `json:"labels,omitempty" faker:"labels"` - Tags types.JSONStringMap `json:"tags,omitempty" faker:"tags"` - Properties *ConfigItemProperties `json:"properties,omitempty"` - CreatedAt time.Time `json:"created_at" gorm:"<-:create"` - InsertedAt time.Time `json:"inserted_at" gorm:"->;default:now()"` - UpdatedAt *time.Time `json:"updated_at" gorm:"autoUpdateTime:false"` - DeletedAt *time.Time `json:"deleted_at,omitempty"` - DeleteReason string `json:"delete_reason,omitempty"` + ID uuid.UUID `json:"id" faker:"uuid_hyphenated" gorm:"default:generate_ulid()"` + ScraperID *string `json:"scraper_id,omitempty"` + AgentID uuid.UUID `json:"agent_id,omitempty"` + ConfigClass string `json:"config_class" faker:"oneof:File,EC2Instance,KubernetesPod" ` + ExternalID pq.StringArray `gorm:"type:[]text" json:"external_id,omitempty"` + Type *string `json:"type"` + Status *string `json:"status" gorm:"default:null"` + Ready bool `json:"ready"` + Health *Health `json:"health"` + Name *string `json:"name,omitempty" faker:"name"` + Description *string `json:"description"` + Config *string `json:"config"` + Source *string `json:"source,omitempty"` + ParentID *uuid.UUID `json:"parent_id,omitempty" faker:"-"` + Path string `json:"path,omitempty" faker:"-"` + CostPerMinute float64 `gorm:"column:cost_per_minute;default:null" json:"cost_per_minute,omitempty"` + CostTotal1d float64 `gorm:"column:cost_total_1d;default:null" json:"cost_total_1d,omitempty"` + CostTotal7d float64 `gorm:"column:cost_total_7d;default:null" json:"cost_total_7d,omitempty"` + CostTotal30d float64 `gorm:"column:cost_total_30d;default:null" json:"cost_total_30d,omitempty"` + Labels *types.JSONStringMap `json:"labels,omitempty" faker:"labels"` + Tags types.JSONStringMap `json:"tags,omitempty" faker:"tags"` + Properties *OwnedProperties `json:"properties,omitempty"` + CreatedAt time.Time `json:"created_at" gorm:"<-:create"` + InsertedAt time.Time `json:"inserted_at" gorm:"->;default:now()"` + UpdatedAt *time.Time `json:"updated_at" gorm:"autoUpdateTime:false"` + DeletedAt *time.Time `json:"deleted_at,omitempty"` + DeleteReason string `json:"delete_reason,omitempty"` configJson map[string]any `json:"-" yaml:"-" gorm:"-"` } diff --git a/models/config_item_properties.go b/models/owned_property.go similarity index 60% rename from models/config_item_properties.go rename to models/owned_property.go index 0616a65ff..b61bc167f 100644 --- a/models/config_item_properties.go +++ b/models/owned_property.go @@ -21,31 +21,31 @@ const ( PropertyCreatorTypePerson = "person" ) -type ConfigItemProperty struct { +type OwnedProperty struct { types.Property CreatedBy string `json:"created_by,omitempty"` CreatorType string `json:"creator_type,omitempty"` } -type ConfigItemProperties []*ConfigItemProperty +type OwnedProperties []*OwnedProperty -func NewConfigItemProperties(props types.Properties) ConfigItemProperties { +func NewOwnedProperties(props types.Properties) OwnedProperties { if props == nil { return nil } - out := make(ConfigItemProperties, len(props)) + out := make(OwnedProperties, len(props)) for i, prop := range props { if prop == nil { continue } - out[i] = &ConfigItemProperty{Property: *prop.DeepCopy()} + out[i] = &OwnedProperty{Property: *prop.DeepCopy()} } return out } -func (p ConfigItemProperties) AsProperties() types.Properties { +func (p OwnedProperties) AsProperties() types.Properties { if p == nil { return nil } @@ -60,22 +60,22 @@ func (p ConfigItemProperties) AsProperties() types.Properties { return out } -func (m ConfigItemProperties) MarshalJSON() ([]byte, error) { +func (m OwnedProperties) MarshalJSON() ([]byte, error) { if m == nil { return nil, nil } - t := ([]*ConfigItemProperty)(m) + t := ([]*OwnedProperty)(m) return json.Marshal(t) } -func (m *ConfigItemProperties) UnmarshalJSON(b []byte) error { - t := []*ConfigItemProperty{} +func (m *OwnedProperties) UnmarshalJSON(b []byte) error { + t := []*OwnedProperty{} err := json.Unmarshal(b, &t) - *m = ConfigItemProperties(t) + *m = OwnedProperties(t) return err } -func (p ConfigItemProperties) AsJSON() []byte { +func (p OwnedProperties) AsJSON() []byte { if len(p) == 0 { return []byte("[]") } @@ -86,16 +86,16 @@ func (p ConfigItemProperties) AsJSON() []byte { return data } -func (p ConfigItemProperties) Value() (driver.Value, error) { +func (p OwnedProperties) Value() (driver.Value, error) { if len(p) == 0 { return nil, nil } return p.AsJSON(), nil } -func (p *ConfigItemProperties) Scan(val interface{}) error { +func (p *OwnedProperties) Scan(val interface{}) error { if val == nil { - *p = make(ConfigItemProperties, 0) + *p = make(OwnedProperties, 0) return nil } var ba []byte @@ -108,11 +108,11 @@ func (p *ConfigItemProperties) Scan(val interface{}) error { return json.Unmarshal(ba, p) } -func (ConfigItemProperties) GormDataType() string { +func (OwnedProperties) GormDataType() string { return "config_item_properties" } -func (ConfigItemProperties) GormDBDataType(db *gorm.DB, field *schema.Field) string { +func (OwnedProperties) GormDBDataType(db *gorm.DB, field *schema.Field) string { switch db.Dialector.Name() { case "sqlite": return "TEXT" @@ -124,24 +124,29 @@ func (ConfigItemProperties) GormDBDataType(db *gorm.DB, field *schema.Field) str return "" } -func (p ConfigItemProperties) GormValue(ctx context.Context, db *gorm.DB) clause.Expr { +func (p OwnedProperties) GormValue(ctx context.Context, db *gorm.DB) clause.Expr { data, _ := json.Marshal(p) return gorm.Expr("?", data) } type UpdateConfigItemPropertiesResult struct { Changed bool - Properties ConfigItemProperties + Properties OwnedProperties } -func UpdateConfigItemPropertiesForCreator(tx *gorm.DB, configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) (UpdateConfigItemPropertiesResult, error) { +// UpdateConfigItemProperties replaces only the properties owned by the given +// creator on a config item. Existing properties from other creators, and legacy +// properties without ownership metadata, are preserved; incoming properties are +// stamped with creator_type/created_by before being merged. Passing an empty +// incoming list removes that creator's owned properties. +func UpdateConfigItemProperties(tx *gorm.DB, configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) (UpdateConfigItemPropertiesResult, error) { incomingJSON := incoming.AsJSON() var result struct { Changed bool Properties string } - if err := tx.Raw(`SELECT changed, properties FROM update_config_item_properties_for_creator(@configID, @creatorType, @createdBy, CAST(@incoming AS jsonb))`, + if err := tx.Raw(`SELECT changed, properties FROM update_config_item_properties(@configID, @creatorType, @createdBy, CAST(@incoming AS jsonb))`, stdsql.Named("configID", configID), stdsql.Named("creatorType", creatorType), stdsql.Named("createdBy", createdBy), @@ -150,7 +155,7 @@ func UpdateConfigItemPropertiesForCreator(tx *gorm.DB, configID uuid.UUID, creat return UpdateConfigItemPropertiesResult{}, err } - var merged ConfigItemProperties + var merged OwnedProperties if result.Properties != "" { if err := json.Unmarshal([]byte(result.Properties), &merged); err != nil { return UpdateConfigItemPropertiesResult{}, fmt.Errorf("unmarshal merged properties: %w", err) diff --git a/query/template_test.go b/query/template_test.go index 52fc92687..1cd05fd2f 100644 --- a/query/template_test.go +++ b/query/template_test.go @@ -43,7 +43,7 @@ func TestMatchQuery(t *testing.T) { "app.kubernetes.io/component": "backend", "deployment.kubernetes.io/revision": "42", }, - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "cpu", Text: "2000m"}, {Name: "memory", Text: "4Gi"}, {Name: "replicas", Value: lo.ToPtr(int64(3))}, diff --git a/rbac/objects.go b/rbac/objects.go index 9aac94452..595dfe3ee 100644 --- a/rbac/objects.go +++ b/rbac/objects.go @@ -152,7 +152,7 @@ var dbResourceObjMap = map[string]string{ "push_queue_summary": policy.ObjectMonitor, "responders": policy.ObjectIncident, - "rpc/update_config_item_properties_for_creator": policy.ObjectCatalog, + "rpc/update_config_item_properties": policy.ObjectCatalog, "rpc/lookup_component_config_id_related_components": policy.ObjectTopology, "rpc/_related_config_ids_recursive": policy.ObjectCatalog, "rpc/check_summary_for_component": policy.ObjectCanary, diff --git a/tests/config_item_properties_function_test.go b/tests/config_item_properties_function_test.go index b3650eff5..36a7da6a4 100644 --- a/tests/config_item_properties_function_test.go +++ b/tests/config_item_properties_function_test.go @@ -12,20 +12,20 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("update_config_item_properties_for_creator", func() { +var _ = Describe("update_config_item_properties", func() { It("preserves user, other scraper, and legacy properties", func() { configID := uuid.New() scraperA := uuid.New() scraperB := uuid.New() person := uuid.New() - seedConfigItemWithProperties(configID, models.ConfigItemProperties{ + seedConfigItemWithProperties(configID, models.OwnedProperties{ {Property: types.Property{Name: "Owner", Text: "Team"}, CreatorType: models.PropertyCreatorTypePerson, CreatedBy: person.String()}, {Property: types.Property{Name: "URL", Text: "old"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperA.String()}, {Property: types.Property{Name: "Runbook", Text: "rb"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperB.String()}, {Property: types.Property{Name: "Legacy", Text: "keep"}}, }) - result := callUpdateConfigItemPropertiesForCreator(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{ + result := callUpdateConfigItemProperties(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{ {Name: "URL", Text: "new"}, {Name: "Region", Text: "us-east-1"}, }) @@ -43,11 +43,11 @@ var _ = Describe("update_config_item_properties_for_creator", func() { It("returns changed=false with merged properties when no update is needed", func() { configID := uuid.New() scraper := uuid.New() - seedConfigItemWithProperties(configID, models.ConfigItemProperties{ + seedConfigItemWithProperties(configID, models.OwnedProperties{ {Property: types.Property{Name: "URL", Text: "new"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraper.String()}, }) - result := callUpdateConfigItemPropertiesForCreator(configID, models.PropertyCreatorTypeScraper, scraper, types.Properties{{Name: "URL", Text: "new"}}) + result := callUpdateConfigItemProperties(configID, models.PropertyCreatorTypeScraper, scraper, types.Properties{{Name: "URL", Text: "new"}}) Expect(result.Changed).To(BeFalse()) Expect(findProperty(propertyMaps(result.Properties), "URL")).To(HaveKeyWithValue("created_by", scraper.String())) @@ -57,13 +57,13 @@ var _ = Describe("update_config_item_properties_for_creator", func() { configID := uuid.New() scraperA := uuid.New() scraperB := uuid.New() - seedConfigItemWithProperties(configID, models.ConfigItemProperties{ + seedConfigItemWithProperties(configID, models.OwnedProperties{ {Property: types.Property{Name: "URL", Text: "old"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperA.String()}, {Property: types.Property{Name: "Runbook", Text: "rb"}, CreatorType: models.PropertyCreatorTypeScraper, CreatedBy: scraperB.String()}, {Property: types.Property{Name: "Legacy", Text: "keep"}}, }) - result := callUpdateConfigItemPropertiesForCreator(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{}) + result := callUpdateConfigItemProperties(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{}) Expect(result.Changed).To(BeTrue()) props := propertyMaps(result.Properties) @@ -83,11 +83,11 @@ var _ = Describe("update_config_item_properties_for_creator", func() { wg.Add(2) go func() { defer wg.Done() - errs <- callUpdateConfigItemPropertiesForCreatorErr(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{{Name: "A", Text: "a"}}) + errs <- callUpdateConfigItemPropertiesErr(configID, models.PropertyCreatorTypeScraper, scraperA, types.Properties{{Name: "A", Text: "a"}}) }() go func() { defer wg.Done() - errs <- callUpdateConfigItemPropertiesForCreatorErr(configID, models.PropertyCreatorTypeScraper, scraperB, types.Properties{{Name: "B", Text: "b"}}) + errs <- callUpdateConfigItemPropertiesErr(configID, models.PropertyCreatorTypeScraper, scraperB, types.Properties{{Name: "B", Text: "b"}}) }() wg.Wait() close(errs) @@ -95,13 +95,13 @@ var _ = Describe("update_config_item_properties_for_creator", func() { Expect(err).ToNot(HaveOccurred()) } - maps := propertyMaps(getConfigItemProperties(configID)) + maps := propertyMaps(getOwnedProperties(configID)) Expect(findProperty(maps, "A")).To(HaveKeyWithValue("created_by", scraperA.String())) Expect(findProperty(maps, "B")).To(HaveKeyWithValue("created_by", scraperB.String())) }) }) -func seedConfigItemWithProperties(id uuid.UUID, properties models.ConfigItemProperties) { +func seedConfigItemWithProperties(id uuid.UUID, properties models.OwnedProperties) { configType := "test" config := "{}" Expect(DefaultContext.DB().Create(&models.ConfigItem{ @@ -113,27 +113,27 @@ func seedConfigItemWithProperties(id uuid.UUID, properties models.ConfigItemProp }).Error).To(Succeed()) } -func getConfigItemProperties(configID uuid.UUID) models.ConfigItemProperties { +func getOwnedProperties(configID uuid.UUID) models.OwnedProperties { var propertiesJSON string Expect(DefaultContext.DB().Raw(`SELECT COALESCE(properties, '[]'::jsonb)::text FROM config_items WHERE id = ?`, configID).Scan(&propertiesJSON).Error).To(Succeed()) - var props models.ConfigItemProperties + var props models.OwnedProperties Expect(json.Unmarshal([]byte(propertiesJSON), &props)).To(Succeed()) return props } -func callUpdateConfigItemPropertiesForCreator(configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) models.UpdateConfigItemPropertiesResult { - result, err := models.UpdateConfigItemPropertiesForCreator(DefaultContext.DB(), configID, creatorType, createdBy, incoming) +func callUpdateConfigItemProperties(configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) models.UpdateConfigItemPropertiesResult { + result, err := models.UpdateConfigItemProperties(DefaultContext.DB(), configID, creatorType, createdBy, incoming) Expect(err).ToNot(HaveOccurred()) return result } -func callUpdateConfigItemPropertiesForCreatorErr(configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) error { - _, err := models.UpdateConfigItemPropertiesForCreator(DefaultContext.DB(), configID, creatorType, createdBy, incoming) +func callUpdateConfigItemPropertiesErr(configID uuid.UUID, creatorType string, createdBy uuid.UUID, incoming types.Properties) error { + _, err := models.UpdateConfigItemProperties(DefaultContext.DB(), configID, creatorType, createdBy, incoming) return err } -func propertyMaps(props models.ConfigItemProperties) []map[string]any { +func propertyMaps(props models.OwnedProperties) []map[string]any { data, err := json.Marshal(props) Expect(err).ToNot(HaveOccurred()) var result []map[string]any diff --git a/tests/fixtures/dummy/config.go b/tests/fixtures/dummy/config.go index 7f42da42e..0ae07a56b 100644 --- a/tests/fixtures/dummy/config.go +++ b/tests/fixtures/dummy/config.go @@ -127,7 +127,7 @@ var KubernetesNodeA = models.ConfigItem{ "role": "worker", "region": "us-east-1", }), - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(64))}, {Name: "region", Text: "us-east-1"}, })), @@ -155,7 +155,7 @@ var KubernetesNodeB = models.ConfigItem{ "region": "us-west-2", "storageprofile": "managed", }), - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(32))}, {Name: "region", Text: "us-west-2"}, {Name: "os", Text: "linux"}, diff --git a/tests/fixtures/dummy/config_demo_cluster.go b/tests/fixtures/dummy/config_demo_cluster.go index 5164376c1..03508a3a9 100644 --- a/tests/fixtures/dummy/config_demo_cluster.go +++ b/tests/fixtures/dummy/config_demo_cluster.go @@ -78,7 +78,7 @@ var KubernetesNodeAKSPool1 = models.ConfigItem{ "cluster": "demo", "subscription": "018fbd67-bb86-90e1-07c9-243eedc73892", }), - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(64))}, })), CostTotal30d: 100, diff --git a/types/resource_selector_test.go b/types/resource_selector_test.go index 77a9ceb4b..35344680e 100644 --- a/types/resource_selector_test.go +++ b/types/resource_selector_test.go @@ -438,12 +438,12 @@ var _ = Describe("Resource Selector", func() { {FieldSelector: "properties.color=red"}, }, selectable: models.ConfigItem{ - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "color", Text: "red"}, })), }, unselectable: models.ConfigItem{ - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "color", Text: "green"}, })), }, @@ -454,12 +454,12 @@ var _ = Describe("Resource Selector", func() { {FieldSelector: "properties.memory>50"}, }, selectable: models.ConfigItem{ - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(64))}, })), }, unselectable: models.ConfigItem{ - Properties: lo.ToPtr(models.NewConfigItemProperties(types.Properties{ + Properties: lo.ToPtr(models.NewOwnedProperties(types.Properties{ {Name: "memory", Value: lo.ToPtr(int64(32))}, })), }, From 88d845fd67503e36b72b01807010787821276643 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 28 Apr 2026 21:36:00 +0545 Subject: [PATCH 3/7] chore: document --- functions/config_item_properties.sql | 168 ++++++++++++++++++++------- 1 file changed, 126 insertions(+), 42 deletions(-) diff --git a/functions/config_item_properties.sql b/functions/config_item_properties.sql index 1022c6082..8f01ec2a7 100644 --- a/functions/config_item_properties.sql +++ b/functions/config_item_properties.sql @@ -1,7 +1,100 @@ --- Replaces the config item's properties owned by (p_creator_type, p_created_by) --- with p_properties stamped with that ownership. Properties owned by other --- creators, and legacy properties without ownership metadata, are preserved; --- passing an empty/null p_properties removes this creator's owned properties. +-- update_config_item_properties replaces the subset of a config item's +-- properties owned by one creator. +-- +-- Ownership model +-- ---------------- +-- Each property can be owned by a creator using the metadata fields: +-- +-- { +-- "creator_type": "...", +-- "created_by": "..." +-- } +-- +-- This function treats (p_creator_type, p_created_by) as the ownership key for +-- the incoming update. It removes existing properties owned by that exact key, +-- preserves everything else, then appends the incoming properties stamped with +-- that same ownership key. +-- +-- Preserved properties +-- -------------------- +-- The following existing properties are intentionally preserved: +-- +-- 1. Properties owned by another creator_type / created_by pair. +-- 2. Legacy properties that do not have creator_type / created_by metadata. +-- 3. Properties whose ownership metadata is incomplete or does not exactly +-- match the incoming ownership key. +-- +-- Empty replacement +-- ----------------- +-- Passing NULL or [] as p_properties removes this creator's currently-owned +-- properties and appends nothing. Properties owned by others and legacy +-- properties remain untouched. +-- +-- Return value +-- ------------ +-- Returns one row for the requested config item when it exists: +-- +-- changed true if the row was actually updated, false if the computed +-- properties were identical to the current properties. +-- properties the final/current properties array after the function runs. +-- +-- If p_config_id does not match a config_items row, the function returns no +-- rows. +-- +-- Why the CTEs exist +-- ------------------ +-- stamped: +-- Converts p_properties into an array where every property has creator_type +-- and created_by set to the incoming ownership key. +-- +-- computed: +-- Builds the exact final properties array once: +-- +-- existing properties not owned by this creator +-- || +-- incoming stamped properties +-- +-- updated: +-- Performs the UPDATE only when the computed value is actually distinct from +-- the current value. This avoids no-op updates, extra row versions, triggers, +-- and misleading changed=true results. +-- +-- Example +-- ------- +-- Given existing properties: +-- +-- [ +-- {"name": "legacy"}, +-- {"name": "cpu", "value": "old", "creator_type": "scraper", "created_by": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}, +-- {"name": "region", "value": "us-east", "creator_type": "manual", "created_by": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"} +-- ] +-- +-- Calling: +-- +-- SELECT * +-- FROM update_config_item_properties( +-- '', +-- 'scraper', +-- 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', +-- '[{"name": "cpu", "value": "new"}, {"name": "memory", "value": "high"}]'::jsonb +-- ); +-- +-- Produces final properties like: +-- +-- [ +-- {"name": "legacy"}, +-- {"name": "region", "value": "us-east", "creator_type": "manual", "created_by": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"}, +-- {"name": "cpu", "value": "new", "creator_type": "scraper", "created_by": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"}, +-- {"name": "memory", "value": "high", "creator_type": "scraper", "created_by": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} +-- ] +-- +-- Notes +-- ----- +-- Existing preserved properties keep their original relative order. Incoming +-- properties are appended after preserved properties. +-- +-- p_properties is expected to be a JSON array. Passing a JSON object, string, +-- number, or boolean will raise an error from jsonb_array_elements(). CREATE OR REPLACE FUNCTION update_config_item_properties( p_config_id uuid, p_creator_type text, @@ -17,53 +110,44 @@ BEGIN 'creator_type', p_creator_type, 'created_by', p_created_by::text ) + ORDER BY ord ), '[]'::jsonb ) AS incoming - FROM jsonb_array_elements(COALESCE(p_properties, '[]'::jsonb)) AS incoming(prop) + FROM jsonb_array_elements(COALESCE(p_properties, '[]'::jsonb)) + WITH ORDINALITY AS incoming(prop, ord) + ), computed AS ( + SELECT + ci.id, + ci.properties AS current_properties, + COALESCE( + ( + SELECT jsonb_agg(prop ORDER BY ord) + FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) + WITH ORDINALITY AS existing(prop, ord) + WHERE ( + prop->>'creator_type' = p_creator_type + AND prop->>'created_by' = p_created_by::text + ) IS NOT TRUE + ), + '[]'::jsonb + ) || stamped.incoming AS new_properties + FROM config_items ci + CROSS JOIN stamped + WHERE ci.id = p_config_id ), updated AS ( UPDATE config_items ci - SET properties = - ( - COALESCE( - ( - SELECT jsonb_agg(prop ORDER BY ord) - FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) - WITH ORDINALITY AS existing(prop, ord) - WHERE ( - prop->>'creator_type' = p_creator_type - AND prop->>'created_by' = p_created_by::text - ) IS NOT TRUE - ), - '[]'::jsonb - ) - || (SELECT incoming FROM stamped) - ) - WHERE ci.id = p_config_id - AND ci.properties IS DISTINCT FROM - ( - COALESCE( - ( - SELECT jsonb_agg(prop ORDER BY ord) - FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) - WITH ORDINALITY AS existing(prop, ord) - WHERE ( - prop->>'creator_type' = p_creator_type - AND prop->>'created_by' = p_created_by::text - ) IS NOT TRUE - ), - '[]'::jsonb - ) - || (SELECT incoming FROM stamped) - ) + SET properties = computed.new_properties + FROM computed + WHERE ci.id = computed.id + AND computed.current_properties IS DISTINCT FROM computed.new_properties RETURNING true AS changed, ci.properties ) SELECT updated.changed, updated.properties FROM updated UNION ALL - SELECT false AS changed, ci.properties - FROM config_items ci - WHERE ci.id = p_config_id - AND NOT EXISTS (SELECT 1 FROM updated); + SELECT false AS changed, computed.current_properties AS properties + FROM computed + WHERE NOT EXISTS (SELECT 1 FROM updated); END; $$ LANGUAGE plpgsql; From 3a444bdd9eaa57fc8e374d00d636f2b3b0f4ceec Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 28 Apr 2026 21:39:32 +0545 Subject: [PATCH 4/7] fix(config): report missing config item property updates UpdateConfigItemProperties previously used Raw().Scan() without checking whether the SQL function returned any rows. When the config item ID did not exist, GORM left the result at its zero value and returned nil, making the missing item look like an unchanged update. Capture the scan result, return scan errors as before, and return an explicit not-found error when RowsAffected is zero. Add coverage for the missing config item case. --- models/owned_property.go | 10 +++++++--- tests/config_item_properties_function_test.go | 9 +++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/models/owned_property.go b/models/owned_property.go index b61bc167f..70e67dbda 100644 --- a/models/owned_property.go +++ b/models/owned_property.go @@ -146,13 +146,17 @@ func UpdateConfigItemProperties(tx *gorm.DB, configID uuid.UUID, creatorType str Changed bool Properties string } - if err := tx.Raw(`SELECT changed, properties FROM update_config_item_properties(@configID, @creatorType, @createdBy, CAST(@incoming AS jsonb))`, + q := tx.Raw(`SELECT changed, properties FROM update_config_item_properties(@configID, @creatorType, @createdBy, CAST(@incoming AS jsonb))`, stdsql.Named("configID", configID), stdsql.Named("creatorType", creatorType), stdsql.Named("createdBy", createdBy), stdsql.Named("incoming", string(incomingJSON)), - ).Scan(&result).Error; err != nil { - return UpdateConfigItemPropertiesResult{}, err + ).Scan(&result) + if q.Error != nil { + return UpdateConfigItemPropertiesResult{}, q.Error + } + if q.RowsAffected == 0 { + return UpdateConfigItemPropertiesResult{}, fmt.Errorf("config item not found: %s", configID) } var merged OwnedProperties diff --git a/tests/config_item_properties_function_test.go b/tests/config_item_properties_function_test.go index 36a7da6a4..c4bd122fa 100644 --- a/tests/config_item_properties_function_test.go +++ b/tests/config_item_properties_function_test.go @@ -53,6 +53,15 @@ var _ = Describe("update_config_item_properties", func() { Expect(findProperty(propertyMaps(result.Properties), "URL")).To(HaveKeyWithValue("created_by", scraper.String())) }) + It("returns an error when the config item does not exist", func() { + missingID := uuid.New() + scraper := uuid.New() + + err := callUpdateConfigItemPropertiesErr(missingID, models.PropertyCreatorTypeScraper, scraper, types.Properties{{Name: "URL", Text: "new"}}) + + Expect(err).To(MatchError(ContainSubstring("config item not found: " + missingID.String()))) + }) + It("removes creator-owned properties when incoming properties are empty", func() { configID := uuid.New() scraperA := uuid.New() From a619f6658008021be6d2c0cb4777299d1d5a6e43 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 28 Apr 2026 21:51:45 +0545 Subject: [PATCH 5/7] fix(config): serialize property ownership updates Concurrent updates for different property creators could derive their replacement JSON from the same stale config_items.properties snapshot. The later writer could then overwrite the earlier creator's slice, breaking the preservation guarantee. Lock the target config_items row before computing new_properties so concurrent calls serialize on the latest row state. Also allow OwnedProperties.Scan to accept string values returned by text-backed drivers such as sqlite or sqlserver. --- functions/config_item_properties.sql | 16 +++++++++++----- models/owned_property.go | 2 ++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/functions/config_item_properties.sql b/functions/config_item_properties.sql index 8f01ec2a7..f3754c632 100644 --- a/functions/config_item_properties.sql +++ b/functions/config_item_properties.sql @@ -116,14 +116,21 @@ BEGIN ) AS incoming FROM jsonb_array_elements(COALESCE(p_properties, '[]'::jsonb)) WITH ORDINALITY AS incoming(prop, ord) - ), computed AS ( + ), locked AS ( SELECT ci.id, - ci.properties AS current_properties, + COALESCE(ci.properties, '[]'::jsonb) AS current_properties + FROM config_items ci + WHERE ci.id = p_config_id + FOR UPDATE + ), computed AS ( + SELECT + locked.id, + locked.current_properties, COALESCE( ( SELECT jsonb_agg(prop ORDER BY ord) - FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) + FROM jsonb_array_elements(locked.current_properties) WITH ORDINALITY AS existing(prop, ord) WHERE ( prop->>'creator_type' = p_creator_type @@ -132,9 +139,8 @@ BEGIN ), '[]'::jsonb ) || stamped.incoming AS new_properties - FROM config_items ci + FROM locked CROSS JOIN stamped - WHERE ci.id = p_config_id ), updated AS ( UPDATE config_items ci SET properties = computed.new_properties diff --git a/models/owned_property.go b/models/owned_property.go index 70e67dbda..d461aac2b 100644 --- a/models/owned_property.go +++ b/models/owned_property.go @@ -102,6 +102,8 @@ func (p *OwnedProperties) Scan(val interface{}) error { switch v := val.(type) { case []byte: ba = v + case string: + ba = []byte(v) default: return errors.New(fmt.Sprint("Failed to unmarshal config item properties value:", val)) } From f75d3ab2384c551d23d3f913e32b906ee9d8c338 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 28 Apr 2026 22:22:19 +0545 Subject: [PATCH 6/7] feat(config): add property delete rpc Add a PostgREST-accessible SQL function for deleting a single config item property owned by a specific creator.\n\nThe delete RPC removes only the matching owned property by name while preserving other owners and legacy unowned properties, so UI deletes do not replace the user's entire property list. --- functions/config_item_properties.sql | 56 +++++++++++++++++++ rbac/objects.go | 1 + tests/config_item_properties_function_test.go | 48 ++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/functions/config_item_properties.sql b/functions/config_item_properties.sql index f3754c632..0cd2e03ad 100644 --- a/functions/config_item_properties.sql +++ b/functions/config_item_properties.sql @@ -157,3 +157,59 @@ BEGIN WHERE NOT EXISTS (SELECT 1 FROM updated); END; $$ LANGUAGE plpgsql; + + +-- Deletes a single property by name from the config item's properties owned by +-- (p_creator_type, p_created_by). Properties owned by other creators, and legacy +-- properties without ownership metadata, are preserved. +CREATE +OR REPLACE FUNCTION delete_config_item_property( + p_config_id uuid, + p_creator_type TEXT, + p_created_by uuid, + p_property_name TEXT +) RETURNS TABLE(changed BOOLEAN, properties jsonb) AS $$ +BEGIN + RETURN QUERY + WITH updated AS ( + UPDATE config_items ci + SET properties = + COALESCE( + ( + SELECT jsonb_agg(prop ORDER BY ord) + FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) + WITH ORDINALITY AS existing(prop, ord) + WHERE ( + prop->>'creator_type' = p_creator_type + AND prop->>'created_by' = p_created_by::text + AND prop->>'name' = p_property_name + ) IS NOT TRUE + ), + '[]'::jsonb + ) + WHERE ci.id = p_config_id + AND ci.properties IS DISTINCT FROM + COALESCE( + ( + SELECT jsonb_agg(prop ORDER BY ord) + FROM jsonb_array_elements(COALESCE(ci.properties, '[]'::jsonb)) + WITH ORDINALITY AS existing(prop, ord) + WHERE ( + prop->>'creator_type' = p_creator_type + AND prop->>'created_by' = p_created_by::text + AND prop->>'name' = p_property_name + ) IS NOT TRUE + ), + '[]'::jsonb + ) + RETURNING true AS changed, ci.properties + ) + SELECT updated.changed, updated.properties + FROM updated + UNION ALL + SELECT false AS changed, ci.properties + FROM config_items ci + WHERE ci.id = p_config_id + AND NOT EXISTS (SELECT 1 FROM updated); +END; +$$ LANGUAGE plpgsql; \ No newline at end of file diff --git a/rbac/objects.go b/rbac/objects.go index 595dfe3ee..251d12584 100644 --- a/rbac/objects.go +++ b/rbac/objects.go @@ -153,6 +153,7 @@ var dbResourceObjMap = map[string]string{ "responders": policy.ObjectIncident, "rpc/update_config_item_properties": policy.ObjectCatalog, + "rpc/delete_config_item_property": policy.ObjectCatalog, "rpc/lookup_component_config_id_related_components": policy.ObjectTopology, "rpc/_related_config_ids_recursive": policy.ObjectCatalog, "rpc/check_summary_for_component": policy.ObjectCanary, diff --git a/tests/config_item_properties_function_test.go b/tests/config_item_properties_function_test.go index c4bd122fa..275e0cc1e 100644 --- a/tests/config_item_properties_function_test.go +++ b/tests/config_item_properties_function_test.go @@ -81,6 +81,27 @@ var _ = Describe("update_config_item_properties", func() { Expect(findProperty(props, "Legacy")).To(HaveKeyWithValue("text", "keep")) }) + It("deletes one creator-owned property without replacing the whole owner slice", func() { + configID := uuid.New() + personA := uuid.New() + personB := uuid.New() + seedConfigItemWithProperties(configID, models.OwnedProperties{ + {Property: types.Property{Name: "Owner", Text: "Team"}, CreatorType: models.PropertyCreatorTypePerson, CreatedBy: personA.String()}, + {Property: types.Property{Name: "Runbook", Text: "rb"}, CreatorType: models.PropertyCreatorTypePerson, CreatedBy: personA.String()}, + {Property: types.Property{Name: "Owner", Text: "Other Team"}, CreatorType: models.PropertyCreatorTypePerson, CreatedBy: personB.String()}, + {Property: types.Property{Name: "Legacy", Text: "keep"}}, + }) + + result := callDeleteConfigItemProperty(configID, models.PropertyCreatorTypePerson, personA, "Owner") + + Expect(result.Changed).To(BeTrue()) + props := propertyMaps(result.Properties) + Expect(findPropertyByOwner(props, "Owner", personA.String())).To(BeNil()) + Expect(findPropertyByOwner(props, "Runbook", personA.String())).To(HaveKeyWithValue("text", "rb")) + Expect(findPropertyByOwner(props, "Owner", personB.String())).To(HaveKeyWithValue("text", "Other Team")) + Expect(findProperty(props, "Legacy")).To(HaveKeyWithValue("text", "keep")) + }) + It("does not clobber concurrent updates from different scrapers", func() { configID := uuid.New() scraperA := uuid.New() @@ -142,6 +163,24 @@ func callUpdateConfigItemPropertiesErr(configID uuid.UUID, creatorType string, c return err } +func callDeleteConfigItemProperty(configID uuid.UUID, creatorType string, createdBy uuid.UUID, propertyName string) models.UpdateConfigItemPropertiesResult { + var row struct { + Changed bool + Properties string + } + Expect(DefaultContext.DB().Raw( + `SELECT changed, properties FROM delete_config_item_property(?, ?, ?, ?)`, + configID, + creatorType, + createdBy, + propertyName, + ).Scan(&row).Error).To(Succeed()) + + var props models.OwnedProperties + Expect(json.Unmarshal([]byte(row.Properties), &props)).To(Succeed()) + return models.UpdateConfigItemPropertiesResult{Changed: row.Changed, Properties: props} +} + func propertyMaps(props models.OwnedProperties) []map[string]any { data, err := json.Marshal(props) Expect(err).ToNot(HaveOccurred()) @@ -158,3 +197,12 @@ func findProperty(props []map[string]any, name string) map[string]any { } return nil } + +func findPropertyByOwner(props []map[string]any, name string, createdBy string) map[string]any { + for _, prop := range props { + if prop["name"] == name && prop["created_by"] == createdBy { + return prop + } + } + return nil +} From 4f6025d978244c09fa59cda00c6ffefe74606fb1 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Wed, 29 Apr 2026 18:46:00 +0545 Subject: [PATCH 7/7] fix: benchmark --- bench/insertion_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bench/insertion_test.go b/bench/insertion_test.go index 1904e7f3c..e0f65124b 100644 --- a/bench/insertion_test.go +++ b/bench/insertion_test.go @@ -141,7 +141,7 @@ func BenchmarkUpdateOfConfigsWithProperties(b *testing.B) { cleanupConfigItemBenchRows(b) } -func buildBenchProperties(seed int, payload string) types.Properties { +func buildBenchProperties(seed int, payload string) models.OwnedProperties { properties := make(types.Properties, 0, benchPropertyCount) for i := range benchPropertyCount { properties = append(properties, &types.Property{ @@ -153,7 +153,7 @@ func buildBenchProperties(seed int, payload string) types.Properties { Order: i, }) } - return properties + return models.NewOwnedProperties(properties) } func cleanupExternalUserBenchRows(b *testing.B) {