joseluisll opened a new pull request, #8638:
URL: https://github.com/apache/hadoop/pull/8638
### Description of PR
`dfs.block.access.token.lifetime` is read without validation. A non-positive
value is accepted and produces block access tokens that are already expired
at
the instant they are created, so every write fails with an error that names
neither the property nor the cause.
`BlockManager.createBlockTokenSecretManager` converts the value with
`lifetimeMin * 60 * 1000L` and passes it to `BlockTokenSecretManager`, which
stamps every token with
```java
identifier.setExpiryDate(timer.now() + tokenLifetime); //
BlockTokenSecretManager:490
```
and evaluates expiry with a strict comparison
```java
private static boolean isExpired(long expiryDate) {
return Time.now() > expiryDate; //
BlockTokenSecretManager:443
}
```
With a lifetime of `0` the expiry date equals the creation instant, so the
token is invalid as soon as the clock advances by one millisecond — which it
always has by the time the token has travelled NameNode → client → DataNode.
`DataXceiver` answers `ERROR_ACCESS_TOKEN` and
`DataStreamer.createBlockOutputStream` fails with
`InvalidBlockTokenException`.
Unlike `InvalidEncryptionKeyException`, the write path has no token-refetch
retry, so each DataNode is marked bad and excluded in turn until the pipeline
is exhausted.
Reproduced on a MiniDFSCluster with `dfs.block.access.token.enable=true` and
`dfs.block.access.token.lifetime=0`, before the fix:
```
WARN hdfs.DataStreamer (DataStreamer.java:createBlockOutputStream(1960)) -
Exception in createBlockOutputStream blk_1073741825_1001
org.apache.hadoop.hdfs.security.token.block.InvalidBlockTokenException:
Got access token error, status message , ack with firstBadLink as
127.0.0.1:50639
at
DataTransferProtoUtil.checkBlockOpStatus(DataTransferProtoUtil.java:113)
at DataStreamer.createBlockOutputStream(DataStreamer.java:1948)
at DataStreamer.setupPipelineForCreate(DataStreamer.java:1843)
WARN - Abandoning BP-47005896-192.168.1.76-1785266294724:blk_1073741825_1001
WARN - Excluding datanode DatanodeInfoWithStorage[127.0.0.1:50639,...]
WARN - DataStreamer Exception
java.io.IOException: Unable to create new block.
```
Two further consequences of the same arithmetic are worth recording:
* `generateDataEncryptionKey()` also stamps `timer.now() + tokenLifetime`
(`BlockTokenSecretManager:541`), so a non-positive lifetime additionally
produces data-transfer encryption keys that are born expired.
* The value propagates cluster-wide through `ExportedBlockKeys`: DataNodes
(`DataNode:2132`) and the Balancer (`KeyManager:74`) build their own
`BlockTokenSecretManager` from the value the NameNode advertises.
Validating
at the NameNode therefore covers the whole cluster, and no DataNode-side
check is needed.
#### The change
`BlockManager.createBlockTokenSecretManager` now rejects a non-positive
lifetime with `HadoopIllegalArgumentException`, naming the property and the
constraint. The check runs only when `dfs.block.access.token.enable` is true,
so clusters that do not use block tokens are unaffected. Because it sits in
the
`BlockManager` constructor it applies to every path that builds an
`FSNamesystem` — the NameNode will neither format nor start with an invalid
value, and `NameNode.main` reports it through the existing
`terminate(1, e)` path.
The bound is simply "greater than zero", not some larger floor. The property
is
expressed in minutes, so the smallest representable positive value (`1`, i.e.
60,000 ms) already exceeds the mint-to-verify window by about four orders of
magnitude; there is no positive value in this config's units that reproduces
the failure. Choosing an arbitrary minimum would reject configurations that
work correctly today and would invent policy this project has never defined.
Short-but-positive lifetimes do have real costs — tolerance to
NameNode/DataNode
clock skew, and read-path token refetch churn (cf. HDFS-16332) — but those
are
tuning concerns rather than the defect reported here.
`dfs.block.access.key.update.interval` was examined as a possible sibling of
this bug and deliberately left alone: with an interval of `0` the retiring
key
still receives an expiry of `now + keyUpdateInterval + tokenLifetime`, so
tokens
remain verifiable. Key rotation becomes wasteful, not broken, and it is out
of
scope here.
#### Compatibility
This is technically an incompatible change: a cluster configured with a
non-positive `dfs.block.access.token.lifetime` starts today and will now
refuse
to start. It cannot regress a working deployment, because such a cluster is
already unable to complete any write, but it is called out here explicitly
rather than left to be discovered in review.
Contains content generated by Anthropic Claude Code.
### How was this patch tested?
New tests, both added by this patch:
* `TestBlockTokenZeroLifetime` (6 tests, no cluster) pins the underlying
behaviour at the secret-manager level in both legacy and protobuf token
formats: with a lifetime of `0` the minted token's expiry date falls inside
the creation instant, and the DataNode-side (worker) manager then rejects
it
with `InvalidToken ... is expired`; a 600-minute control verifies cleanly.
The expiry assertion waits for one clock tick past the recorded expiry
rather
than sleeping, so it is deterministic rather than racing the clock.
* `TestBlockTokenZeroLifetimeWithDFS` (5 tests, MiniDFSCluster) covers format
rejecting `0` and `-1`, an already-formatted NameNode refusing to restart
after the value is changed to `0`, a 600-minute control that writes and
verifies a file, and — importantly — that the value is still ignored when
`dfs.block.access.token.enable` is false.
Full runs on the branch (JDK 17, `mvn test -pl
hadoop-hdfs-project/hadoop-hdfs`):
```
TestBlockTokenZeroLifetime 6 tests 0 failures
TestBlockTokenZeroLifetimeWithDFS 5 tests 0 failures
TestBlockToken 25 tests 0 failures (2 skipped:
Linux-only /proc/self/fd)
TestBlockTokenWithDFS 4 tests 0 failures
TestFailoverWithBlockTokensEnabled 4 tests 0 failures
TestEncryptedTransfer 32 tests 0 failures
TestHdfsConfigFields 4 tests 0 failures
```
The pre-fix reproduction quoted above was produced by the same
`TestBlockTokenZeroLifetimeWithDFS` fixture with the validation reverted; the
committed version of that test asserts the post-fix behaviour.
No existing test or configuration file in the tree sets
`dfs.block.access.token.lifetime`, so the new validation changes no existing
test's behaviour.
### For code changes:
- [x] Does the title or this PR starts with the corresponding JIRA issue id
(e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the
endpoint declared according to the connector-specific documentation? *(N/A — no
object-storage changes)*
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)? *(N/A — no new
dependencies)*
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`,
`NOTICE-binary` files? *(N/A)*
### AI Tooling
This contribution was prepared with the assistance of Anthropic Claude Code:
the root-cause analysis, the validation change, the tests and this
description
were drafted with its assistance, and were reviewed, executed and verified
against the source by the contributor.
- [x] The PR includes the phrase "Contains content generated by <tool>"
where <tool> is the name of the AI tool used.
- [x] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
