szetszwo commented on code in PR #6089:
URL: https://github.com/apache/ozone/pull/6089#discussion_r1466620715
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/S3InMemoryCache.java:
##########
@@ -45,12 +45,9 @@ public void put(String id, S3SecretValue secretValue) {
@Override
public void invalidate(String id) {
S3SecretValue secret = cache.getIfPresent(id);
- if (secret == null) {
- return;
+ if (secret != null) {
+ cache.put(id, secret.deleted());
}
- secret.setDeleted(true);
- secret.setAwsSecret(null);
- cache.put(id, secret);
Review Comment:
Use `computeIfPresent`:
```java
cache.asMap().computeIfPresent(id, (k, secret) -> secret.deleted());
```
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3SecretValue.java:
##########
@@ -38,16 +38,29 @@ public static Codec<S3SecretValue> getCodec() {
}
// TODO: This field should be renamed to accessId for generalization.
- private String kerberosID;
- private String awsSecret;
- private boolean isDeleted;
- private long transactionLogIndex;
+ private final String kerberosID;
+ private final String awsSecret;
+ private final boolean isDeleted;
+ private final long transactionLogIndex;
- public S3SecretValue(String kerberosID, String awsSecret) {
- this(kerberosID, awsSecret, false, 0L);
+ public static S3SecretValue of(String kerberosID, String awsSecret) {
+ return of(kerberosID, awsSecret, 0);
}
- public S3SecretValue(String kerberosID, String awsSecret, boolean isDeleted,
+ public static S3SecretValue of(String kerberosID, String awsSecret, long
transactionLogIndex) {
+ return new S3SecretValue(
+ Objects.requireNonNull(kerberosID),
+ Objects.requireNonNull(awsSecret),
+ false,
+ transactionLogIndex
+ );
+ }
+
+ public S3SecretValue deleted() {
+ return new S3SecretValue(kerberosID, awsSecret, true, transactionLogIndex);
Review Comment:
Pass an empty string for `awsSecret`?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/S3SecretValue.java:
##########
@@ -38,16 +38,29 @@ public static Codec<S3SecretValue> getCodec() {
}
// TODO: This field should be renamed to accessId for generalization.
- private String kerberosID;
- private String awsSecret;
- private boolean isDeleted;
- private long transactionLogIndex;
+ private final String kerberosID;
+ private final String awsSecret;
Review Comment:
An unrelated comment: It is not a good idea store a secret in a plaintext
`String` and write it to db.
The usual practice is to encrypt it in `byte[]` and erase it after use.
--
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]
