Skip to content

fix: unify labels and tags via resource_labels#327

Open
davidbolet wants to merge 4 commits into
mainfrom
task/resource-labels-pr
Open

fix: unify labels and tags via resource_labels#327
davidbolet wants to merge 4 commits into
mainfrom
task/resource-labels-pr

Conversation

@davidbolet

@davidbolet davidbolet commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduce unified resource_labels model/table for labels and key-configuration tags.
  • Migrate existing key labels and tags into resource_labels with new tenant migrations.
  • Update managers, authz repo resource types/policies, and affected tests to use the new storage.

Test plan

  • go test ./... (integration tests may require local container/runtime setup)

Copilot AI review requested due to automatic review settings July 13, 2026 19:50
@davidbolet
davidbolet force-pushed the task/resource-labels-pr branch from 83c3eff to 7f96c84 Compare July 13, 2026 19:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a unified resource_labels storage model to consolidate key labels and key-configuration tags into a single table, updates managers/wiring/authz to use it, and refactors tests accordingly.

Changes:

  • Add resource_labels table + ResourceLabel model and ResourceLabelManager, and adapt existing Label/Tag managers to delegate to it.
  • Add tenant migrations intended to move existing key labels and tags into resource_labels.
  • Update authz resource types/policies and a broad set of unit/integration tests to use the new storage paths.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/integration/tenant-manager/provisioning_test.go Wires TagManager through ResourceLabelManager in integration suite setup.
migrations/tenant/schema/00016_add_resource_labels_table.sql Adds resource_labels table and indexes/uniqueness constraints.
migrations/tenant/schema/00017_migrate_labels_and_tags_data.sql Migrates existing label/tag data into resource_labels.
internal/repo/query.go Adds query fields needed for resource-label filtering (resource_type, value).
internal/operator/operator_test.go Updates operator test wiring to use ResourceLabelManager-backed TagManager.
internal/model/resource_label.go Introduces ResourceLabel model, resource type constants, and system.tag key constant.
internal/manager/resource_label.go Implements unified label/tag CRUD via ResourceLabelManager.
internal/manager/resource_label_test.go Adds unit tests for ResourceLabelManager label/tag behavior.
internal/manager/tag.go Refactors TagManager into an adapter over ResourceLabelManager.
internal/manager/tag_test.go Updates tag manager tests to assert against resource_labels storage.
internal/manager/key_label.go Refactors LabelManager into an adapter over ResourceLabelManager.
internal/manager/key_label_test.go Updates label manager tests to assert against resource_labels.
internal/manager/base.go Updates main manager composition to construct and share ResourceLabelManager.
internal/manager/workflow_test.go Updates workflow test wiring for new TagManager construction.
internal/manager/tenant_test.go Updates tenant test wiring for new TagManager construction.
internal/manager/system_test.go Updates system test wiring for new TagManager construction.
internal/manager/keyversion_test.go Updates keyversion test wiring for new TagManager construction.
internal/manager/keyconfiguration_test.go Updates key configuration tests to create/count tags via resource_labels.
internal/manager/key_test.go Updates key tests to use ResourceLabelManager-backed TagManager.
internal/manager/errors.go Adds/adjusts error sentinels used by the updated managers.
internal/controllers/cmk/keyconfiguration_tags_controller_test.go Updates controller tests to create/verify tags via resource_labels.
internal/controllers/cmk/key_labels_controller_test.go Updates controller tests to create/verify labels via resource_labels.
internal/constants/table-names.go Adds resource_labels table name constant.
internal/authz/policies.go Adds authz repo resource type for resource_labels.
internal/authz/repo_business_policies.go Grants appropriate actions on resource_labels for business roles.
internal/authz/policy_tests/workflow_autoassign_test.go Updates authz policy test wiring for TagManager changes.
internal/authz/policy_tests/keystore_pool_test.go Updates authz policy test wiring for TagManager changes.
internal/authz/policy_tests/hyok_sync_test.go Updates authz policy test wiring for TagManager changes.
internal/async/tasks/tenant/workflow_expiry_test.go Updates async task test wiring for TagManager changes.
cmd/tenant-manager/main.go Updates tenant-manager wiring for TagManager changes.
cmd/tenant-manager/main_test.go Avoids fixed port collision by using an ephemeral listener for status address.
cmd/tenant-manager-cli/commands/commands.go Updates CLI wiring for TagManager changes.
cmd/tenant-manager-cli/cli_test.go Updates CLI test wiring for TagManager changes.
cmd/task-worker/main.go Updates task worker wiring for TagManager changes.
charts/cmk/values-dev.yaml Updates dev chart values (feature gates/http swagger/plugins block).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +18 to +39
-- Migrate existing key_configuration_tags to resource_labels
-- Maps: key_configuration_tags + keyconfigurations_tags -> resource_labels
-- with resource_type='KEY_CONFIG' and key='system.tag'
DO $$
BEGIN
-- These legacy tables may not exist on fresh installs (they are dropped in migration 00002).
IF to_regclass('public.keyconfigurations_tags') IS NOT NULL
AND to_regclass('public.key_configuration_tags') IS NOT NULL THEN
INSERT INTO resource_labels (id, resource_type, resource_id, key, value, created_at, updated_at)
SELECT
gen_random_uuid() AS id, -- Generate new UUIDs for tag entries
'KEY_CONFIG' AS resource_type,
kt.key_configuration_id AS resource_id,
'system.tag' AS key,
t.value AS value,
CURRENT_TIMESTAMP AS created_at,
CURRENT_TIMESTAMP AS updated_at
FROM keyconfigurations_tags kt
JOIN key_configuration_tags t ON kt.key_configuration_tag_id = t.id
ON CONFLICT (resource_type, resource_id, key, value) DO NOTHING;
END IF;
END $$;
Comment on lines +22 to +35
-- Enable combined filtering and enforce uniqueness per resource
-- Prevents duplicate (resource_type, resource_id, key, value) combinations
CREATE UNIQUE INDEX idx_resource_labels_unique
ON resource_labels(resource_type, resource_id, key, value);

-- +goose StatementEnd

-- +goose Down
-- +goose StatementBegin
DROP INDEX IF EXISTS idx_resource_labels_unique;
DROP INDEX IF EXISTS idx_resource_labels_key;
DROP INDEX IF EXISTS idx_resource_labels_resource_type_id;
DROP TABLE IF EXISTS resource_labels;
-- +goose StatementEnd
Comment on lines +72 to +77
return m.r.Transaction(ctx, func(ctx context.Context) error {
for _, label := range labels {
// Ensure the label has correct resource type and ID
label.ResourceType = resourceType
label.ResourceID = resourceID

Comment on lines +53 to +56
labels, count, err := repo.ListAndCount(ctx, m.r, pagination, model.ResourceLabel{}, query)
if err != nil {
return nil, 0, errs.Wrap(ErrQueryLabelList, err)
}
require.NoError(t, err)
})

t.Run("Should rollback on error in transaction", func(t *testing.T) {
Copilot AI review requested due to automatic review settings July 13, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.

Comment on lines +18 to +39
-- Migrate existing key_configuration_tags to resource_labels
-- Maps: key_configuration_tags + keyconfigurations_tags -> resource_labels
-- with resource_type='KEY_CONFIG' and key='system.tag'
DO $$
BEGIN
-- These legacy tables may not exist on fresh installs (they are dropped in migration 00002).
IF to_regclass('public.keyconfigurations_tags') IS NOT NULL
AND to_regclass('public.key_configuration_tags') IS NOT NULL THEN
INSERT INTO resource_labels (id, resource_type, resource_id, key, value, created_at, updated_at)
SELECT
gen_random_uuid() AS id, -- Generate new UUIDs for tag entries
'KEY_CONFIG' AS resource_type,
kt.key_configuration_id AS resource_id,
'system.tag' AS key,
t.value AS value,
CURRENT_TIMESTAMP AS created_at,
CURRENT_TIMESTAMP AS updated_at
FROM keyconfigurations_tags kt
JOIN key_configuration_tags t ON kt.key_configuration_tag_id = t.id
ON CONFLICT (resource_type, resource_id, key, value) DO NOTHING;
END IF;
END $$;
Comment on lines +204 to +222
// Create new tags
for _, tag := range tags {
if tag == "" {
continue // Skip empty tags
}

label := &model.ResourceLabel{
ID: uuid.New(),
ResourceType: resourceType,
ResourceID: resourceID,
Key: model.SystemTagKey,
Value: tag,
}

err = m.r.Create(ctx, label)
if err != nil {
return errs.Wrap(ErrCreateTag, err)
}
}
Comment on lines +73 to +77
for _, label := range labels {
// Ensure the label has correct resource type and ID
label.ResourceType = resourceType
label.ResourceID = resourceID

Comment thread internal/manager/resource_label.go Outdated
Comment on lines +249 to +250
// Helper to marshal tags to JSON (for backwards compatibility if needed)
func marshalTags(tags []string) (json.RawMessage, error) {
- Introduce unified resource_labels model/table (resource_type, resource_id, key, value)
- Treat tags as labels with key=system.tag for KEY_CONFIG
- Add tenant schema + data migrations (without dropping legacy tables)
- Ensure cleanup on resource deletion via managers

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: David <davidbolet@gmail.com>
@davidbolet
davidbolet force-pushed the task/resource-labels-pr branch from 7f96c84 to 2a156d8 Compare July 15, 2026 10:44
Signed-off-by: David <davidbolet@gmail.com>
Copilot AI review requested due to automatic review settings July 17, 2026 10:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Comment on lines +24 to +25
IF to_regclass('public.keyconfigurations_tags') IS NOT NULL
AND to_regclass('public.key_configuration_tags') IS NOT NULL THEN
Comment on lines +59 to +64
// Build composite key to filter by resource type and resource ID
ck := repo.NewCompositeKey().
Where(repo.ResourceTypeField, resourceType).
Where(repo.ResourceIDField, resourceID).
Where(repo.KeyField, model.SystemTagKey, repo.NotEq)

Comment on lines +254 to +272
// Create new tags
for _, tag := range tags {
if tag == "" {
continue // Skip empty tags
}

label := &model.ResourceLabel{
ID: uuid.New(),
ResourceType: resourceType,
ResourceID: resourceID,
Key: model.SystemTagKey,
Value: tag,
}

err = m.r.Create(ctx, label)
if err != nil {
return errs.Wrap(ErrCreateTag, err)
}
}
Signed-off-by: David <davidbolet@gmail.com>
Copilot AI review requested due to automatic review settings July 17, 2026 11:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Comment on lines +18 to +39
-- Migrate existing key_configuration_tags to resource_labels
-- Maps: key_configuration_tags + keyconfigurations_tags -> resource_labels
-- with resource_type='KEY_CONFIG' and key='system.tag'
DO $$
BEGIN
-- These legacy tables may not exist on fresh installs (they are dropped in migration 00002).
IF to_regclass('public.keyconfigurations_tags') IS NOT NULL
AND to_regclass('public.key_configuration_tags') IS NOT NULL THEN
INSERT INTO resource_labels (id, resource_type, resource_id, key, value, created_at, updated_at)
SELECT
gen_random_uuid() AS id, -- Generate new UUIDs for tag entries
'KEY_CONFIG' AS resource_type,
kt.key_configuration_id AS resource_id,
'system.tag' AS key,
t.value AS value,
CURRENT_TIMESTAMP AS created_at,
CURRENT_TIMESTAMP AS updated_at
FROM keyconfigurations_tags kt
JOIN key_configuration_tags t ON kt.key_configuration_tag_id = t.id
ON CONFLICT (resource_type, resource_id, key, value) DO NOTHING;
END IF;
END $$;
Comment on lines +176 to +194
// Create new tags
for _, tag := range tags {
if tag == "" {
continue // Skip empty tags
}

label := &model.ResourceLabel{
ID: uuid.New(),
ResourceType: resourceType,
ResourceID: resourceID,
Key: model.SystemTagKey,
Value: tag,
}

err = m.r.Create(ctx, label)
if err != nil {
return errs.Wrap(ErrCreateTag, err)
}
}
Comment on lines +86 to +97
return m.r.Transaction(ctx, func(ctx context.Context) error {
for _, label := range labels {
// Ensure the label has correct resource type and ID
label.ResourceType = resourceType
label.ResourceID = resourceID

if err := m.upsertLabel(ctx, label); err != nil {
return err
}
}
return nil
})
Copilot AI review requested due to automatic review settings July 17, 2026 11:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 5 comments.

Comment on lines +176 to +194
// Create new tags
for _, tag := range tags {
if tag == "" {
continue // Skip empty tags
}

label := &model.ResourceLabel{
ID: uuid.New(),
ResourceType: resourceType,
ResourceID: resourceID,
Key: model.SystemTagKey,
Value: tag,
}

err = m.r.Create(ctx, label)
if err != nil {
return errs.Wrap(ErrCreateTag, err)
}
}
Comment on lines +22 to +35
-- Enable combined filtering and enforce uniqueness per resource
-- Prevents duplicate (resource_type, resource_id, key, value) combinations
CREATE UNIQUE INDEX idx_resource_labels_unique
ON resource_labels(resource_type, resource_id, key, value);

-- +goose StatementEnd

-- +goose Down
-- +goose StatementBegin
DROP INDEX IF EXISTS idx_resource_labels_unique;
DROP INDEX IF EXISTS idx_resource_labels_key;
DROP INDEX IF EXISTS idx_resource_labels_resource_type_id;
DROP TABLE IF EXISTS resource_labels;
-- +goose StatementEnd
Comment on lines +18 to +39
-- Migrate existing key_configuration_tags to resource_labels
-- Maps: key_configuration_tags + keyconfigurations_tags -> resource_labels
-- with resource_type='KEY_CONFIG' and key='system.tag'
DO $$
BEGIN
-- These legacy tables may not exist on fresh installs (they are dropped in migration 00002).
IF to_regclass('public.keyconfigurations_tags') IS NOT NULL
AND to_regclass('public.key_configuration_tags') IS NOT NULL THEN
INSERT INTO resource_labels (id, resource_type, resource_id, key, value, created_at, updated_at)
SELECT
gen_random_uuid() AS id, -- Generate new UUIDs for tag entries
'KEY_CONFIG' AS resource_type,
kt.key_configuration_id AS resource_id,
'system.tag' AS key,
t.value AS value,
CURRENT_TIMESTAMP AS created_at,
CURRENT_TIMESTAMP AS updated_at
FROM keyconfigurations_tags kt
JOIN key_configuration_tags t ON kt.key_configuration_tag_id = t.id
ON CONFLICT (resource_type, resource_id, key, value) DO NOTHING;
END IF;
END $$;
Comment on lines +54 to +56
DELETE FROM resource_labels
WHERE resource_type = 'KEY_CONFIG'
AND key = 'system.tag';
require.NoError(t, err)
})

t.Run("Should rollback on error in transaction", func(t *testing.T) {
@openkcm openkcm deleted a comment from coderabbitai Bot Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants