Skip to content

Fix account unlock propagation for pipe password checks#17814

Open
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/pipe-account-unlock-broadcast
Open

Fix account unlock propagation for pipe password checks#17814
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/pipe-account-unlock-broadcast

Conversation

@Caideyipi
Copy link
Copy Markdown
Collaborator

@Caideyipi Caideyipi commented Jun 2, 2026

What failed

IoTDBPipePermissionIT.testIllegalPassword failed with an AssertionError at:

TestUtils.assertDataEventuallyOnEnv(...)
IoTDBPipePermissionIT.java:606

The test expected the receiver to eventually observe count(root.vehicle.plane.pressure) = 2, but the count stayed at 1.

The cluster logs showed repeated pipe metadata push failures on one sender DataNode:

Failed to PIPE_PUSH_ALL_META / PIPE_PUSH_SINGLE_META on DataNodeId: 2
Failed to handle pipe meta changes for a2b, because Failed to check password for pipe a2b.

Why it failed

In this test, the pipe password is intentionally changed to an invalid/stale value after restart. The pipe keeps retrying with the stale password and can trigger the login lock for user thulab on the DataNode that handles the pipe metadata push.

The test then runs:

alter user thulab account unlock

Before this patch, ACCOUNT_UNLOCK was handled only inside the DataNode that executed the SQL. It directly called the local LoginLockManager and never went through the ConfigNode auth procedure. That means only one DataNode was unlocked, while another DataNode could still keep thulab locked in its local in-memory login-lock state.

As a result, ConfigNode kept trying to push updated pipe metadata to the still-locked DataNode. That DataNode rejected the pipe password check, metadata synchronization was delayed, and the new insert was not replicated to the receiver before the assertion timed out.

Why this needs a cluster-wide fix

Login lock state is local DataNode memory, not persistent auth metadata stored only on ConfigNode. A user-level ACCOUNT_UNLOCK statement is therefore incomplete if it only clears the lock on the SQL coordinator DataNode. For pipe password checks, any DataNode can be the one validating the pipe credentials during metadata push/recovery, so the unlock needs to reach every DataNode.

What this patch changes

This patch routes ACCOUNT_UNLOCK through the normal ConfigNode auth operation procedure and reuses the existing DataNode permission-cache invalidation broadcast to clear the login lock on every DataNode.

Concretely:

  • Adds tree-model ConfigPhysicalPlanType.AccountUnlock and maps tree AuthorType.ACCOUNT_UNLOCK to it.
  • Keeps relational RAccountUnlock handling and adds the missing special mapping for AuthorRType.ACCOUNT_UNLOCK.
  • Treats account unlock as a no-op persistent auth plan on ConfigNode, while still validating that the user exists.
  • Marks the DataNode invalidation request for account unlock, so each DataNode clears its local LoginLockManager entry and invalidates the user permission cache.
  • Adds factory/visitor/executor support so the new plan type can be serialized, deserialized, and dispatched correctly.
  • Adds auth procedure serialization coverage for both AccountUnlock and RAccountUnlock.
  • Adds the missing fail() assertion in IoTDBPipePermissionIT.testIllegalPassword after the intentionally invalid pipe password alteration.

This avoids thrift IDL changes and keeps the unlock operation scoped to the existing auth-cache invalidation path.

Tests

mvn -Ddevelocity.off=true test -pl iotdb-core/confignode -Dtest=AuthOperationProcedureTest -DforkCount=0
mvn -Ddevelocity.off=true compile -pl iotdb-core/datanode -DskipTests
mvn -Ddevelocity.off=true test-compile -pl integration-test -P with-integration-tests -DskipTests

I did not run the full IoTDBPipePermissionIT#testIllegalPassword ClusterIT locally because this machine repeatedly hit Windows pagefile/native-memory exhaustion when starting forked JVMs. The focused compile and serialization checks above passed.

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.

1 participant