Skip to content

feat: flatten tenant_configs table with type column#304

Open
niazkhansap wants to merge 1 commit into
mainfrom
improvement/flatten-tenant-configs
Open

feat: flatten tenant_configs table with type column#304
niazkhansap wants to merge 1 commit into
mainfrom
improvement/flatten-tenant-configs

Conversation

@niazkhansap

@niazkhansap niazkhansap commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Related cleanup PR needs to be rolled out after this.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9d344ea3-5aea-4444-be6e-a48942dd1b29

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The PR restructures TenantConfig storage from a single legacy JSON blob to typed flat rows keyed by (Key, Type), introducing LegacyTenantConfig for backward-compatible reads. TenantConfigManager gains flat-row read/write logic with legacy fallback, a TenantConfigStore interface wires into CryptoAccessDataSyncer/CryptoReconciler, authz policies grant List access, mock repo indexing is updated, and schema/data migrations backfill flat rows.

Changes

TenantConfig flat-row storage and dependent wiring

Layer / File(s) Summary
TenantConfig model schema change
internal/model/tenantconfigs.go
TenantConfig gains a Type primary-key field, Value changes from json.RawMessage/jsonb to string/text, and a new LegacyTenantConfig model is added for legacy JSONB reads.
TenantConfigManager flat-row read/write implementation
internal/manager/tenantconfigs.go, internal/manager/export_test.go, internal/manager/tenantconfigs_test.go
Adds flat-row constants/keys, rewrites workflow/keystore config get/set to prefer flat rows with legacy blob fallback, adds reconstruction/parsing/writing helpers and transactional row upsert/replace helpers, removes obsolete legacy converters, and adds dual-read/round-trip/edge-case tests.
Mock repo composite-key indexing
internal/repo/mock/db.go, internal/repo/mock/db_test.go, internal/repo/mock/constant.go
In-memory repo now indexes TenantConfig/LegacyTenantConfig by composite (key, type); Create, Get, Delete, Update are updated accordingly, and tests switch Value to plain strings.
TenantConfigStore interface and reconciler/syncer wiring
internal/event-processor/access_data.go, internal/event-processor/reconciler.go, cmd/event-reconciler/main.go, test/integration/event-processor/reconciliation_test.go
Adds a TenantConfigStore interface used by CryptoAccessDataSyncer, extends NewCryptoReconciler with a tenantConfigStore parameter, and wires a constructed TenantConfigManager through the CLI entrypoint and integration test setup.
Test/authz-policy wiring and repo authorization updates
internal/authz/policy_tests/event_reconciler_test.go, internal/event-processor/access_data_test.go, internal/event-processor/reconciler_test.go, internal/authz/repo_internal_policies.go, internal/authz/repo_internal_policies_test_coverage.md
Tests construct and pass TenantConfigManager into reconciler/syncer constructors, seed LegacyTenantConfig rows with string values, and internal roles gain RepoActionList on TenantConfig (documented in the coverage markdown).
Controller and testutils TenantConfig.Value string conversions
internal/controllers/cmk/key_controller_test.go, internal/controllers/cmk/tenantconfigs_controller_test.go, internal/testutils/model.go, internal/manager/workflow_test.go
Test setups switch to model.LegacyTenantConfig and persist marshaled JSON as strings via a consistent unmarshal/marshal round-trip pattern.
Schema and data migrations for flattened tenant configs
migrations/tenant/data/00002_repair_keystore_config_shape.go, migrations/tenant/data/00003_flatten_tenant_configs.go, migrations/tenant/data/main.go, migrations/tenant/schema/00014_flatten_tenant_configs.sql, test/db-migration/migration_test.go
Adds migrations to repair the DEFAULT_KEYSTORE JSON shape and add type/value_text columns with composite key/indexes, backfills flat rows from legacy blobs, registers both migrations, and adds schema/data migration tests validating up/down behavior.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@niazkhansap

Copy link
Copy Markdown
Contributor Author

@coderabbitai summary

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
✅ Action performed

Summary regeneration triggered.

minh-nghia
minh-nghia previously approved these changes Jul 6, 2026
@niazkhansap
niazkhansap force-pushed the improvement/flatten-tenant-configs branch from bb8f912 to cf29c9d Compare July 7, 2026 11:43
@niazkhansap
niazkhansap requested a review from minh-nghia July 7, 2026 14:44
Comment on lines +6 to +17
UPDATE tenant_configs
SET value = jsonb_build_object(
'roleManagementConfig', jsonb_build_object( -- NOSONAR
'localityId', value::jsonb -> 'localityId', -- NOSONAR
'commonName', value::jsonb -> 'commonName', -- NOSONAR
'accessData', value::jsonb -> 'managementAccessData'
),
'supportedRegions', value::jsonb -> 'supportedRegions' -- NOSONAR
)::jsonb
WHERE "key" = 'DEFAULT_KEYSTORE'
AND value::jsonb ? 'localityId'
AND NOT (value::jsonb ? 'roleManagementConfig');

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.

These are data migrations, not schema ones. This is also a long running one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice catch moved

Comment on lines +1 to +30
-- This migration flattens tenant_configs from a single JSON blob per key into
-- (key, value, type) flat rows so individual config entries can be filtered by
-- type/key. Data migration 00002_flatten_tenant_configs.go backfills flat rows
-- from the legacy blobs; the legacy blobs are dropped in a future release.
--
-- Down drops flat rows. Since post-flatten writes target flat rows only, Down
-- discards any changes made after the migration. Use only before traffic;
-- restore from backup for operational rollback after production writes.
-- +goose Up

ALTER TABLE tenant_configs ADD COLUMN IF NOT EXISTS "type" varchar(255) NOT NULL DEFAULT '';

ALTER TABLE tenant_configs ALTER COLUMN value TYPE text USING value::text;

ALTER TABLE tenant_configs DROP CONSTRAINT IF EXISTS tenant_configs_pkey;
ALTER TABLE tenant_configs ADD CONSTRAINT tenant_configs_pkey PRIMARY KEY ("key", "type");

CREATE INDEX IF NOT EXISTS idx_tenant_configs_type ON tenant_configs ("type");

-- +goose Down

DROP INDEX IF EXISTS idx_tenant_configs_type;

-- Flat rows hold non-JSON text and would fail the jsonb cast below.
DELETE FROM tenant_configs WHERE length("type") > 0;

ALTER TABLE tenant_configs DROP CONSTRAINT IF EXISTS tenant_configs_pkey;
ALTER TABLE tenant_configs DROP COLUMN IF EXISTS "type";
ALTER TABLE tenant_configs ALTER COLUMN value TYPE jsonb USING value::jsonb;
ALTER TABLE tenant_configs ADD CONSTRAINT tenant_configs_pkey PRIMARY KEY ("key");

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.

None of this is safe. We need to use Expand and Contract pattern. If we run it like this we break running instances

@niazkhansap niazkhansap Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reworked to ensure use of the Expand & Contract pattern

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.

These data migrations are also not safe. THere needs to be version/schema checks before running them as they must only be executed if the Database is in a certain state, otherwise they need to be skipped

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added guards to both migrations

@niazkhansap
niazkhansap force-pushed the improvement/flatten-tenant-configs branch 2 times, most recently from 78a6ea9 to 7b1bed4 Compare July 9, 2026 18:09
@niazkhansap

Copy link
Copy Markdown
Contributor Author

@coderabbitai summary

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
✅ Action performed

Summary regeneration triggered.

Signed-off-by: Niaz Khan <niazkhansap@gmail.com>
@niazkhansap
niazkhansap force-pushed the improvement/flatten-tenant-configs branch from 7b1bed4 to 3808e84 Compare July 14, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants