[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r455608012
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##
@@ -111,6 +110,10 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
OMClientResponse omClientResponse = null;
Result result = null;
try {
+ keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap);
Review comment:
Opened https://issues.apache.org/jira/browse/HDDS-3971 for the
improvement.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r455460584
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##
@@ -111,6 +110,10 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
OMClientResponse omClientResponse = null;
Result result = null;
try {
+ keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap);
Review comment:
OK, I'll check if I can update the patch to address this.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454810305
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##
@@ -111,6 +110,10 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
OMClientResponse omClientResponse = null;
Result result = null;
try {
+ keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap);
Review comment:
> But once HA becomes the default, we can optimize this, as in HA there
is only a single thread executor.
Does that mean we can completely get rid of the locks?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454807157
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadAbortRequest.java
##
@@ -152,27 +158,29 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
omDoubleBufferHelper);
if (acquiredLock) {
-omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
-bucketName);
+omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK,
+keyArgs.getVolumeName(), keyArgs.getBucketName());
}
}
// audit log
auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
-OMAction.ABORT_MULTIPART_UPLOAD, buildKeyArgsAuditMap(keyArgs),
+OMAction.ABORT_MULTIPART_UPLOAD, auditMap,
exception, getOmRequest().getUserInfo()));
switch (result) {
case SUCCESS:
LOG.debug("Abort Multipart request is successfully completed for " +
- "KeyName {} in VolumeName/Bucket {}/{}", keyName, volumeName,
- bucketName);
+ "KeyName {} in VolumeName/Bucket {}/{}", keyName,
requestedVolume,
Review comment:
Updated, thanks.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454807058
##
File path: hadoop-ozone/dist/src/main/compose/ozone/test.sh
##
@@ -26,24 +26,24 @@ source "$COMPOSE_DIR/../testlib.sh"
start_docker_env
-#Due to the limitation of the current auditparser test, it should be the
-#first test in a clean cluster.
-
-#Disabling for now, audit parser tool during parse getting exception.
-#execute_robot_test om auditparser
-
execute_robot_test scm lib
+execute_robot_test scm ozone-lib
execute_robot_test scm basic
execute_robot_test scm gdpr
-execute_robot_test scm -v SCHEME:ofs ozonefs/ozonefs.robot
-execute_robot_test scm -v SCHEME:o3fs ozonefs/ozonefs.robot
+for scheme in ofs o3fs; do
+ for bucket in link bucket; do
+execute_robot_test scm -v SCHEME:${scheme} -v BUCKET_TYPE:${bucket}
ozonefs/ozonefs.robot
+ done
+done
execute_robot_test scm security/ozone-secure-token.robot
-execute_robot_test scm s3
+for bucket in link generated; do
+ execute_robot_test scm -v BUCKET:${bucket} s3
Review comment:
> S3 tests not using this bucket right?
Other tests do use bucket or link created by setup. Thanks for spotting
this instance, fixed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454782738
##
File path: hadoop-ozone/dist/src/main/smoketest/basic/links.robot
##
@@ -0,0 +1,152 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test bucket links via Ozone CLI
+Library OperatingSystem
+Resource../commonlib.robot
+Resource../ozone-lib/shell.robot
+Test Setup Run Keyword if'${SECURITY_ENABLED}' == 'true'Kinit
test user testuser testuser.keytab
+Test Timeout2 minute
+Suite Setup Create volumes
+
+*** Variables ***
+${prefix}generated
+
+*** Keywords ***
+Create volumes
+${random} = Generate Random String 5 [NUMBERS]
+Set Suite Variable ${source} ${random}-source
+Set Suite Variable ${target} ${random}-target
+Execute ozone sh volume create ${source}
+Execute ozone sh volume create ${target}
+Run Keyword if '${SECURITY_ENABLED}' == 'true'Setup ACL tests
+
+Setup ACL tests
+Execute ozone sh bucket create ${source}/readable-bucket
+Execute ozone sh key put
${source}/readable-bucket/key-in-readable-bucket /etc/passwd
+Execute ozone sh bucket create ${source}/unreadable-bucket
+Execute ozone sh bucket link ${source}/readable-bucket
${target}/readable-link
+Execute ozone sh bucket link ${source}/readable-bucket
${target}/unreadable-link
+Execute ozone sh bucket link ${source}/unreadable-bucket
${target}/link-to-unreadable-bucket
+Execute ozone sh volume addacl --acl
user:testuser2/s...@example.com:r ${target}
+Execute ozone sh volume addacl --acl
user:testuser2/s...@example.com:rl ${source}
+Execute ozone sh bucket addacl --acl
user:testuser2/s...@example.com:rl ${source}/readable-bucket
+Execute ozone sh bucket addacl --acl
user:testuser2/s...@example.com:r ${target}/readable-link
+Execute ozone sh bucket addacl --acl
user:testuser2/s...@example.com:r ${target}/link-to-unreadable-bucket
+
+Can follow link with read access
+Execute kdestroy
+Run Keyword Kinit test user testuser2
testuser2.keytab
+${result} = Execute And Ignore Errorozone sh key list
${target}/readable-link
+Should Contain ${result}
key-in-readable-bucket
+
+Cannot follow link without read access
+Execute kdestroy
+Run Keyword Kinit test user testuser2
testuser2.keytab
+${result} = Execute And Ignore Errorozone sh key list
${target}/unreadable-link
+Should Contain ${result}
PERMISSION_DENIED
+
+ACL verified on source bucket
+Execute kdestroy
+Run Keyword Kinit test user testuser2
testuser2.keytab
+${result} = Execute ozone sh bucket info
${target}/link-to-unreadable-bucket
+Should Contain ${result}
link-to-unreadable-bucket
+Should Not Contain ${result}
PERMISSION_DENIED
+${result} = Execute And Ignore Errorozone sh key list
${target}/link-to-unreadable-bucket
+Should Contain ${result}
PERMISSION_DENIED
+
+*** Test Cases ***
+Link to non-existent bucket
+Execute ozone sh bucket link
${source}/no-such-bucket ${target}/dangling-link
+${result} = Execute And Ignore Errorozone sh key list
${target}/dangling-link
+Should Contain ${result}
BUCKET_NOT_FOUND
+
+Key create passthrough
+Execute ozone sh bucket link
${source}/bucket1 ${target}/link1
+Execute ozone sh bucket create
${source}/bucket1
+Execute
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104: URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454782295 ## File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java ## @@ -85,10 +87,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OMMetrics omMetrics = ozoneManager.getMetrics(); omMetrics.incNumKeyDeletes(); -Map auditMap = null; String volumeName = deleteKeyArgs.getVolumeName(); String bucketName = deleteKeyArgs.getBucketName(); -String keyName = ""; +Map auditMap = new LinkedHashMap<>(); +auditMap.put(VOLUME, volumeName); Review comment: Similar to `volumeName` and `bucketName` variables, this is also required for the case when we encounter exception before reaching `bucket.audit()`. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454781508
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -3314,4 +3396,59 @@ private void startJVMPauseMonitor() {
jvmPauseMonitor.init(configuration);
jvmPauseMonitor.start();
}
+
+ public ResolvedBucket resolveBucketLink(KeyArgs args) throws IOException {
+return resolveBucketLink(
+Pair.of(args.getVolumeName(), args.getBucketName()));
+ }
+
+ public ResolvedBucket resolveBucketLink(OmKeyArgs args)
+ throws IOException {
+return resolveBucketLink(
+Pair.of(args.getVolumeName(), args.getBucketName()));
+ }
+
+ public ResolvedBucket resolveBucketLink(Pair requested)
+ throws IOException {
+Pair resolved =
+resolveBucketLink(requested, new HashSet<>());
+return new ResolvedBucket(requested, resolved);
+ }
+
+ /**
+ * Resolves bucket symlinks. Read permission is required for following links.
+ *
+ * @param volumeAndBucket the bucket to be resolved (if it is a link)
+ * @param visited collects link buckets visited during the resolution to
+ * avoid infinite loops
+ * @return bucket location possibly updated with its actual volume and bucket
+ * after following bucket links
+ * @throws IOException (most likely OMException) if ACL check fails, bucket
is
+ * not found, loop is detected in the links, etc.
+ */
+ private Pair resolveBucketLink(
+ Pair volumeAndBucket,
+ Set> visited) throws IOException {
+
+String volumeName = volumeAndBucket.getLeft();
+String bucketName = volumeAndBucket.getRight();
+OmBucketInfo info = bucketManager.getBucketInfo(volumeName, bucketName);
+if (!info.isLink()) {
+ return volumeAndBucket;
+}
+
+if (!visited.add(volumeAndBucket)) {
+ throw new OMException("Detected loop in bucket links", INTERNAL_ERROR);
Review comment:
Sure.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454781348
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##
@@ -88,21 +88,20 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
DeleteKeyRequest deleteKeyRequest = getOmRequest().getDeleteKeyRequest();
-OzoneManagerProtocolProtos.KeyArgs deleteKeyArgs =
+OzoneManagerProtocolProtos.KeyArgs keyArgs =
deleteKeyRequest.getKeyArgs();
+Map auditMap = buildKeyArgsAuditMap(keyArgs);
-String volumeName = deleteKeyArgs.getVolumeName();
-String bucketName = deleteKeyArgs.getBucketName();
-String keyName = deleteKeyArgs.getKeyName();
+String volumeName = keyArgs.getVolumeName();
Review comment:
> We can skip getting from original Args, as anyway final Volume/Bucket
we get is from resolvedBucket returned keyArgs.
Unfortunately not, because `volumeName` and `bucketName` are used later to
log result. If `resolveBucketLink` throws exception (eg. due to lack of
permission), then we exit from `try` earlier than getting `resolvedBucket` and
we need the original values from request.
https://github.com/apache/hadoop-ozone/blob/fbf04a33ecad2d23e8bd57ecceca4842a9cf5281/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java#L173-L183
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r452020792
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -2145,37 +2168,51 @@ public OmKeyLocationInfo allocateBlock(OmKeyArgs args,
long clientID,
*/
@Override
public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
+ResolvedBucket bucket = resolveBucketLink(args);
Review comment:
> I am not sure what is the expected behavior, how is this semantics
derived?
> Can we rely on underlying bucket acls, as anyway we verify already. Any
downside/security issue?
This matches unix symlinks permissions: if you have read access on the link,
you see where it points to and can follow it. ACL was discussed in design doc
update (#1009) and the current behavior was approved. Please check with @arp7
if we need any changes. Also pinging @ChenSammi for input.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447453885
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##
@@ -174,6 +175,12 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
Result result = null;
try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(keyArgs);
+ keyArgs = bucket.update(keyArgs);
Review comment:
Thanks, extracted to a method.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447383235
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java
##
@@ -136,54 +137,49 @@ public void createBucket(OmBucketInfo bucketInfo) throws
IOException {
throw new OMException("Bucket already exist",
OMException.ResultCodes.BUCKET_ALREADY_EXISTS);
}
+
BucketEncryptionKeyInfo bek = bucketInfo.getEncryptionKeyInfo();
- BucketEncryptionKeyInfo.Builder bekb = null;
- if (bek != null) {
-if (kmsProvider == null) {
- throw new OMException("Invalid KMS provider, check configuration " +
- CommonConfigurationKeys.HADOOP_SECURITY_KEY_PROVIDER_PATH,
- OMException.ResultCodes.INVALID_KMS_PROVIDER);
-}
-if (bek.getKeyName() == null) {
- throw new OMException("Bucket encryption key needed.", OMException
- .ResultCodes.BUCKET_ENCRYPTION_KEY_NOT_FOUND);
-}
-// Talk to KMS to retrieve the bucket encryption key info.
-KeyProvider.Metadata metadata = getKMSProvider().getMetadata(
-bek.getKeyName());
-if (metadata == null) {
- throw new OMException("Bucket encryption key " + bek.getKeyName()
- + " doesn't exist.",
- OMException.ResultCodes.BUCKET_ENCRYPTION_KEY_NOT_FOUND);
-}
-// If the provider supports pool for EDEKs, this will fill in the pool
-kmsProvider.warmUpEncryptedKeys(bek.getKeyName());
-bekb = new BucketEncryptionKeyInfo.Builder()
-.setKeyName(bek.getKeyName())
-.setVersion(CryptoProtocolVersion.ENCRYPTION_ZONES)
-.setSuite(CipherSuite.convert(metadata.getCipher()));
- }
- List acls = new ArrayList<>();
- acls.addAll(bucketInfo.getAcls());
- volumeArgs.getAclMap().getDefaultAclList().forEach(
- a -> acls.add(OzoneAcl.fromProtobufWithAccessType(a)));
-
- OmBucketInfo.Builder omBucketInfoBuilder = OmBucketInfo.newBuilder()
- .setVolumeName(bucketInfo.getVolumeName())
- .setBucketName(bucketInfo.getBucketName())
- .setAcls(acls)
- .setStorageType(bucketInfo.getStorageType())
- .setIsVersionEnabled(bucketInfo.getIsVersionEnabled())
- .setCreationTime(Time.now())
- .addAllMetadata(bucketInfo.getMetadata());
+
+ boolean hasSourceVolume = bucketInfo.getSourceVolume() != null;
+ boolean hasSourceBucket = bucketInfo.getSourceBucket() != null;
+
+ if (hasSourceBucket != hasSourceVolume) {
+throw new OMException("Both source volume and source bucket are " +
+"required for bucket links",
+OMException.ResultCodes.INVALID_REQUEST);
+ }
Review comment:
Thanks, I missed that. Will update the patch.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447378831
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
##
@@ -149,6 +150,12 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
List missingParentInfos;
try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(keyArgs);
+ keyArgs = bucket.update(keyArgs);
Review comment:
Audit log includes both.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447376939
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -2197,20 +2234,25 @@ public void renameKey(OmKeyArgs args, String toKeyName)
throws IOException {
*/
@Override
public void deleteKey(OmKeyArgs args) throws IOException {
+Map auditMap = args.toAuditMap();
Review comment:
I would have preferred removing old write code path before implementing
links to avoid duplicate work. However, now that these changes are in place, I
prefer keeping them, and removing old write code path completely in a separate
step.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447374977
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -2145,37 +2168,51 @@ public OmKeyLocationInfo allocateBlock(OmKeyArgs args,
long clientID,
*/
@Override
public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
+ResolvedBucket bucket = resolveBucketLink(args);
Review comment:
Read permission on the link is required to follow it. Do you propose to
completely skip ACL on link and allow anyone to use it?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447373492
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java
##
@@ -136,54 +137,49 @@ public void createBucket(OmBucketInfo bucketInfo) throws
IOException {
throw new OMException("Bucket already exist",
OMException.ResultCodes.BUCKET_ALREADY_EXISTS);
}
+
BucketEncryptionKeyInfo bek = bucketInfo.getEncryptionKeyInfo();
- BucketEncryptionKeyInfo.Builder bekb = null;
- if (bek != null) {
-if (kmsProvider == null) {
- throw new OMException("Invalid KMS provider, check configuration " +
- CommonConfigurationKeys.HADOOP_SECURITY_KEY_PROVIDER_PATH,
- OMException.ResultCodes.INVALID_KMS_PROVIDER);
-}
-if (bek.getKeyName() == null) {
- throw new OMException("Bucket encryption key needed.", OMException
- .ResultCodes.BUCKET_ENCRYPTION_KEY_NOT_FOUND);
-}
-// Talk to KMS to retrieve the bucket encryption key info.
-KeyProvider.Metadata metadata = getKMSProvider().getMetadata(
-bek.getKeyName());
-if (metadata == null) {
- throw new OMException("Bucket encryption key " + bek.getKeyName()
- + " doesn't exist.",
- OMException.ResultCodes.BUCKET_ENCRYPTION_KEY_NOT_FOUND);
-}
-// If the provider supports pool for EDEKs, this will fill in the pool
-kmsProvider.warmUpEncryptedKeys(bek.getKeyName());
-bekb = new BucketEncryptionKeyInfo.Builder()
-.setKeyName(bek.getKeyName())
-.setVersion(CryptoProtocolVersion.ENCRYPTION_ZONES)
-.setSuite(CipherSuite.convert(metadata.getCipher()));
- }
- List acls = new ArrayList<>();
- acls.addAll(bucketInfo.getAcls());
- volumeArgs.getAclMap().getDefaultAclList().forEach(
- a -> acls.add(OzoneAcl.fromProtobufWithAccessType(a)));
-
- OmBucketInfo.Builder omBucketInfoBuilder = OmBucketInfo.newBuilder()
- .setVolumeName(bucketInfo.getVolumeName())
- .setBucketName(bucketInfo.getBucketName())
- .setAcls(acls)
- .setStorageType(bucketInfo.getStorageType())
- .setIsVersionEnabled(bucketInfo.getIsVersionEnabled())
- .setCreationTime(Time.now())
- .addAllMetadata(bucketInfo.getMetadata());
+
+ boolean hasSourceVolume = bucketInfo.getSourceVolume() != null;
+ boolean hasSourceBucket = bucketInfo.getSourceBucket() != null;
+
+ if (hasSourceBucket != hasSourceVolume) {
+throw new OMException("Both source volume and source bucket are " +
+"required for bucket links",
+OMException.ResultCodes.INVALID_REQUEST);
+ }
+
+ if (bek != null && hasSourceBucket) {
+throw new OMException("Encryption cannot be set for bucket links",
+OMException.ResultCodes.INVALID_REQUEST);
+ }
+
+ BucketEncryptionKeyInfo.Builder bekb =
+ createBucketEncryptionKeyInfoBuilder(bek);
+
+ OmBucketInfo.Builder omBucketInfoBuilder = bucketInfo.toBuilder()
+ .setCreationTime(Time.now());
+
+ List defaultAclList =
Review comment:
Source bucket can be deleted any time after the link is created. We
would have to perform a reverse lookup to check if it leaves any dangling link.
Since this is not done, checking upon creation would be inconsistent.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
