hemantk-12 commented on code in PR #7434:
URL: https://github.com/apache/ozone/pull/7434#discussion_r1844382771
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotFileSystem.java:
##########
@@ -107,26 +111,29 @@ public abstract class TestOmSnapshotFileSystem {
protected static final String BUCKET_NAME_LEGACY =
"bucket-legacy-" + RandomStringUtils.randomNumeric(5);
- private static MiniOzoneCluster cluster = null;
- private static OzoneClient client;
- private static ObjectStore objectStore;
- private static OzoneConfiguration conf;
- private static OzoneManagerProtocol writeClient;
- private static OzoneManager ozoneManager;
- private static String keyPrefix;
+ private MiniOzoneCluster cluster = null;
+ private OzoneClient client;
+ private ObjectStore objectStore;
+ private OzoneConfiguration conf;
+ private OzoneManagerProtocol writeClient;
+ private OzoneManager ozoneManager;
+ private String keyPrefix;
private final String bucketName;
+ private final boolean createLinkedBuckets;
private FileSystem fs;
private OzoneFileSystem o3fs;
+ private Map<String, String> linkedBucketMaps = new HashMap<>();
Review Comment:
Do we need to keep it as a map? `Optional` is a better option here if it
contains only one key-value.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2968,15 +2968,18 @@ public ListKeysLightResult listKeysLight(String
volumeName,
public SnapshotInfo getSnapshotInfo(String volumeName, String bucketName,
String snapshotName) throws IOException {
metrics.incNumSnapshotInfos();
+ // Updating the volumeName & bucketName in case the bucket is a linked
bucket. We need to do this before a
+ // permission check, since linked bucket permissions and source bucket
permissions could be different.
Review Comment:
nit: please remove this.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2997,12 +3000,17 @@ public ListSnapshotResponse listSnapshot(
Map<String, String> auditMap = buildAuditMap(volumeName);
auditMap.put(OzoneConsts.BUCKET, bucketName);
try {
+ // Updating the volumeName & bucketName in case the bucket is a linked
bucket. We need to do this before a
+ // permission check, since linked bucket permissions and source bucket
permissions could be different.
+ ResolvedBucket resolvedBucket = resolveBucketLink(Pair.of(volumeName,
bucketName), false);
+ auditMap = buildAuditMap(resolvedBucket.realVolume());
Review Comment:
We don't keep audit logs for read operations. It might been copied-pasted
from the write API. If agree, we can create a clean jira.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -2997,12 +3000,17 @@ public ListSnapshotResponse listSnapshot(
Map<String, String> auditMap = buildAuditMap(volumeName);
auditMap.put(OzoneConsts.BUCKET, bucketName);
try {
+ // Updating the volumeName & bucketName in case the bucket is a linked
bucket. We need to do this before a
+ // permission check, since linked bucket permissions and source bucket
permissions could be different.
+ ResolvedBucket resolvedBucket = resolveBucketLink(Pair.of(volumeName,
bucketName), false);
Review Comment:
nit: you can call
[resolveBucketLink](https://github.com/apache/ozone/blob/516f0a3747bb68f530053c3b6f1fbc854aaca7e9/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L4371)
which is default to `false`.
--
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]
