You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/21 17:06:49 UTC
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1104: HDDS-3612. Allow mounting bucket under other volume
adoroszlai opened a new pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104
## What changes were proposed in this pull request?
Implement [bucket links](https://github.com/apache/hadoop-ozone/blob/master/hadoop-hdds/docs/content/design/ozone-volume-management.md#implementation-details).
#### Implementation details (from the design doc)
* `ozone sh bucket link` operation creates a link bucket. Links are like regular buckets, stored in DB the same way, but with two new, optional pieces of information: source volume and bucket. (The bucket being referenced by the link is called "source", not "target", to follow symlink terminology.)
* Link buckets share the namespace with regular buckets. If a bucket or link with the same name already exists, a `BUCKET_ALREADY_EXISTS` result is returned.
* (Unlike previous per-user S3 volume mapping) Link buckets are not inherently specific to a user, access is restricted only by ACL.
* Links are persistent, ie. they can be used until they are deleted.
* Existing bucket operations (info, delete, ACL) work on the link object in the same way as they do on regular buckets. No new link-specific RPC is required.
* Links are followed for key operations (list, get, put, etc.). Read permission on the link is required for this.
* Checks for existence of the source bucket, as well as ACL, are performed only when following the link (similar to symlinks). Source bucket is not checked when operating on the link bucket itself (eg. deleting it). This avoids the need for reverse checks for each bucket delete or ACL change.
* Bucket links are generic, not restricted to the `s3v` volume.
#### Audit
Audit message for link creation mentions the bucket to which the link points (`sourceVolume` and `sourceBucket`):
```
OMAudit | ... | op=CREATE_BUCKET {volume=60748-target, bucket=link1, gdprEnabled=null, acls=[user:hadoop:a[ACCESS], group:users:a[ACCESS]], isVersionEnabled=false, storageType=DISK, creationTime=1592631444545, bucketEncryptionKey=null, sourceVolume=60748-source, sourceBucket=bucket1} | ret=SUCCESS |
```
As well as audit messages for key operations issued on links:
```
OMAudit | ... | op=ALLOCATE_KEY {volume=60748-target, bucket=link1, key=key1, dataSize=671, replicationType=RATIS, replicationFactor=THREE, sourceVolume=60748-source, sourceBucket=bucket1} | ret=SUCCESS |
OMAudit | ... | op=DELETE_KEY {volume=60748-target, bucket=link1, key=key2, dataSize=0, replicationType=RATIS, replicationFactor=ONE, sourceVolume=60748-source, sourceBucket=bucket1} | ret=SUCCESS |
```
Response for `ALLOCATE_KEY` contains the real (source) volume/bucket, so `COMMIT_KEY` does not know about the link (but it is prepared to handle it anyway):
```
OMAudit | ... | op=COMMIT_KEY {volume=60748-source, bucket=bucket1, key=key1, dataSize=671, replicationType=RATIS, replicationFactor=ONE} | ret=SUCCESS |
```
https://issues.apache.org/jira/browse/HDDS-3612
## How was this patch tested?
Added acceptance tests for testing bucket links via Ozone Shell.
Slightly tweaked existing S3 acceptance tests are executed on both normal buckets and bucket links.
Existing Ozone FS acceptance tests (reorganized in #1093) are also executed on links.
https://github.com/adoroszlai/hadoop-ozone/runs/792817258
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi merged pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
ChenSammi merged pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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 Timeout 2 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/scm@EXAMPLE.COM:r ${target}
+ Execute ozone sh volume addacl --acl user:testuser2/scm@EXAMPLE.COM:rl ${source}
+ Execute ozone sh bucket addacl --acl user:testuser2/scm@EXAMPLE.COM:rl ${source}/readable-bucket
+ Execute ozone sh bucket addacl --acl user:testuser2/scm@EXAMPLE.COM:r ${target}/readable-link
+ Execute ozone sh bucket addacl --acl user:testuser2/scm@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 Error ozone 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 Error ozone 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 Error ozone 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 Error ozone 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 ozone sh key put ${target}/link1/key1 /etc/passwd
+ Key Should Match Local File ${target}/link1/key1 /etc/passwd
+
+Key read passthrough
+ Execute ozone sh key put ${source}/bucket1/key2 /opt/hadoop/NOTICE.txt
+ Key Should Match Local File ${source}/bucket1/key2 /opt/hadoop/NOTICE.txt
+
+Key list passthrough
+ ${target_list} = Execute ozone sh key list ${target}/link1 | jq -r '.name'
+ ${source_list} = Execute ozone sh key list ${source}/bucket1 | jq -r '.name'
+ Should Be Equal ${target_list} ${source_list}
+ Should Contain ${source_list} key1
+ Should Contain ${source_list} key2
+
+Key delete passthrough
+ Execute ozone sh key delete ${target}/link1/key2
+ ${source_list} = Execute ozone sh key list ${source}/bucket1 | jq -r '.name'
+ Should Not Contain ${source_list} key2
+
+Bucket list contains links
+ ${result} = Execute ozone sh bucket list ${target}
+ Should Contain ${result} link1
+ Should Contain ${result} dangling-link
+
+Bucket info shows source
+ ${result} = Execute ozone sh bucket info ${target}/link1 | jq -r '.sourceVolume, .sourceBucket' | xargs
+ Should Be Equal ${result} ${source} bucket1
+
+Source and target have separate ACLs
+ Execute ozone sh bucket addacl --acl user:user1:rwxy ${target}/link1
+ Verify ACL bucket ${target}/link1 USER user1 READ WRITE READ_ACL WRITE_ACL
+ Verify ACL bucket ${source}/bucket1 USER user1 ${EMPTY}
+
+ Execute ozone sh bucket addacl --acl group:group2:r ${source}/bucket1
+ Verify ACL bucket ${target}/link1 GROUP group2 ${EMPTY}
+ Verify ACL bucket ${source}/bucket1 GROUP group2 READ
+
+Buckets and links share namespace
+ Execute ozone sh bucket link ${source}/bucket2 ${target}/link2
+ ${result} = Execute And Ignore Error ozone sh bucket create ${target}/link2
+ Should Contain ${result} BUCKET_ALREADY_EXISTS
+
+ Execute ozone sh bucket create ${target}/bucket3
+ ${result} = Execute And Ignore Error ozone sh bucket link ${source}/bucket1 ${target}/bucket3
+ Should Contain ${result} BUCKET_ALREADY_EXISTS
+
+Can follow link with read access
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Can follow link with read access
+
+Cannot follow link without read access
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Cannot follow link without read access
+
+ACL verified on source bucket
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' ACL verified on source bucket
+
+Loop in link chain is detected
+ Execute ozone sh bucket link ${target}/loop1 ${target}/loop2
+ Execute ozone sh bucket link ${target}/loop2 ${target}/loop3
+ Execute ozone sh bucket link ${target}/loop3 ${target}/loop1
+ ${result} = Execute And Ignore Error ozone sh key list ${target}/loop2
+ Should Contain ${result} INTERNAL_ERROR
Review comment:
Done.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-commenter commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-651585829
# [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=h1) Report
> Merging [#1104](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/0f2a11827db011eb9902adcf867cbe8f6af5116c&el=desc) will **increase** coverage by `0.16%`.
> The diff coverage is `77.01%`.
[](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1104 +/- ##
============================================
+ Coverage 73.32% 73.49% +0.16%
- Complexity 9953 10002 +49
============================================
Files 969 971 +2
Lines 49413 49613 +200
Branches 4839 4843 +4
============================================
+ Hits 36231 36461 +230
+ Misses 10879 10856 -23
+ Partials 2303 2296 -7
```
| [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...main/java/org/apache/hadoop/ozone/OzoneConsts.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvT3pvbmVDb25zdHMuamF2YQ==) | `84.21% <ø> (ø)` | `1.00 <0.00> (ø)` | |
| [...n/java/org/apache/hadoop/ozone/audit/OMAction.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2F1ZGl0L09NQWN0aW9uLmphdmE=) | `100.00% <ø> (ø)` | `2.00 <0.00> (ø)` | |
| [...doop/ozone/om/helpers/BucketEncryptionKeyInfo.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL29tL2hlbHBlcnMvQnVja2V0RW5jcnlwdGlvbktleUluZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [.../org/apache/hadoop/ozone/om/BucketManagerImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9CdWNrZXRNYW5hZ2VySW1wbC5qYXZh) | `16.03% <0.00%> (-0.45%)` | `8.00 <0.00> (ø)` | |
| [...ozone/om/request/bucket/OMBucketCreateRequest.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9yZXF1ZXN0L2J1Y2tldC9PTUJ1Y2tldENyZWF0ZVJlcXVlc3QuamF2YQ==) | `78.15% <0.00%> (-4.15%)` | `12.00 <0.00> (ø)` | |
| [...op/ozone/om/request/key/OMTrashRecoverRequest.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9yZXF1ZXN0L2tleS9PTVRyYXNoUmVjb3ZlclJlcXVlc3QuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...ache/hadoop/ozone/shell/bucket/BucketCommands.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL3Rvb2xzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvc2hlbGwvYnVja2V0L0J1Y2tldENvbW1hbmRzLmphdmE=) | `60.00% <ø> (ø)` | `3.00 <0.00> (ø)` | |
| [.../java/org/apache/hadoop/ozone/om/OzoneManager.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9Pem9uZU1hbmFnZXIuamF2YQ==) | `68.50% <61.32%> (+1.34%)` | `201.00 <31.00> (+2.00)` | |
| [...e/hadoop/ozone/shell/bucket/LinkBucketHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL3Rvb2xzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvc2hlbGwvYnVja2V0L0xpbmtCdWNrZXRIYW5kbGVyLmphdmE=) | `80.00% <80.00%> (ø)` | `3.00 <3.00> (?)` | |
| [...g/apache/hadoop/ozone/om/helpers/OmBucketInfo.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL29tL2hlbHBlcnMvT21CdWNrZXRJbmZvLmphdmE=) | `83.52% <84.61%> (+1.09%)` | `33.00 <10.00> (+10.00)` | |
| ... and [47 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1104/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=footer). Last update [0f2a118...30fe042](https://codecov.io/gh/apache/hadoop-ozone/pull/1104?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-651495214
> Question: Now when list at the root "/" with ofs filesystem, for link buckets they will be displayed as a new entry of only resolved buckets will be shown. What should be the behavior here.
I'm afraid I don't understand the question. Can you please give an example?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447312649
##########
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:
Looks like this logic is needed for all KeyRequests. Can we move this to a common method, instead of duplicating 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447302394
##########
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<String, String> auditMap = args.toAuditMap();
Review comment:
The same comment for all write requests:
Can we remove the changes from old write code path, which is not required? It is unnecessary now.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447313113
##########
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<OmKeyInfo> missingParentInfos;
try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(keyArgs);
+ keyArgs = bucket.update(keyArgs);
Review comment:
And also do you think it would be useful to log actual bucket/volume request, and then resolvedBucket/resolvedVolume instead of just updating volumeName/bucketName directly.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<String, String> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r455147202
##########
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:
we cannot completely get rid of locks, as there are readers. But there will ne no parallel writers to have a situation like in non-HA, where other writer thread can acquire a lock in between
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447291849
##########
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<OzoneAcl> 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<OzoneManagerProtocolProtos.OzoneAclInfo> defaultAclList =
Review comment:
Question: No, where we checked source volume/bucket exists or not.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659501553
> > > > When deleting a bucket, we check bucket is empty or not. Do we need to check resolvedBucket is empty or not?
> > >
> > >
> > > Link can be deleted regardless of the existence or emptiness of the real bucket it points to. So no need to check resolved bucket.
> >
> >
> > This is the semantics for link buckets, because this is not a delete link, it is same delete bucket op which OM has received.
>
> The same "delete bucket" op works for deleting links. Since links cannot have direct entries, the emptiness check will not find any, hence delete is always allowed. We don't resolve the bucket the link points to, because its emptiness does not affect whether we can delete the link.
>
> Please let me know if I misunderstood the question.
Makes sense to me. Thanks for the explanation.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-654594774
> > Question: Now when list at the root "/" with ofs filesystem, for link buckets they will be displayed as a new entry of only resolved buckets will be shown. What should be the behavior here.
>
> I'm afraid I don't understand the question. Can you please give an example?
when I perform ls on volume/bucket, which has below buckets. It will show only resolved buckets or all the buckets? Or it follows normal ls semantics?
if it has mounted buckets
b1 -> b2
b2
b4 -> b5
what should be the behavior of ls in this case?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659108416
> > > When deleting a bucket, we check bucket is empty or not. Do we need to check resolvedBucket is empty or not?
> >
> > Link can be deleted regardless of the existence or emptiness of the real bucket it points to. So no need to check resolved bucket.
>
> This is the semantics for link buckets, because this is not a delete link, it is same delete bucket op which OM has received.
The same "delete bucket" op works for deleting links. Since links cannot have direct entries, the emptiness check will not find any, hence delete is always allowed. We don't resolve the bucket the link points to, because its emptiness does not affect whether we can delete the link.
Please let me know if I misunderstood the question.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai edited a comment on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai edited a comment on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659220643
> I did some simple tests locally and looks good.
Thanks @ChenSammi for that.
> 1. When the source bucket is deleted, bucket link still exists. It's an expected behavior, right?
Yes.
> 2. (Unlike previous per-user S3 volume mapping) Link buckets are not inherently specific to a user, access is restricted only by ACL. Not very clear about what it means. would you explain it further?
Previously each user had its own S3 volume and only by sharing the AWS access key could others use it. Now if you create a bucket link, other users can access it as long as ACL permits (similar to other Ozone objects).
> Can we set different ACL on source bucket and bucket link?
Yes. When following links eg. to create a key, only read permission is checked on the link. The source bucket is still checked for the permission required for the specific operation (eg. key list, create, or delete).
> 3. We're working on Volume quota implementation. I suppose bucket link will require 0 quota from it's parent volume, only the source bucket size will be controlled by it's volume's quota, right?
I'm not familiar with quotas, but I guess links should work like empty buckets in this respect. Keys are created in the source bucket, so they should not affect space usage in the link's volume.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 edited a comment on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658868821
> Thanks @bharatviswa504 for the review. I have updated the patch based on your comments.
>
> > When deleting a bucket, we check bucket is empty or not. Do we need to check resolvedBucket is empty or not?
>
> Link can be deleted regardless of the existence or emptiness of the real bucket it points to. So no need to check resolved bucket.
This is the semantics for link buckets, because this is not a delete link, it is same delete bucket op which OM has received.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659515813
> I have one suggestion, can we make this mount configurable option, as this is required only when old S3 buckets needs to be exposed via O3fs/ofs. In this way, we can avoid extra bucket checks. (I know that it is in-memory cache, but still I feel if some one does not require this feature, it is totally disabled). I think we can make that check in 2 places. One in create bucket/and other in resolveBucketLink.
>
> Fine to do in another Jira also.
I take back this comment. This feature helps to expose buckets created via shell/ofs to S3. So making it configurable is not needed. (As by default if user decides to expose any bucket created via shell, it can be exposed without any restart.)
Thank You @arp7 for the offline discussion.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<String, String> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 edited a comment on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659066783
@adoroszlai Any comments on left over 2 questions.
One related to delete bucket and other multiple times of validation of bucket.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<OzoneAcl> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-651426116
Question: Now when list at the root "/" with ofs filesystem, for link buckets they will be displayed as a new entry of only resolved buckets will be shown. What should be the behavior here.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<OmKeyInfo> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658282794
@adoroszlai This patch needs a rebase. (This has been due to one of my commit only).
I will take a final look at it today, and will post my comments.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659518247
> I take back this comment. This feature helps to expose buckets created via shell/ofs to S3. So making it configurable is not needed. (As by default if user decides to expose any bucket created via shell, it can be exposed without any restart.)
>
> Thank You @arp7 for the offline discussion.
OK.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659517803
> I have one suggestion, can we make this mount configurable option, as this is required only when old S3 buckets needs to be exposed via O3fs/ofs. In this way, we can avoid extra bucket checks. (I know that it is in-memory cache, but still I feel if some one does not require this feature, it is totally disabled). I think we can make that check in 2 places. One in create bucket/and other in resolveBucketLink.
>
> Fine to do in another Jira also.
Thanks for the suggestion, sounds good to me. I think if we want to disable it by default, then it should be done as part of this task. If it's enabled by default, then it's fine to add the config later in separate task. Let me know what should be the default value. CC @arp7 @elek
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-654620400
> > > Question: Now when list at the root "/" with ofs filesystem, for link buckets they will be displayed as a new entry of only resolved buckets will be shown. What should be the behavior here.
> >
> >
> > I'm afraid I don't understand the question. Can you please give an example?
>
> when user perform `ls` on volume/bucket, which has below buckets. It will show only resolved buckets or all the buckets? Or it follows normal ls semantics?
>
> if it has mounted buckets
>
> b1 -> b2
> b2
> b4 -> b5
>
> what should be the behavior of ls in this case?
Thanks for the clarification. `ls` on volume shows buckets and links in that volume, will not resolve links. `ls` on bucket shows the keys in that bucket. `ls` on link shows keys in the bucket that the link points to.
Example created by the robot test:
```
$ ozone fs -ls ofs://om/63433-target/
Found 7 items
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:50 ofs://om/63433-target/bucket3
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:49 ofs://om/63433-target/dangling-link
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:50 ofs://om/63433-target/link2
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/link3
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/loop1
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/loop2
drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/loop3
```
```
$ ozone fs -ls ofs://om/63433-source/bucket1
Found 2 items
-rw-rw-rw- 3 hadoop hadoop 671 2020-07-07 05:49 ofs://om/63433-source/bucket1/key1
-rw-rw-rw- 3 hadoop hadoop 372 2020-07-07 05:51 ofs://om/63433-source/bucket1/key3
```
```
$ ozone fs -ls ofs://om/63433-target/link3
Found 2 items
-rw-rw-rw- 3 hadoop hadoop 671 2020-07-07 05:49 ofs://om/63433-target/link3/key1
-rw-rw-rw- 3 hadoop hadoop 372 2020-07-07 05:51 ofs://om/63433-target/link3/key3
```
```
$ ozone sh bucket info /63433-target/link3
{
"volumeName" : "63433-target",
"name" : "link3",
...
"sourceVolume" : "63433-source",
"sourceBucket" : "bucket1"
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659229040
>
>
> > I did some simple tests locally and looks good.
>
> Thanks @ChenSammi for that.
>
> > 1. When the source bucket is deleted, bucket link still exists. It's an expected behavior, right?
>
> Yes.
>
> > 1. (Unlike previous per-user S3 volume mapping) Link buckets are not inherently specific to a user, access is restricted only by ACL. Not very clear about what it means. would you explain it further?
>
> Previously each user had its own S3 volume and only by sharing the AWS access key could others use it. Now if you create a bucket link, other users can access it as long as ACL permits (similar to other Ozone objects).
>
> > Can we set different ACL on source bucket and bucket link?
>
> Yes. When following links eg. to create a key, only read permission is checked on the link. The source bucket is still checked for the permission required for the specific operation (eg. key list, create, or delete).
>
> > 1. We're working on Volume quota implementation. I suppose bucket link will require 0 quota from it's parent volume, only the source bucket size will be controlled by it's volume's quota, right?
>
> I'm not familiar with quotas, but I guess links should work like empty buckets in this respect. Keys are created in the source bucket, so they should not affect space usage in the link's volume.
@adoroszlai , thanks for the explanation.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659831676
Thanks @adoroszlai for the contribution and @bharatviswa504 for the review.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447301716
##########
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:
Now for each operation if it is link bucket where if it has sourceVolume/SourceBucket we do checkAcls twice. One with READ permission on sourceBucket/SourceVolume. It is not clear why do we need that.
If it requires, do you think we need to have an API where we can check all required ACLS with a single checkAcl call?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447311657
##########
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:
Okay, I see is in resolveBucketLink we check Acls for read on provided bucket, and in actual request check acls on sourceBucket/sourceVolume which it is resolved to. Why do we need 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r451876665
##########
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<OzoneAcl> 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<OzoneManagerProtocolProtos.OzoneAclInfo> defaultAclList =
Review comment:
Okay error seems to be confusing because the link bucket is happening through create bucket, with parameters sourcevolume/sourceBucket.
I am not sure if someone will use our direct API's OzoneBucket/RpcClient.
But the explanation makes sense to me.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 edited a comment on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658904714
Overall LGTM.
Few minor comments/questions, once those are addressed good to go.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r455199679
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -172,6 +172,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
IOException exception = null;
try {
+ keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap);
+ volumeName = keyArgs.getVolumeName();
+ bucketName = keyArgs.getBucketName();
Review comment:
openKeyName should be generated from resolveBucketLink.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447313113
##########
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<OmKeyInfo> missingParentInfos;
try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(keyArgs);
+ keyArgs = bucket.update(keyArgs);
Review comment:
And also do you think it would be useful to log the actual bucket/volume request which it resolved also?
This might help during debug purposes.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r449449837
##########
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<OzoneAcl> 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<OzoneManagerProtocolProtos.OzoneAclInfo> defaultAclList =
Review comment:
This is the same as the behavior of sym links:
```
mkdir source
ln -s source dest
rm r source
```
Now you have a wrong pointer. You can list the content of the directory, but when you try to create a fie it will fail.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447318712
##########
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<OzoneAcl> 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<OzoneManagerProtocolProtos.OzoneAclInfo> defaultAclList =
Review comment:
Reading more understood that if we create link for /vol1/buck1 -> /vol2/buck2 (source)
We create in DB /vol1/buck1 and they have sourceVolume as vol2 and sourceBucket as buck2.
Now, when someone calls lookupKey on unresolved bucket, during actual request of lookupKey, this will result in Bucket_NOT_FOUND. Do you think, we need to make sure that source volume/source bucket exists during link creation to avoid such scenarios?
My reason was this looks strange, the user thinks he created a link bucket with some source volume/source bucket that has passed without any issues, but now when creating key it is saying bucket does not exist.
Following ln -s <source> <dest> looks confusing in our scenario.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r455147202
##########
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:
we cannot completely get rid of locks, as there are readers.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447279299
##########
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<OzoneAcl> 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:
Old write code is not being used anymore.
This logic needs to be added to new Class OMBucketCreateRequest.java
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659066783
@adoroszlai Any comments on left over 2 questions.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-651590619
Thanks @bharatviswa504 for the review. I have updated the patch based on your code comments.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447301716
##########
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:
Now for each operation if it is link bucket where if it has sourceVolume/SourceBucket we do checkAcls twice. One with READ permission on sourceBucket/SourceVolume in resoleBucketLink and one in actual request with required ACL type. It is not clear why do we need that.
If it requires, do you think we need to have an API where we can check all required ACLS with a single checkAcl call?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r452921889
##########
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:
> This matches unix symlinks permissions: if you have read access on the link, you see where it points to and can follow it.
+1
##########
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:
> This matches unix symlinks permissions: if you have read access on the link, you see where it points to and can follow it.
+1
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658904714
Overall LGTM.
Few minor comments, once those are addressed good to go.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447318712
##########
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<OzoneAcl> 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<OzoneManagerProtocolProtos.OzoneAclInfo> defaultAclList =
Review comment:
Reading more understood that if we create link for /vol1/buck1 -> /vol2/buck2 (source)
We create in DB /vol1/buck1 and they have sourceVolume as vol2 and sourceBucket as buck2.
Now, when someone calls lookupKey on unresolved bucket, during actual request of lookupKey, this will result in Bucket_NOT_FOUND. Do you think, we need to make sure that source volume/source bucket exists during link creation to avoid such scenarios?
My reason was this looks strange, the user thinks he created a link bucket with some source volume/source bucket that has passed without any issues, but now when creating key it is saying bucket does not exist.
Following ln -s <<source>> <<dest>> looks confusing in our scenario.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<OzoneAcl> 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<OzoneManagerProtocolProtos.OzoneAclInfo> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<String, String> auditMap = null;
String volumeName = deleteKeyArgs.getVolumeName();
String bucketName = deleteKeyArgs.getBucketName();
- String keyName = "";
+ Map<String, String> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-655806776
> > > > Question: Now when list at the root "/" with ofs filesystem, for link buckets they will be displayed as a new entry of only resolved buckets will be shown. What should be the behavior here.
> > >
> > >
> > > I'm afraid I don't understand the question. Can you please give an example?
> >
> >
> > when user perform `ls` on volume/bucket, which has below buckets. It will show only resolved buckets or all the buckets? Or it follows normal ls semantics?
> > if it has mounted buckets
> > b1 -> b2
> > b2
> > b4 -> b5
> > what should be the behavior of ls in this case?
>
> Thanks for the clarification. `ls` on volume shows buckets and links in that volume, will not resolve links. `ls` on bucket shows the keys in that bucket. `ls` on link shows keys in the bucket that the link points to.
>
> Example created by the robot test:
>
> ```
> $ ozone fs -ls ofs://om/63433-target/
> Found 7 items
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:50 ofs://om/63433-target/bucket3
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:49 ofs://om/63433-target/dangling-link
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:50 ofs://om/63433-target/link2
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/link3
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/loop1
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/loop2
> drwxrwxrwx - hadoop hadoop 0 2020-07-07 05:51 ofs://om/63433-target/loop3
> ```
>
> ```
> $ ozone fs -ls ofs://om/63433-source/bucket1
> Found 2 items
> -rw-rw-rw- 3 hadoop hadoop 671 2020-07-07 05:49 ofs://om/63433-source/bucket1/key1
> -rw-rw-rw- 3 hadoop hadoop 372 2020-07-07 05:51 ofs://om/63433-source/bucket1/key3
> ```
>
> ```
> $ ozone fs -ls ofs://om/63433-target/link3
> Found 2 items
> -rw-rw-rw- 3 hadoop hadoop 671 2020-07-07 05:49 ofs://om/63433-target/link3/key1
> -rw-rw-rw- 3 hadoop hadoop 372 2020-07-07 05:51 ofs://om/63433-target/link3/key3
> ```
>
> ```
> $ ozone sh bucket info /63433-target/link3
> {
> "volumeName" : "63433-target",
> "name" : "link3",
> ...
> "sourceVolume" : "63433-source",
> "sourceBucket" : "bucket1"
> }
> ```
Thanks for your clarification. Even ls on the filesystem is same behavior it shows link also. I think we should be fine here.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658561325
Thanks @bharatviswa504 for the review. I have updated the patch based on your comments.
> When deleting a bucket, we check bucket is empty or not. Do we need to check resolvedBucket is empty or not?
Link can be deleted regardless of the existence or emptiness of the real bucket it points to. So no need to check resolved bucket.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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<String, String> requested)
+ throws IOException {
+ Pair<String, String> 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<String, String> resolveBucketLink(
+ Pair<String, String> volumeAndBucket,
+ Set<Pair<String, String>> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658868821
> Thanks @bharatviswa504 for the review. I have updated the patch based on your comments.
>
> > When deleting a bucket, we check bucket is empty or not. Do we need to check resolvedBucket is empty or not?
>
> Link can be deleted regardless of the existence or emptiness of the real bucket it points to. So no need to check resolved bucket.
This is the semantics for link buckets?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r450605941
##########
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?
Because with this approach for all mounted buckets, we do 2 checkAcls, we might put pressure on Ranger.
Can we rely on underlying bucket acls, as anyway we verify already. Any downside/security issue?
##########
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<String, String> auditMap = args.toAuditMap();
Review comment:
Makes sense to me.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659851288
Thanks @arp7, @elek for high-level review, @bharatviswa504 for the detailed reviews, @ChenSammi for testing and committing 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447292219
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2020,60 +2026,72 @@ public OmBucketInfo getBucketInfo(String volume, String bucket)
*/
@Override
public OpenKeySession openKey(OmKeyArgs args) throws IOException {
+ ResolvedBucket bucket = resolveBucketLink(args);
+
if (isAclEnabled) {
try {
checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE,
- args.getVolumeName(), args.getBucketName(), args.getKeyName());
+ bucket.realVolume(), bucket.realBucket(), args.getKeyName());
Review comment:
Same, for all, write requests old code is not used anymore.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659503325
I have one suggestion, can we make this mount configurable option, as this is required only when old S3 buckets needs to be exposed via O3fs/ofs. In this way, we can avoid extra bucket checks. (I know that it is in-memory cache, but still I feel if some one does not require this feature, it is totally disabled). I think we can make that check in 2 places. One in create bucket/and other in resolveBucketLink.
Fine to do in another Jira also.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447311657
##########
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:
Okay, I see is in resolveBucketLink we check Acls for read on provided bucket, and in actual request check acls on sourceBucket/sourceVolume which it is resolved to. Let me know if i am missing something here. Why do we need 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659211305
@adoroszlai, thanks for working on this feature. I did some simple tests locally and looks good. A few questions about the expected behavior,
1. When the source bucket is deleted, bucket link still exists. It's an expected behavior, right?
2. (Unlike previous per-user S3 volume mapping) Link buckets are not inherently specific to a user, access is restricted only by ACL. Not very clear about what it means. would you explain it further? Can we set different ACL on source bucket and bucket link?
3. We're working on Volume quota implementation. I suppose bucket link will require 0 quota from it's parent volume, only the source bucket size will be controlled by it's volume's quota, right?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-649939780
@bharatviswa504 may I ask you to review 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-658382641
> This patch needs a rebase. (This has been due to one of my commit only).
Done.
> I will take a final look at it today, and will post my comments.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-653003019
@ChenSammi this feature is continuation of S3 volume mapping changes. Can you please take a look?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-659220643
> 1. When the source bucket is deleted, bucket link still exists. It's an expected behavior, right?
Yes.
> 2. (Unlike previous per-user S3 volume mapping) Link buckets are not inherently specific to a user, access is restricted only by ACL. Not very clear about what it means. would you explain it further?
Previously each user had its own S3 volume and only by sharing the AWS access key could others use it. Now if you create a bucket link, other users can access it as long as ACL permits (similar to other Ozone objects).
> Can we set different ACL on source bucket and bucket link?
Yes. When following links eg. to create a key, only read permission is checked on the link. The source bucket is still checked for the permission required for the specific operation (eg. key list, create, or delete).
> 3. We're working on Volume quota implementation. I suppose bucket link will require 0 quota from it's parent volume, only the source bucket size will be controlled by it's volume's quota, right?
I'm not familiar with quotas, but I guess links should work like empty buckets in this respect. Keys are created in the source bucket, so they should not affect space usage in the link's volume.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-650118591
Thanks to working on this @adoroszlai
Checked only on high-level (yet) but structure looks good to me. This is exactly what we planned. Will try out in a deployment.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 edited a comment on pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#issuecomment-654594774
> > Question: Now when list at the root "/" with ofs filesystem, for link buckets they will be displayed as a new entry of only resolved buckets will be shown. What should be the behavior here.
>
> I'm afraid I don't understand the question. Can you please give an example?
when user perform `ls` on volume/bucket, which has below buckets. It will show only resolved buckets or all the buckets? Or it follows normal ls semantics?
if it has mounted buckets
b1 -> b2
b2
b4 -> b5
what should be the behavior of ls in this case?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r454552913
##########
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<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
- String volumeName = deleteKeyArgs.getVolumeName();
- String bucketName = deleteKeyArgs.getBucketName();
- String keyName = deleteKeyArgs.getKeyName();
+ String volumeName = keyArgs.getVolumeName();
Review comment:
The same comment applies for all requests
##########
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<String, String> requested)
+ throws IOException {
+ Pair<String, String> 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<String, String> resolveBucketLink(
+ Pair<String, String> volumeAndBucket,
+ Set<Pair<String, String>> 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:
Can we change the error code here, the reason for this is in HA when error code is INTERNAL_ERROR we terminate OM. (This is done to avoid DB divergence)
##########
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?
For example
Delete existing bucket
# Bucket already is created in Test Setup.
${bucket} = Create bucket
Execute AWSS3APICli delete-bucket --bucket ${bucket}
And bucket is returned from
Delete existing bucket
# Bucket already is created in Test Setup.
${bucket} = Create bucket
Execute AWSS3APICli delete-bucket --bucket ${bucket}
##########
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<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
- String volumeName = deleteKeyArgs.getVolumeName();
- String bucketName = deleteKeyArgs.getBucketName();
- String keyName = deleteKeyArgs.getKeyName();
+ String volumeName = keyArgs.getVolumeName();
Review comment:
Minor: We can skip getting from original Args, as anyway final Volume/Bucket we get is from resolvedBucket returned keyArgs.
##########
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:
Now with this approach, we validate bucket exist twice. Once is resolvedBucketLink, and again in ValidateVolumeAndBucket. If this is link, can we skip 2nd-time check?
(Now bucket/volume is in the full cache, but I think it will be better to avoid still)
Might be in non-HA, this might create a problem as we release lock and then re-acquire some other thread might delete the bucket. But once HA becomes the default, we can optimize this, as in HA there is only a single thread executor. If you also think the same open the Jira for improvement
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java
##########
@@ -114,12 +117,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
getOmRequest());
OMClientResponse omClientResponse = null;
try {
+ keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap);
+
// TODO to support S3 ACL later.
acquiredBucketLock =
- omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
Review comment:
Minor: Can we assign to volumeName and bucketName, similar to Key requests and use that, instead of getting multiple times.
##########
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<String, String> auditMap = null;
String volumeName = deleteKeyArgs.getVolumeName();
String bucketName = deleteKeyArgs.getBucketName();
- String keyName = "";
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ auditMap.put(VOLUME, volumeName);
Review comment:
Minor: We can skip this, as anyway bucket.audit has already taken care of this.
##########
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:
I see for all key requests we logged resolvedVolume/resolvedBucket. For MPU we log requested volume/bucket any reason for this?. Can we follow one approach for all requests?
##########
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 Timeout 2 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/scm@EXAMPLE.COM:r ${target}
+ Execute ozone sh volume addacl --acl user:testuser2/scm@EXAMPLE.COM:rl ${source}
+ Execute ozone sh bucket addacl --acl user:testuser2/scm@EXAMPLE.COM:rl ${source}/readable-bucket
+ Execute ozone sh bucket addacl --acl user:testuser2/scm@EXAMPLE.COM:r ${target}/readable-link
+ Execute ozone sh bucket addacl --acl user:testuser2/scm@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 Error ozone 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 Error ozone 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 Error ozone 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 Error ozone 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 ozone sh key put ${target}/link1/key1 /etc/passwd
+ Key Should Match Local File ${target}/link1/key1 /etc/passwd
+
+Key read passthrough
+ Execute ozone sh key put ${source}/bucket1/key2 /opt/hadoop/NOTICE.txt
+ Key Should Match Local File ${source}/bucket1/key2 /opt/hadoop/NOTICE.txt
+
+Key list passthrough
+ ${target_list} = Execute ozone sh key list ${target}/link1 | jq -r '.name'
+ ${source_list} = Execute ozone sh key list ${source}/bucket1 | jq -r '.name'
+ Should Be Equal ${target_list} ${source_list}
+ Should Contain ${source_list} key1
+ Should Contain ${source_list} key2
+
+Key delete passthrough
+ Execute ozone sh key delete ${target}/link1/key2
+ ${source_list} = Execute ozone sh key list ${source}/bucket1 | jq -r '.name'
+ Should Not Contain ${source_list} key2
+
+Bucket list contains links
+ ${result} = Execute ozone sh bucket list ${target}
+ Should Contain ${result} link1
+ Should Contain ${result} dangling-link
+
+Bucket info shows source
+ ${result} = Execute ozone sh bucket info ${target}/link1 | jq -r '.sourceVolume, .sourceBucket' | xargs
+ Should Be Equal ${result} ${source} bucket1
+
+Source and target have separate ACLs
+ Execute ozone sh bucket addacl --acl user:user1:rwxy ${target}/link1
+ Verify ACL bucket ${target}/link1 USER user1 READ WRITE READ_ACL WRITE_ACL
+ Verify ACL bucket ${source}/bucket1 USER user1 ${EMPTY}
+
+ Execute ozone sh bucket addacl --acl group:group2:r ${source}/bucket1
+ Verify ACL bucket ${target}/link1 GROUP group2 ${EMPTY}
+ Verify ACL bucket ${source}/bucket1 GROUP group2 READ
+
+Buckets and links share namespace
+ Execute ozone sh bucket link ${source}/bucket2 ${target}/link2
+ ${result} = Execute And Ignore Error ozone sh bucket create ${target}/link2
+ Should Contain ${result} BUCKET_ALREADY_EXISTS
+
+ Execute ozone sh bucket create ${target}/bucket3
+ ${result} = Execute And Ignore Error ozone sh bucket link ${source}/bucket1 ${target}/bucket3
+ Should Contain ${result} BUCKET_ALREADY_EXISTS
+
+Can follow link with read access
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Can follow link with read access
+
+Cannot follow link without read access
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Cannot follow link without read access
+
+ACL verified on source bucket
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' ACL verified on source bucket
+
+Loop in link chain is detected
+ Execute ozone sh bucket link ${target}/loop1 ${target}/loop2
+ Execute ozone sh bucket link ${target}/loop2 ${target}/loop3
+ Execute ozone sh bucket link ${target}/loop3 ${target}/loop1
+ ${result} = Execute And Ignore Error ozone sh key list ${target}/loop2
+ Should Contain ${result} INTERNAL_ERROR
Review comment:
This needs to be updated, once error code is modified.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r447301716
##########
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:
Now for each operation if it is link bucket where if it has sourceVolume/SourceBucket we do checkAcls twice. One with READ permission on sourceBucket/SourceVolume in resoleBucketLink and one in actual request with required ACL type. It is not clear why do we need the first check acl.
If it requires, do you think we need to have an API where we can check all required ACLS with a single checkAcl call?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1104: HDDS-3612. Allow mounting bucket under other volume
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1104:
URL: https://github.com/apache/hadoop-ozone/pull/1104#discussion_r451877316
##########
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:
Any info on this question?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org