kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

Consider not using `UUID.ramdomUUID()`

Open ikhoon opened this issue 1 year ago • 3 comments

Describe the bug

Problem

Armeria uses https://github.com/reactor/BlockHound to detect blocking IO on non-blocking threads such as event loops. BlockHound agent reported UUID.ramdomUUID() as an improper usage. Because SecureRandom used in UUID.ramdomUUID() triggers blocking operations.

java.lang.Exception: java.io.FileInputStream#readBytes
	at com.linecorp.armeria.internal.testing.InternalTestingBlockHoundIntegration.writeBlockingMethod(InternalTestingBlockHoundIntegration.java:84)
	at reactor.blockhound.BlockHound$Builder.lambda$install$8(BlockHound.java:472)
	at reactor.blockhound.BlockHoundRuntime.checkBlocking(BlockHoundRuntime.java:89)
	at java.base/java.io.FileInputStream.readBytes(FileInputStream.java)
	at java.base/java.io.FileInputStream.read(FileInputStream.java:293)
	at java.base/java.io.FilterInputStream.read(FilterInputStream.java:119)
	at java.base/sun.security.provider.NativePRNG$RandomIO.readFully(NativePRNG.java:425)
	at java.base/sun.security.provider.NativePRNG$RandomIO.ensureBufferValid(NativePRNG.java:528)
	at java.base/sun.security.provider.NativePRNG$RandomIO.implNextBytes(NativePRNG.java:547)
	at java.base/sun.security.provider.NativePRNG.engineNextBytes(NativePRNG.java:221)
	at java.base/java.security.SecureRandom.nextBytes(SecureRandom.java:758)
	at java.base/java.util.UUID.randomUUID(UUID.java:151)
	at io.fabric8.kubernetes.client.http.StandardHttpRequest.<init>(StandardHttpRequest.java:116)
	at io.fabric8.kubernetes.client.http.StandardHttpRequest$Builder.build(StandardHttpRequest.java:201)

If the UUID is used for logging and tracing, I don't think a SecureRandom is genuinely necessary.

Suggestion

I propose to directly create UUID with a random variable or an atomic value. For example:

  • https://github.com/apache/logging-log4j2/blob/035e251056f0d2d77be7290e954c45862042e284/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java#L142
  • https://github.com/spring-projects/spring-framework/blob/a4db0e7448287028d228d46fe7b4df202150958a/spring-core/src/main/java/org/springframework/util/SimpleIdGenerator.java#L36

Fabric8 Kubernetes Client version

SNAPSHOT

Steps to reproduce

Create StandardHttpRequest

Expected behavior

Not to use SecureRandom.

Runtime

minikube

Kubernetes API Server version

1.25.3@latest

Environment

Linux

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

ikhoon avatar Jan 31 '24 09:01 ikhoon

I think it's only used to create simple IDs, a solution such as the one Spring Framework uses should definitely work.

manusa avatar Jan 31 '24 09:01 manusa

We can also consider just removing / deprecating from StandardHttpRequest - it's not currently used because we changed the way the logging logic worked.

shawkins avatar Jan 31 '24 11:01 shawkins

It's used in multiple places (besides StandardHttpRequest) for similar purposes.

(PostHandler, PodUpload, and also on NamespaceExtension, WebSocketSession)

manusa avatar Jan 31 '24 11:01 manusa