From 4efd948ce0865021f1e8b6938bfc39ae283943eb Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Wed, 12 Jan 2022 19:22:07 -0800 Subject: [PATCH 1/3] HDDS-6171. Create an API to change Bucket Owner --- .../hadoop/ozone/client/OzoneBucket.java | 10 + .../ozone/client/protocol/ClientProtocol.java | 10 + .../hadoop/ozone/client/rpc/RpcClient.java | 13 ++ .../hadoop/ozone/om/helpers/OmBucketArgs.java | 33 ++- .../hadoop/ozone/om/helpers/OmBucketInfo.java | 10 +- .../rpc/TestOzoneRpcClientAbstract.java | 20 ++ .../src/main/proto/OmClientProtocol.proto | 1 + .../ratis/utils/OzoneManagerRatisUtils.java | 9 +- .../bucket/OMBucketSetOwnerRequest.java | 196 ++++++++++++++++++ .../bucket/OMBucketSetOwnerResponse.java | 66 ++++++ .../ozone/shell/bucket/BucketCommands.java | 3 +- .../shell/bucket/UpdateBucketHandler.java | 58 ++++++ 12 files changed, 422 insertions(+), 7 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java create mode 100644 hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 23cf922918f7..49890d891a3c 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -905,6 +905,16 @@ public OzoneMultipartUploadList listMultipartUploads(String prefix) return proxy.listMultipartUploads(volumeName, getName(), prefix); } + /** + * Sets/Changes the owner of this Bucket. + * @param userName new owner + * @throws IOException + */ + public void setOwner(String userName) throws IOException{ + proxy.setBucketOwner(volumeName, name, userName); + this.owner = userName; + } + /** * An Iterator to iterate over {@link OzoneKey} list. */ diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 8d6ea10e412e..919db976396f 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -772,4 +772,14 @@ OzoneKey headObject(String volumeName, String bucketName, * Clears the S3 Authentication information attached to the thread. */ void clearTheadLocalS3Auth(); + + /** + * Sets the owner of bucket. + * @param volumeName Name of the Volume + * @param bucketName Name of the Bucket + * @param owner to be set for the bucket + * @throws IOException + */ + void setBucketOwner(String volumeName, String bucketName, + String owner) throws IOException; } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 4f2a4942b1f0..5832894b5c46 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -1579,4 +1579,17 @@ public S3Auth getThreadLocalS3Auth() { public void clearTheadLocalS3Auth() { ozoneManagerClient.clearThreadLocalS3Auth(); } + + @Override + public void setBucketOwner(String volumeName, String bucketName, + String owner) throws IOException { + verifyVolumeName(volumeName); + verifyBucketName(bucketName); + Preconditions.checkNotNull(owner); + OmBucketArgs.Builder builder = OmBucketArgs.newBuilder(); + builder.setVolumeName(volumeName) + .setBucketName(bucketName) + .setOwnerName(owner); + ozoneManagerClient.setBucketProperty(builder.build()); + } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java index 1c8c18a4b397..0b05cae84fd0 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java @@ -51,6 +51,10 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { private long quotaInBytes; private long quotaInNamespace; + /** + * Bucket Owner Name. + */ + private String ownerName; /** * Private constructor, constructed via builder. @@ -61,9 +65,11 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { * @param quotaInBytes Volume quota in bytes. * @param quotaInNamespace Volume quota in counts. */ + @SuppressWarnings("checkstyle:ParameterNumber") private OmBucketArgs(String volumeName, String bucketName, Boolean isVersionEnabled, StorageType storageType, - Map metadata, long quotaInBytes, long quotaInNamespace) { + Map metadata, long quotaInBytes, long quotaInNamespace, + String ownerName) { this.volumeName = volumeName; this.bucketName = bucketName; this.isVersionEnabled = isVersionEnabled; @@ -71,6 +77,7 @@ private OmBucketArgs(String volumeName, String bucketName, this.metadata = metadata; this.quotaInBytes = quotaInBytes; this.quotaInNamespace = quotaInNamespace; + this.ownerName = ownerName; } /** @@ -121,6 +128,14 @@ public long getQuotaInNamespace() { return quotaInNamespace; } + /** + * Returns Bucket Owner Name. + * @return ownerName. + */ + public String getOwnerName() { + return ownerName; + } + /** * Returns new builder class that builds a OmBucketArgs. * @return Builder @@ -155,7 +170,7 @@ public static class Builder { private Map metadata; private long quotaInBytes; private long quotaInNamespace; - + private String ownerName; /** * Constructs a builder. */ @@ -199,6 +214,11 @@ public Builder setQuotaInNamespace(long quota) { return this; } + public Builder setOwnerName(String owner) { + ownerName = owner; + return this; + } + /** * Constructs the OmBucketArgs. * @return instance of OmBucketArgs. @@ -207,7 +227,7 @@ public OmBucketArgs build() { Preconditions.checkNotNull(volumeName); Preconditions.checkNotNull(bucketName); return new OmBucketArgs(volumeName, bucketName, isVersionEnabled, - storageType, metadata, quotaInBytes, quotaInNamespace); + storageType, metadata, quotaInBytes, quotaInNamespace, ownerName); } } @@ -230,6 +250,9 @@ public BucketArgs getProtobuf() { if(quotaInNamespace > 0 || quotaInNamespace == OzoneConsts.QUOTA_RESET) { builder.setQuotaInNamespace(quotaInNamespace); } + if(ownerName != null) { + builder.setOwnerName(ownerName); + } return builder.build(); } @@ -247,6 +270,8 @@ public static OmBucketArgs getFromProtobuf(BucketArgs bucketArgs) { bucketArgs.getStorageType()) : null, KeyValueUtil.getFromProtobuf(bucketArgs.getMetadataList()), bucketArgs.getQuotaInBytes(), - bucketArgs.getQuotaInNamespace()); + bucketArgs.getQuotaInNamespace(), + bucketArgs.hasOwnerName() ? + bucketArgs.getOwnerName() : null); } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index 786bb74a75b7..adbf3986c5a5 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -91,7 +91,7 @@ public final class OmBucketInfo extends WithObjectID implements Auditable { */ private BucketLayout bucketLayout; - private final String owner; + private String owner; /** * Private constructor, constructed via builder. @@ -297,6 +297,14 @@ public String getOwner() { return owner; } + public void setModificationTime(long modificationTime) { + this.modificationTime = modificationTime; + } + + public void setOwner(String ownerName) { + this.owner = ownerName; + } + /** * Returns new builder class that builds a OmBucketInfo. * diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index 222e352154eb..838cb4f103e4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -297,6 +297,26 @@ public void testVolumeSetOwner() throws IOException { proxy.setVolumeOwner(volumeName, ownerName); } + @Test + public void testBucketSetOwner() throws IOException { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + store.createVolume(volumeName); + store.getVolume(volumeName).createBucket(bucketName); + + String oldOwner = store.getVolume(volumeName).getBucket(bucketName) + .getOwner(); + String ownerName = "testUser"; + + ClientProtocol proxy = store.getClientProxy(); + proxy.setBucketOwner(volumeName, bucketName, ownerName); + String newOwner = store.getVolume(volumeName).getBucket(bucketName) + .getOwner(); + + assertEquals(ownerName, newOwner); + assertNotEquals(oldOwner, newOwner); + } + @Test public void testSetAndClrQuota() throws Exception { String volumeName = UUID.randomUUID().toString(); diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 1dd922c8655d..027b16a0a65c 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -623,6 +623,7 @@ message BucketArgs { repeated hadoop.hdds.KeyValue metadata = 7; optional uint64 quotaInBytes = 8; optional uint64 quotaInNamespace = 9; + optional string ownerName = 10; } message PrefixInfo { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java index 0480859c39ed..47c2eb83ead8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java @@ -39,6 +39,7 @@ import org.apache.hadoop.ozone.om.request.OMKeyRequestFactory; import org.apache.hadoop.ozone.om.request.bucket.OMBucketCreateRequest; import org.apache.hadoop.ozone.om.request.bucket.OMBucketDeleteRequest; +import org.apache.hadoop.ozone.om.request.bucket.OMBucketSetOwnerRequest; import org.apache.hadoop.ozone.om.request.bucket.OMBucketSetPropertyRequest; import org.apache.hadoop.ozone.om.request.OMClientRequest; import org.apache.hadoop.ozone.om.request.bucket.acl.OMBucketAddAclRequest; @@ -141,7 +142,13 @@ public static OMClientRequest createClientRequest(OMRequest omRequest, case DeleteBucket: return new OMBucketDeleteRequest(omRequest); case SetBucketProperty: - return new OMBucketSetPropertyRequest(omRequest); + boolean hasBucketOwner = omRequest.getSetBucketPropertyRequest() + .getBucketArgs().hasOwnerName(); + if (hasBucketOwner) { + return new OMBucketSetOwnerRequest(omRequest); + } else { + return new OMBucketSetPropertyRequest(omRequest); + } case AddAcl: case RemoveAcl: case SetAcl: diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java new file mode 100644 index 000000000000..34cd3b944961 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java @@ -0,0 +1,196 @@ +/** + * 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. + */ +package org.apache.hadoop.ozone.om.request.bucket; + +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import org.apache.hadoop.hdds.utils.db.cache.CacheKey; +import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.audit.AuditLogger; +import org.apache.hadoop.ozone.audit.OMAction; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.OMMetrics; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; +import org.apache.hadoop.ozone.om.request.OMClientRequest; +import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.response.bucket.OMBucketSetOwnerResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BucketArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SetBucketPropertyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.SetBucketPropertyResponse; +import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; +import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.util.Time; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; + +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; + +/** + * Handle set owner request for bucket. + */ +public class OMBucketSetOwnerRequest extends OMClientRequest { + private static final Logger LOG = + LoggerFactory.getLogger(OMBucketSetOwnerRequest.class); + + public OMBucketSetOwnerRequest(OMRequest omRequest) { + super(omRequest); + } + + @Override + public OMRequest preExecute(OzoneManager ozoneManager) + throws IOException { + long modificationTime = Time.now(); + OzoneManagerProtocolProtos.SetBucketPropertyRequest.Builder + setBucketPropertyRequestBuilder = getOmRequest() + .getSetBucketPropertyRequest().toBuilder() + .setModificationTime(modificationTime); + + return getOmRequest().toBuilder() + .setSetBucketPropertyRequest(setBucketPropertyRequestBuilder) + .setUserInfo(getUserInfo()) + .build(); + } + + @Override + public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, + long transactionLogIndex, + OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) { + SetBucketPropertyRequest setBucketPropertyRequest = + getOmRequest().getSetBucketPropertyRequest(); + Preconditions.checkNotNull(setBucketPropertyRequest); + + OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder( + getOmRequest()); + + if (!setBucketPropertyRequest.getBucketArgs().hasOwnerName()) { + omResponse.setStatus(OzoneManagerProtocolProtos.Status.INVALID_REQUEST) + .setSuccess(false); + return new OMBucketSetOwnerResponse(omResponse.build()); + } + + OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); + OMMetrics omMetrics = ozoneManager.getMetrics(); + omMetrics.incNumBucketUpdates(); + + BucketArgs bucketArgs = setBucketPropertyRequest.getBucketArgs(); + OmBucketArgs omBucketArgs = OmBucketArgs.getFromProtobuf(bucketArgs); + + String volumeName = bucketArgs.getVolumeName(); + String bucketName = bucketArgs.getBucketName(); + String newOwner = bucketArgs.getOwnerName(); + String oldOwner = null; + + AuditLogger auditLogger = ozoneManager.getAuditLogger(); + OzoneManagerProtocolProtos.UserInfo userInfo = getOmRequest().getUserInfo(); + IOException exception = null; + boolean acquiredBucketLock = false, success = true; + OMClientResponse omClientResponse = null; + try { + // check Acl + if (ozoneManager.getAclsEnabled()) { + checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, + OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, + volumeName, bucketName, null); + } + + // acquire lock. + acquiredBucketLock = omMetadataManager.getLock().acquireWriteLock( + BUCKET_LOCK, volumeName, bucketName); + + String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); + OmBucketInfo omBucketInfo = + omMetadataManager.getBucketTable().get(bucketKey); + //Check if bucket exist + if (omBucketInfo == null) { + LOG.debug("Bucket: {} not found ", bucketName); + throw new OMException("Bucket doesnt exist", + OMException.ResultCodes.BUCKET_NOT_FOUND); + } + + oldOwner = omBucketInfo.getOwner(); + + if (oldOwner.equals(newOwner)) { + LOG.warn("Bucket '{}/{}' owner is already user '{}'.", + volumeName, bucketName, oldOwner); + omResponse.setStatus(OzoneManagerProtocolProtos.Status.INVALID_REQUEST) + .setMessage("Bucket '" + volumeName + "/" + bucketName + + "' owner is already '" + newOwner + "'.") + .setSuccess(false); + return new OMBucketSetOwnerResponse(omResponse.build()); + } + + omBucketInfo.setOwner(newOwner); + LOG.debug("Updating bucket owner to {} for bucket: {} in volume: {}", + newOwner, bucketName, volumeName); + + omBucketInfo.setModificationTime( + setBucketPropertyRequest.getModificationTime()); + // Set the updateID to current transaction log index + omBucketInfo.setUpdateID(transactionLogIndex, + ozoneManager.isRatisEnabled()); + + // Update table cache. + omMetadataManager.getBucketTable().addCacheEntry( + new CacheKey<>(bucketKey), + new CacheValue<>(Optional.of(omBucketInfo), transactionLogIndex)); + + omResponse.setSetBucketPropertyResponse( + SetBucketPropertyResponse.newBuilder().build()); + omClientResponse = new OMBucketSetOwnerResponse( + omResponse.build(), omBucketInfo); + } catch (IOException ex) { + success = false; + exception = ex; + omClientResponse = new OMBucketSetOwnerResponse( + createErrorOMResponse(omResponse, exception)); + } finally { + addResponseToDoubleBuffer(transactionLogIndex, omClientResponse, + ozoneManagerDoubleBufferHelper); + if (acquiredBucketLock) { + omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, + bucketName); + } + } + + // Performing audit logging outside of the lock. + auditLog(auditLogger, buildAuditMessage(OMAction.UPDATE_BUCKET, + omBucketArgs.toAuditMap(), exception, userInfo)); + + // return response. + if (success) { + LOG.debug("Successfully changed Owner of Bucket {}/{} from {} -> {}", + volumeName, bucketName, oldOwner, newOwner); + return omClientResponse; + } else { + LOG.error("Setting Owner failed for bucket:{} in volume:{}", + bucketName, volumeName, exception); + omMetrics.incNumBucketUpdateFails(); + return omClientResponse; + } + } +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java new file mode 100644 index 000000000000..69b9a55f324a --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java @@ -0,0 +1,66 @@ +/** + * 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. + */ +package org.apache.hadoop.ozone.om.response.bucket; + +import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.response.CleanupTableInfo; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos + .OMResponse; + +import javax.annotation.Nonnull; +import java.io.IOException; + +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; + +/** + * Response for set owner request. + */ +@CleanupTableInfo(cleanupTables = {BUCKET_TABLE}) +public class OMBucketSetOwnerResponse extends OMClientResponse { + + private OmBucketInfo omBucketInfo; + + public OMBucketSetOwnerResponse(@Nonnull OMResponse omResponse, + @Nonnull OmBucketInfo omBucketInfo) { + super(omResponse); + this.omBucketInfo = omBucketInfo; + } + + /** + * For when the request is not successful. + * For a successful request, the other constructor should be used. + */ + public OMBucketSetOwnerResponse(@Nonnull OMResponse omResponse) { + super(omResponse); + checkStatusNotOK(); + } + + @Override + public void addToDBBatch(OMMetadataManager omMetadataManager, + BatchOperation batchOperation) throws IOException { + + String dbBucketKey = + omMetadataManager.getBucketKey(omBucketInfo.getVolumeName(), + omBucketInfo.getBucketName()); + omMetadataManager.getBucketTable().putWithBatch(batchOperation, + dbBucketKey, omBucketInfo); + } +} diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/BucketCommands.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/BucketCommands.java index cfb37636cf49..2de229081eb7 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/BucketCommands.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/BucketCommands.java @@ -48,7 +48,8 @@ RemoveAclBucketHandler.class, GetAclBucketHandler.class, SetAclBucketHandler.class, - ClearQuotaHandler.class + ClearQuotaHandler.class, + UpdateBucketHandler.class }, mixinStandardHelpOptions = true, versionProvider = HddsVersionProvider.class) diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java new file mode 100644 index 000000000000..71ddb4ac9645 --- /dev/null +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java @@ -0,0 +1,58 @@ +/** + * 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. + */ +package org.apache.hadoop.ozone.shell.bucket; + +import org.apache.hadoop.ozone.client.OzoneBucket; +import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.client.OzoneClientException; +import org.apache.hadoop.ozone.shell.OzoneAddress; + +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +import java.io.IOException; + +/** + * Executes update bucket calls. + */ +@Command(name = "update", + description = "Updates the parameters of the bucket") +public class UpdateBucketHandler extends BucketHandler { + + @Option(names = {"--user", "-u"}, + description = "Owner of the bucket to set") + private String ownerName; + + @Override + protected void execute(OzoneClient client, OzoneAddress address) + throws IOException, OzoneClientException { + + String volumeName = address.getVolumeName(); + String bucketName = address.getBucketName(); + OzoneBucket bucket = client.getObjectStore().getVolume(volumeName) + .getBucket(bucketName); + + if (ownerName != null && !ownerName.isEmpty()) { + bucket.setOwner(ownerName); + } + + OzoneBucket updatedBucket = client.getObjectStore().getVolume(volumeName) + .getBucket(bucketName); + printObjectAsJson(updatedBucket); + } +} From 7c9977c130c81d3d0c3a782bd88709c77c72ed73 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 18 Jan 2022 20:41:29 -0800 Subject: [PATCH 2/3] HDDS-6171. Added suggestions. --- .../hadoop/ozone/client/OzoneBucket.java | 5 +++-- .../ozone/client/protocol/ClientProtocol.java | 2 +- .../hadoop/ozone/client/rpc/RpcClient.java | 4 ++-- .../hadoop/ozone/om/helpers/OmBucketArgs.java | 3 +++ .../om/protocol/OzoneManagerProtocol.java | 12 ++++++++++ ...ManagerProtocolClientSideTranslatorPB.java | 22 +++++++++++++++++++ .../rpc/TestOzoneRpcClientAbstract.java | 2 ++ .../src/main/proto/OmClientProtocol.proto | 2 +- .../bucket/OMBucketSetOwnerRequest.java | 11 ++++++---- .../bucket/OMBucketSetOwnerResponse.java | 16 +++++++++++++- .../shell/bucket/UpdateBucketHandler.java | 6 ++++- 11 files changed, 73 insertions(+), 12 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 49890d891a3c..16d5a1b2f0c6 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -910,9 +910,10 @@ public OzoneMultipartUploadList listMultipartUploads(String prefix) * @param userName new owner * @throws IOException */ - public void setOwner(String userName) throws IOException{ - proxy.setBucketOwner(volumeName, name, userName); + public boolean setOwner(String userName) throws IOException{ + boolean result = proxy.setBucketOwner(volumeName, name, userName); this.owner = userName; + return result; } /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 919db976396f..043160471b79 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -780,6 +780,6 @@ OzoneKey headObject(String volumeName, String bucketName, * @param owner to be set for the bucket * @throws IOException */ - void setBucketOwner(String volumeName, String bucketName, + boolean setBucketOwner(String volumeName, String bucketName, String owner) throws IOException; } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 5832894b5c46..0f9e43382968 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -1581,7 +1581,7 @@ public void clearTheadLocalS3Auth() { } @Override - public void setBucketOwner(String volumeName, String bucketName, + public boolean setBucketOwner(String volumeName, String bucketName, String owner) throws IOException { verifyVolumeName(volumeName); verifyBucketName(bucketName); @@ -1590,6 +1590,6 @@ public void setBucketOwner(String volumeName, String bucketName, builder.setVolumeName(volumeName) .setBucketName(bucketName) .setOwnerName(owner); - ozoneManagerClient.setBucketProperty(builder.build()); + return ozoneManagerClient.setBucketOwner(builder.build()); } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java index 0b05cae84fd0..50bfcb990888 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java @@ -156,6 +156,9 @@ public Map toAuditMap() { if(this.storageType != null){ auditMap.put(OzoneConsts.STORAGE_TYPE, this.storageType.name()); } + if (this.ownerName != null) { + auditMap.put(OzoneConsts.OWNER, this.ownerName); + } return auditMap; } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 7bc67da7fa4c..7da3bb8406ca 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -199,6 +199,18 @@ default void setBucketProperty(OmBucketArgs args) throws IOException { "this to be implemented, as write requests use a new approach."); } + /** + * Changes the owner of a bucket. + * @param args - OMBucketArgs + * @return true if operation succeeded, false if specified user is + * already the owner. + * @throws IOException + */ + default boolean setBucketOwner(OmBucketArgs args) throws IOException { + throw new UnsupportedOperationException("OzoneManager does not require " + + "this to be implemented, as write requests use a new approach."); + } + /** * Open the given key and return an open key session. diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index b2c367f9c762..e2bf9f026da4 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -526,6 +526,28 @@ public void setBucketProperty(OmBucketArgs args) } + /** + * {@inheritDoc} + */ + @Override + public boolean setBucketOwner(OmBucketArgs args) + throws IOException { + SetBucketPropertyRequest.Builder req = + SetBucketPropertyRequest.newBuilder(); + BucketArgs bucketArgs = args.getProtobuf(); + req.setBucketArgs(bucketArgs); + + OMRequest omRequest = createOMRequest(Type.SetBucketProperty) + .setSetBucketPropertyRequest(req) + .build(); + + OMResponse omResponse = submitRequest(omRequest); + SetBucketPropertyResponse response = + handleError(omResponse).getSetBucketPropertyResponse(); + + return response.getResponse(); + } + /** * List buckets in a volume. * diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index 838cb4f103e4..897ad2295f5d 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -315,6 +315,8 @@ public void testBucketSetOwner() throws IOException { assertEquals(ownerName, newOwner); assertNotEquals(oldOwner, newOwner); + store.getVolume(volumeName).deleteBucket(bucketName); + store.deleteVolume(volumeName); } @Test diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 027b16a0a65c..2d694b807c8e 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -731,7 +731,7 @@ message SetBucketPropertyRequest { } message SetBucketPropertyResponse { - + optional bool response = 1; } message DeleteBucketRequest { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java index 34cd3b944961..6aad48b4c71d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java @@ -137,11 +137,14 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (oldOwner.equals(newOwner)) { LOG.warn("Bucket '{}/{}' owner is already user '{}'.", volumeName, bucketName, oldOwner); - omResponse.setStatus(OzoneManagerProtocolProtos.Status.INVALID_REQUEST) + omResponse.setStatus(OzoneManagerProtocolProtos.Status.OK) .setMessage("Bucket '" + volumeName + "/" + bucketName + "' owner is already '" + newOwner + "'.") .setSuccess(false); - return new OMBucketSetOwnerResponse(omResponse.build()); + omResponse.setSetBucketPropertyResponse( + SetBucketPropertyResponse.newBuilder().setResponse(false).build()); + omClientResponse = new OMBucketSetOwnerResponse(omResponse.build()); + return omClientResponse; } omBucketInfo.setOwner(newOwner); @@ -160,7 +163,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, new CacheValue<>(Optional.of(omBucketInfo), transactionLogIndex)); omResponse.setSetBucketPropertyResponse( - SetBucketPropertyResponse.newBuilder().build()); + SetBucketPropertyResponse.newBuilder().setResponse(true).build()); omClientResponse = new OMBucketSetOwnerResponse( omResponse.build(), omBucketInfo); } catch (IOException ex) { @@ -178,7 +181,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } // Performing audit logging outside of the lock. - auditLog(auditLogger, buildAuditMessage(OMAction.UPDATE_BUCKET, + auditLog(auditLogger, buildAuditMessage(OMAction.SET_OWNER, omBucketArgs.toAuditMap(), exception, userInfo)); // return response. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java index 69b9a55f324a..268787f33a97 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketSetOwnerResponse.java @@ -24,6 +24,7 @@ import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import javax.annotation.Nonnull; import java.io.IOException; @@ -50,7 +51,20 @@ public OMBucketSetOwnerResponse(@Nonnull OMResponse omResponse, */ public OMBucketSetOwnerResponse(@Nonnull OMResponse omResponse) { super(omResponse); - checkStatusNotOK(); + if (omResponse.getSuccess()) { + checkStatusNotOK(); + } + } + + @Override + public void checkAndUpdateDB(OMMetadataManager omMetadataManager, + BatchOperation batchOperation) throws IOException { + // When newOwner is the same as oldOwner, status is OK but success is false. + // We don't want to add it to DB batch in this case. + if (getOMResponse().getStatus() == OzoneManagerProtocolProtos.Status.OK && + getOMResponse().getSuccess()) { + addToDBBatch(omMetadataManager, batchOperation); + } } @Override diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java index 71ddb4ac9645..7ba62a5ce1cf 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/UpdateBucketHandler.java @@ -48,7 +48,11 @@ protected void execute(OzoneClient client, OzoneAddress address) .getBucket(bucketName); if (ownerName != null && !ownerName.isEmpty()) { - bucket.setOwner(ownerName); + boolean result = bucket.setOwner(ownerName); + if (LOG.isDebugEnabled() && !result) { + out().format("Bucket '%s' owner is already '%s'. Unchanged.%n", + volumeName + "/" + bucketName, ownerName); + } } OzoneBucket updatedBucket = client.getObjectStore().getVolume(volumeName) From a65fc5d9b6d208a35e01ef4a8729738d963620ae Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Wed, 19 Jan 2022 13:52:12 -0800 Subject: [PATCH 3/3] Fixed spacing --- .../java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java index 50bfcb990888..1806a0354211 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java @@ -253,7 +253,7 @@ public BucketArgs getProtobuf() { if(quotaInNamespace > 0 || quotaInNamespace == OzoneConsts.QUOTA_RESET) { builder.setQuotaInNamespace(quotaInNamespace); } - if(ownerName != null) { + if (ownerName != null) { builder.setOwnerName(ownerName); } return builder.build();