Skip to content

Conversation

calvinbrewer
Copy link
Contributor

Fixes issues with EQL configuration remove functions and updates documentation to match current behavior.

Issues Fixed

  1. Remove functions failing on empty configurations: Functions would delete pending configs but still try to migrate them, causing "No pending configuration exists to encrypt" errors
  2. Database constraints rejecting empty configs: Updated constraints to allow empty tables and properly handle missing cast_as fields
  3. Aggressive cleanup in remove_search_config: Now only removes the specific index instead of the entire column configuration
  4. Outdated documentation: Added missing migrating parameter and clarified behavioral differences

Breaking Changes

None - remove is now working as expected

Copy link
Contributor

@auxesis auxesis left a comment

Choose a reason for hiding this comment

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

LGTM, with some suggested improvements.


-- Step 4: Verify configuration
SELECT * FROM eql_v2.config();
```
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a step 5 to show a successful query of the encrypted_email column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want to keep this section focused on config with EQL

Comment on lines +389 to +402
- `eql_v2.add_column(table_name, column_name, cast_as DEFAULT 'text', migrating DEFAULT false)` - Add a new encrypted column
- `eql_v2.remove_column(table_name, column_name, migrating DEFAULT false)` - Remove an encrypted column completely

**Index Management:**
- `eql_v2.add_search_config(table_name, column_name, index_name, cast_as DEFAULT 'text', opts DEFAULT '{}', migrating DEFAULT false)` - Add a search index to a column
- `eql_v2.remove_search_config(table_name, column_name, index_name, migrating DEFAULT false)` - Remove a specific search index (preserves column configuration)
- `eql_v2.modify_search_config(table_name, column_name, index_name, cast_as DEFAULT 'text', opts DEFAULT '{}', migrating DEFAULT false)` - Modify an existing search index

**Configuration Management:**
- `eql_v2.migrate_config()` - Manually migrate pending configuration to encrypting state
- `eql_v2.activate_config()` - Manually activate encrypting configuration
- `eql_v2.discard()` - Discard pending configuration changes
- `eql_v2.config()` - View current configuration in tabular format (returns a table with columns: state, relation, col_name, decrypts_as, indexes)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice way of denoting which functions do what.

calvinbrewer and others added 2 commits July 15, 2025 08:52
Co-authored-by: Lindsay Holmwood <[email protected]>
Co-authored-by: Lindsay Holmwood <[email protected]>
@calvinbrewer calvinbrewer merged commit 5d09904 into main Jul 15, 2025
4 checks passed
@calvinbrewer calvinbrewer deleted the fix-remvoe branch July 15, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants