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 2021/11/17 10:07:39 UTC
[GitHub] [ozone] smengcl opened a new pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret (`ozone tenant user setsecret`)
smengcl opened a new pull request #2845:
URL: https://github.com/apache/ozone/pull/2845
https://issues.apache.org/jira/browse/HDDS-5972
- Implemented `ozone tenant user setsecret`.
- Fixed a bug where revoke-admin would fail if tenantName is not given on the CLI.
- Renamed OmDBAccessIdInfo fields.
## Testing
- Added new integration tests.
- Added new robot(acceptance) test cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] prashantpogde commented on a change in pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret (`ozone tenant user setsecret`)
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#discussion_r752794526
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java
##########
@@ -178,6 +178,18 @@ public S3SecretValue getS3Secret(String kerberosID, boolean createIfNotExist)
return proxy.getS3Secret(kerberosID, createIfNotExist);
}
+ /**
+ * Set secretKey for accessId.
+ * @param accessId
+ * @param secretKey
+ * @return S3SecretValue <accessId, secretKey> pair
+ * @throws IOException
+ */
+ public S3SecretValue setSecret(String accessId, String secretKey)
Review comment:
perhaps setS3Secret is more appropriate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl merged pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret: `ozone tenant user setsecret` and `ozone s3 setsecret`
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #2845:
URL: https://github.com/apache/ozone/pull/2845
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret (`ozone tenant user setsecret`)
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#issuecomment-973604984
> nit : why are we changing the tenantId/tenantName ? it is causing changes in too many files.
Hey thanks @prashantpogde for the review. The rename was part of the refactoring plan. The goal is to make the variable names consistent for maintainability / less confusion in the future. I accidentally renamed `tenantId` -> `tenantName` while the plan is to rename the existing `tenantName` -> `tenantId`. That has been addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret (`ozone tenant user setsecret`)
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#discussion_r752786969
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java
##########
@@ -178,6 +178,18 @@ public S3SecretValue getS3Secret(String kerberosID, boolean createIfNotExist)
return proxy.getS3Secret(kerberosID, createIfNotExist);
}
+ /**
+ * Set secretKey for accessId.
+ * @param accessId
+ * @param secretKey
+ * @return S3SecretValue <accessId, secretKey> pair
+ * @throws IOException
+ */
+ public S3SecretValue setSecret(String accessId, String secretKey)
Review comment:
I named this `setSecret` instead of `setS3Secret` because it is more generic. Or maybe I should just use `setS3Secret` to keep the naming consistent with the rest?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret: `ozone tenant user setsecret` and `ozone s3 setsecret`
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#issuecomment-974578340
Got a green run. Thanks @prashantpogde for the +1. Merging in a minute.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] prashantpogde commented on a change in pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret (`ozone tenant user setsecret`)
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#discussion_r752516566
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/OMSetSecretRequest.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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.s3.security;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ipc.ProtobufRpcEngine;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmDBAccessIdInfo;
+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.s3.security.OMSetSecretResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.S3_SECRET_LOCK;
+import static org.apache.hadoop.ozone.om.request.s3.tenant.OMTenantRequestHelper.isUserAccessIdPrincipalOrTenantAdmin;
+
+/**
+ * Handles SetSecret request.
+ */
+public class OMSetSecretRequest extends OMClientRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMSetSecretRequest.class);
+
+ public OMSetSecretRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ final SetSecretRequest setSecretRequest =
+ getOmRequest().getSetSecretRequest();
+
+ final String accessId = setSecretRequest.getAccessId();
+
+ // First check accessId existence
+ final OmDBAccessIdInfo accessIdInfo = ozoneManager.getMetadataManager()
+ .getTenantAccessIdTable().get(accessId);
+ // TODO: Support old `ozone s3 getsecret` S3SecretTable?
Review comment:
Can we support setSecret for the non-multi-tenant case as well ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret: `ozone tenant user setsecret` and `ozone s3 setsecret`
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#discussion_r752809310
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java
##########
@@ -178,6 +178,18 @@ public S3SecretValue getS3Secret(String kerberosID, boolean createIfNotExist)
return proxy.getS3Secret(kerberosID, createIfNotExist);
}
+ /**
+ * Set secretKey for accessId.
+ * @param accessId
+ * @param secretKey
+ * @return S3SecretValue <accessId, secretKey> pair
+ * @throws IOException
+ */
+ public S3SecretValue setSecret(String accessId, String secretKey)
Review comment:
Good idea. 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2845: HDDS-5972. [Multi-Tenant] Implement SetSecret (`ozone tenant user setsecret`)
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2845:
URL: https://github.com/apache/ozone/pull/2845#discussion_r752782951
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/OMSetSecretRequest.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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.s3.security;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ipc.ProtobufRpcEngine;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmDBAccessIdInfo;
+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.s3.security.OMSetSecretResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.S3_SECRET_LOCK;
+import static org.apache.hadoop.ozone.om.request.s3.tenant.OMTenantRequestHelper.isUserAccessIdPrincipalOrTenantAdmin;
+
+/**
+ * Handles SetSecret request.
+ */
+public class OMSetSecretRequest extends OMClientRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMSetSecretRequest.class);
+
+ public OMSetSecretRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ final SetSecretRequest setSecretRequest =
+ getOmRequest().getSetSecretRequest();
+
+ final String accessId = setSecretRequest.getAccessId();
+
+ // First check accessId existence
+ final OmDBAccessIdInfo accessIdInfo = ozoneManager.getMetadataManager()
+ .getTenantAccessIdTable().get(accessId);
+ // TODO: Support old `ozone s3 getsecret` S3SecretTable?
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org