Skip to content

test: add encryption with multi-level storage test case#35131

Merged
guanshengliang merged 1 commit intomainfrom
feat/main/taosk-bug-fix
Apr 15, 2026
Merged

test: add encryption with multi-level storage test case#35131
guanshengliang merged 1 commit intomainfrom
feat/main/taosk-bug-fix

Conversation

@xiao-77
Copy link
Copy Markdown
Contributor

@xiao-77 xiao-77 commented Apr 14, 2026

  • Add test case for encryption with multi-level storage configuration
  • Verify taosk correctly identifies primary disk (level=0, primary=1)
  • Verify encryption keys are stored only on primary disk
  • Test encrypted database operations with multi-level storage
  • Add test to cases.task with multi-level storage parameters (-L 3 -D 2)

Jira: TS-7230

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

- Add test case for encryption with multi-level storage configuration
- Verify taosk correctly identifies primary disk (level=0, primary=1)
- Verify encryption keys are stored only on primary disk
- Test encrypted database operations with multi-level storage
- Add test to cases.task with multi-level storage parameters (-L 3 -D 2)

Jira: TS-7230
@xiao-77 xiao-77 requested a review from a team as a code owner April 14, 2026 06:07
Copilot AI review requested due to automatic review settings April 14, 2026 06:07
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new CI test to validate transparent encryption behavior when TDengine is configured with multi-level storage (multiple dataDir entries), and wires it into the CI task list with multi-level parameters.

Changes:

  • Register a new pytest case in test/ci/cases.task with -L 3 -D 2 to enable multi-level storage during the run.
  • Add test_encryption_mlevel.py to check key placement on the primary disk and run basic encrypted DB CRUD/maintenance operations (flush/compact) under multi-level storage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
test/ci/cases.task Adds the new encryption + multi-level storage pytest to CI with -L 3 -D 2.
test/cases/31-Security/07-EncryptionMultiLevel/test_encryption_mlevel.py New test case covering encryption key placement + encrypted DB operations under multi-level storage.

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

Comment on lines +247 to +252
for i, disk in enumerate(data_dirs):
vnode_dir = os.path.join(disk, "vnode")
if os.path.exists(vnode_dir):
vnode_list = os.listdir(vnode_dir)
tdLog.info(f"Tier {i} ({disk}): {len(vnode_list)} vnode(s)")

Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

In multi-level storage tdDnodes.dnodes[0].dataDir entries are formatted like "<path> <level> <primary>". Here disk is used directly in os.path.join(disk, "vnode"), so the resulting path will include the level/primary suffix and will not match the on-disk directory. Split/extract the real path (e.g., disk.split()[0]) before building vnode_dir (same parsing you already do earlier for disk_entry).

Copilot uses AI. Check for mistakes.
Comment on lines +201 to +206
# Insert data spanning multiple days to trigger multi-level storage
day_offset = j // 100 # Change day every 100 records
sec_offset = j % 100 # Seconds within the day
# Calculate timestamp: current time - days - seconds
ts = current_time_ms - (day_offset * 86400000) - (sec_offset * 1000)
values.append(f"({ts}, {220 + i}, {10.0 + i * 0.1}, {0.95 + i * 0.01}, {25.0 + i})")
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

The generated timestamps only span ~0–9 days in the past (day_offset = j // 100 with rows_per_table=1000), but the database is created with KEEP 40d,70d,120d. This means the test will not actually exercise multi-level storage tiering/migration based on KEEP boundaries, so the later “distribution across tiers” checks/logging won’t validate the intended behavior. Consider generating data older than 40/70/120 days (or reduce KEEP values in the test DB) and add at least one assertion tied to tier placement.

Copilot uses AI. Check for mistakes.
Comment on lines +225 to +236
tdSql.execute(f"FLUSH DATABASE {self.db}")
tdLog.info("Database flushed")

# Wait for flush to complete
import time
time.sleep(2)

# Verify data after flush
tdSql.query("SELECT COUNT(*) FROM meters")
rows_after_flush = tdSql.queryResult[0][0]
assert rows_after_flush == expected_rows, f"Data lost after flush: {rows_after_flush} != {expected_rows}"
tdLog.info(f"✓ Data integrity verified after flush: {rows_after_flush} rows")
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

FLUSH DATABASE completion is assumed after a fixed sleep(2). Flush can take longer under load, making this test flaky. Prefer polling for completion using an observable signal (e.g., retrying a query that depends on flushed state, or a system table/status check) with a bounded timeout, similar to how other tests wait for background operations.

Copilot uses AI. Check for mistakes.
Comment on lines +276 to +281
tdSql.execute(f"COMPACT DATABASE {self.db}")
tdLog.info("Database compacted")

# Wait for compact to complete
time.sleep(2)

Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

COMPACT DATABASE is followed by a fixed sleep(2) and then assertions. Compaction is asynchronous and can easily exceed 2s in CI, so this is prone to intermittent failures or false confidence. Consider waiting for compaction completion by polling SHOW COMPACTS until empty (see test/cases/04-SuperTables/01-Create/test_stable_keep_compact.py), with a timeout.

Copilot uses AI. Check for mistakes.
Comment on lines +91 to +98
# Test 1: Verify taosk identifies primary disk
tdLog.info("=" * 80)
tdLog.info("Test 1: Verify taosk identifies primary disk")
tdLog.info("=" * 80)

taosk_bin = etool.taoskFile()
tdLog.info(f"taosk binary: {taosk_bin}")

Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

This section says it verifies “taosk identifies primary disk”, but the test doesn’t actually invoke taosk or assert anything about its behavior/output (it only logs the binary path and then checks key files on disk). Either add a taosk invocation/assertion relevant to primary-disk selection, or rename the test step text/comments so expectations match what’s being validated.

Copilot uses AI. Check for mistakes.
Comment on lines +141 to +145

if os.path.exists(other_master):
tdLog.exit(f"✗ ERROR: master.bin found on non-primary disk {i}: {other_master}")
else:
tdLog.info(f"✓ Disk {i} ({disk_path}): No encryption keys (correct)")
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

The test claims to verify encryption keys are stored only on the primary disk, but for non-primary disks it only checks master.bin. If derived.bin (and/or other key material) can be generated, this would miss it and still pass. Consider checking derived.bin on non-primary disks as well (and treating its presence as a failure) so the assertion matches the test intent.

Suggested change
if os.path.exists(other_master):
tdLog.exit(f"✗ ERROR: master.bin found on non-primary disk {i}: {other_master}")
else:
tdLog.info(f"✓ Disk {i} ({disk_path}): No encryption keys (correct)")
other_derived = os.path.join(other_key_dir, "derived.bin")
if os.path.exists(other_master):
tdLog.exit(f"✗ ERROR: master.bin found on non-primary disk {i}: {other_master}")
elif os.path.exists(other_derived):
tdLog.exit(f"✗ ERROR: derived.bin found on non-primary disk {i}: {other_derived}")
else:
tdLog.info(f"✓ Disk {i} ({disk_path}): No master.bin or derived.bin (correct)")

Copilot uses AI. Check for mistakes.
@guanshengliang guanshengliang merged commit da2d1c7 into main Apr 15, 2026
11 of 12 checks passed
@guanshengliang guanshengliang deleted the feat/main/taosk-bug-fix branch April 15, 2026 01:15
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.

3 participants