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/07/22 22:53:35 UTC
[GitHub] [ozone] smengcl opened a new pull request #2453: HDDS 5485
smengcl opened a new pull request #2453:
URL: https://github.com/apache/ozone/pull/2453
- HDDS-4945. Initial prototype for MultiTenant support for Ozone (#2403)
- Duplicate OmDBAccessIdInfo and OmDBAccessIdInfoCodec.
- Add TENANT_ACCESS_ID_TABLE
- Implement OmDBAccessIdInfo.
- Polish up OmDBAccessIdInfo.
- More polishing.
- Add table PRINCIPAL_TO_ACCESS_IDS_TABLE.
- Add missing bits for principalToAccessIdsTable.
--
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 #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r677861097
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
+ /**
+ * Kerberos principal this accessId belongs to.
+ */
+ private final String kerberosPrincipal;
+ /**
+ * Shared secret of the accessId. TODO: Encryption?
+ */
+ private final String sharedSecret;
+
+ // This implies above String fields should NOT contain the split key.
+ public static final String SERIALIZATION_SPLIT_KEY = ";";
+
+ public OmDBAccessIdInfo(String tenantId,
+ String kerberosPrincipal, String sharedSecret) {
+ this.tenantId = tenantId;
+ this.kerberosPrincipal = kerberosPrincipal;
+ this.sharedSecret = sharedSecret;
+ }
+
+ private OmDBAccessIdInfo(String accessIdInfoString) {
+ String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
+ Preconditions.checkState(tInfo.length == 3,
+ "Incorrect accessIdInfoString");
+
+ tenantId = tInfo[0];
+ kerberosPrincipal = tInfo[1];
+ sharedSecret = tInfo[2];
+ }
+
+ public String getTenantId() {
+ return tenantId;
+ }
+
+ private String serialize() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(tenantId).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(kerberosPrincipal).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(sharedSecret);
+ return sb.toString();
+ }
+
+ /**
+ * Convert OmDBAccessIdInfo to byteArray to be persisted to DB.
+ * @return byte[]
+ */
+ public byte[] convertToByteArray() {
+ return StringUtils.string2Bytes(serialize());
Review comment:
Yes it copies the string. Because underneath the call it uses `str.getBytes()`, we can't seem to circumvent that if we do want to convert String to byte[]. If there is we can improve StringUtils.
--
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] adoroszlai commented on pull request #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-948505681
@smengcl can we delete the HDDS-5485 branch in `apache/ozone` repo?
--
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 #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r678532701
##########
File path: hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -350,6 +351,10 @@ String getMultipartKey(String volume, String bucket, String key, String
Table<String, String> getTenantUserTable();
+ Table<String, OmDBAccessIdInfo> getTenantAccessIdTable();
+
+ Table<String, OmDBAccessIdInfo> getPrincipalToAccessIdsTable();
Review comment:
yup. thanks for pointing that out!
--
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 #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-888631418
Thanks @errose28 @avijayanhwx @jojochuang for reviewing this! I will merge this into the feature branch shortly.
--
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] errose28 commented on pull request #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-888628891
+1 LGTM. Kubernetes tests will be fixed by merging in master.
--
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] jojochuang commented on a change in pull request #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r677106282
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
+ /**
+ * Kerberos principal this accessId belongs to.
+ */
+ private final String kerberosPrincipal;
+ /**
+ * Shared secret of the accessId. TODO: Encryption?
+ */
+ private final String sharedSecret;
+
+ // This implies above String fields should NOT contain the split key.
+ public static final String SERIALIZATION_SPLIT_KEY = ";";
+
+ public OmDBAccessIdInfo(String tenantId,
+ String kerberosPrincipal, String sharedSecret) {
+ this.tenantId = tenantId;
+ this.kerberosPrincipal = kerberosPrincipal;
+ this.sharedSecret = sharedSecret;
+ }
+
+ private OmDBAccessIdInfo(String accessIdInfoString) {
+ String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
+ Preconditions.checkState(tInfo.length == 3,
+ "Incorrect accessIdInfoString");
+
+ tenantId = tInfo[0];
+ kerberosPrincipal = tInfo[1];
+ sharedSecret = tInfo[2];
+ }
+
+ public String getTenantId() {
+ return tenantId;
+ }
+
+ private String serialize() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(tenantId).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(kerberosPrincipal).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(sharedSecret);
+ return sb.toString();
+ }
+
+ /**
+ * Convert OmDBAccessIdInfo to byteArray to be persisted to DB.
+ * @return byte[]
+ */
+ public byte[] convertToByteArray() {
+ return StringUtils.string2Bytes(serialize());
Review comment:
i understand this is the same pattern used elsewhere, but i wonder is it memory efficient. It might save some by skipping constructing the String object.
(fwiw i don't see StringUtils.string2Bytes() nor StringUtils.bytes2String() popping up in profiler yet)
--
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] sonarcloud[bot] commented on pull request #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-888625293
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_coverage&view=list) [18.7% Coverage](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_duplicated_lines_density&view=list)
--
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 #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #2453:
URL: https://github.com/apache/ozone/pull/2453
--
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 #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-948879228
> @smengcl can we delete the [HDDS-5485](https://issues.apache.org/jira/browse/HDDS-5485) branch in `apache/ozone` repo?
Thanks for the reminder. I didn't realize this branch is pushed to the upstream. My bad. Will delete it 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] sonarcloud[bot] commented on pull request #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-887935344
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_coverage&view=list) [18.7% Coverage](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] removed a comment on pull request #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#issuecomment-887935344
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=hadoop-ozone&pullRequest=2453&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=hadoop-ozone&pullRequest=2453&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_coverage&view=list) [18.7% Coverage](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=hadoop-ozone&pullRequest=2453&metric=new_duplicated_lines_density&view=list)
--
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] errose28 commented on a change in pull request #2453: HDDS-5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r678493268
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -130,10 +132,10 @@
* |----------------------------------------------------------------------|
* | transactionInfoTable| #TRANSACTIONINFO -> OMTransactionInfo |
* |----------------------------------------------------------------------|
- * | tenantUserTable | tenant user name -> a tenant (organization) |
- * | tenantStateTable | tenant user name -> OmDBTenantInfo |
- * | tenantGroupTable | tenant user name -> [tenant group A, B, ...] |
- * | tenantRoleTable | tenant user name -> roles [admin, roleB, ...] |
+ * | tenantAccessIdTable| accessId -> OmTenantAccessIdInfo |
+ * | tenantStateTable | accessId -> OmDBTenantInfo |
+ * | tenantGroupTable | accessId -> [tenant group A, B, ...] |
+ * | tenantRoleTable | accessId -> roles [admin, roleB, ...] |
Review comment:
Nit. New section in the comment block for multi tenant tables, with a line between each table to match formatting in the rest of this block?
##########
File path: hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -350,6 +351,10 @@ String getMultipartKey(String volume, String bucket, String key, String
Table<String, String> getTenantUserTable();
+ Table<String, OmDBAccessIdInfo> getTenantAccessIdTable();
+
+ Table<String, OmDBAccessIdInfo> getPrincipalToAccessIdsTable();
Review comment:
Should it be OmDBKerberosPrincipalInfo as the value in the table return type?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -1265,6 +1285,16 @@ private PersistedUserVolumeInfo getVolumesByUser(String userNameKey)
return tenantUserTable;
}
+ @Override
+ public Table<String, OmDBAccessIdInfo> getTenantAccessIdTable() {
+ return tenantAccessIdTable;
+ }
+
+ @Override
+ public Table<String, OmDBAccessIdInfo> getPrincipalToAccessIdsTable() {
Review comment:
Same as above, should be OmDBKerberosPrincipalInfo for the table value?
--
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] avijayanhwx commented on a change in pull request #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r676831954
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
+ /**
+ * Kerberos principal this accessId belongs to.
+ */
+ private final String kerberosPrincipal;
+ /**
+ * Shared secret of the accessId. TODO: Encryption?
+ */
+ private final String sharedSecret;
+
+ // This implies above String fields should NOT contain the split key.
+ public static final String SERIALIZATION_SPLIT_KEY = ";";
+
+ public OmDBAccessIdInfo(String tenantId,
+ String kerberosPrincipal, String sharedSecret) {
+ this.tenantId = tenantId;
+ this.kerberosPrincipal = kerberosPrincipal;
+ this.sharedSecret = sharedSecret;
+ }
+
+ private OmDBAccessIdInfo(String accessIdInfoString) {
+ String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
+ Preconditions.checkState(tInfo.length == 3,
+ "Incorrect accessIdInfoString");
+
+ tenantId = tInfo[0];
+ kerberosPrincipal = tInfo[1];
+ sharedSecret = tInfo[2];
+ }
+
+ public String getTenantId() {
+ return tenantId;
+ }
+
+ private String serialize() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(tenantId).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(kerberosPrincipal).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(sharedSecret);
+ return sb.toString();
+ }
+
+ /**
+ * Convert OmDBAccessIdInfo to byteArray to be persisted to DB.
+ * @return byte[]
+ */
+ public byte[] convertToByteArray() {
+ return StringUtils.string2Bytes(serialize());
+ }
+
+ /**
+ * Convert byte array to OmDBAccessIdInfo.
+ */
+ public static OmDBAccessIdInfo getFromByteArray(byte[] bytes) {
+ String tInfo = StringUtils.bytes2String(bytes);
+ return new OmDBAccessIdInfo(tInfo);
+ }
+
+ public String getKerberosPrincipal() {
+ return kerberosPrincipal;
+ }
+
+ public String getSharedSecret() {
+ return sharedSecret;
+ }
+
+ /**
+ * Builder for OmDBAccessIdInfo.
+ */
+ @SuppressWarnings("checkstyle:hiddenfield")
+ public static final class Builder {
+ private String tenantId;
+ private String kerberosPrincipal;
+ private String sharedSecret;
+
+ private Builder() {
+ }
+
+ public Builder setTenantName(String tenantId) {
+ this.tenantId = tenantId;
+ return this;
+ }
+
+ public Builder setBucketNamespaceName(String kerberosPrincipal) {
Review comment:
Wrong method name?
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
Review comment:
Does this mean tenantId = volume_name strictly?
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
+ /**
+ * Kerberos principal this accessId belongs to.
+ */
+ private final String kerberosPrincipal;
+ /**
+ * Shared secret of the accessId. TODO: Encryption?
+ */
+ private final String sharedSecret;
+
+ // This implies above String fields should NOT contain the split key.
+ public static final String SERIALIZATION_SPLIT_KEY = ";";
+
+ public OmDBAccessIdInfo(String tenantId,
+ String kerberosPrincipal, String sharedSecret) {
+ this.tenantId = tenantId;
+ this.kerberosPrincipal = kerberosPrincipal;
+ this.sharedSecret = sharedSecret;
+ }
+
+ private OmDBAccessIdInfo(String accessIdInfoString) {
+ String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
+ Preconditions.checkState(tInfo.length == 3,
+ "Incorrect accessIdInfoString");
+
+ tenantId = tInfo[0];
+ kerberosPrincipal = tInfo[1];
+ sharedSecret = tInfo[2];
+ }
+
+ public String getTenantId() {
+ return tenantId;
+ }
+
+ private String serialize() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(tenantId).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(kerberosPrincipal).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(sharedSecret);
+ return sb.toString();
+ }
+
+ /**
+ * Convert OmDBAccessIdInfo to byteArray to be persisted to DB.
+ * @return byte[]
+ */
+ public byte[] convertToByteArray() {
+ return StringUtils.string2Bytes(serialize());
+ }
+
+ /**
+ * Convert byte array to OmDBAccessIdInfo.
+ */
+ public static OmDBAccessIdInfo getFromByteArray(byte[] bytes) {
+ String tInfo = StringUtils.bytes2String(bytes);
+ return new OmDBAccessIdInfo(tInfo);
+ }
+
+ public String getKerberosPrincipal() {
+ return kerberosPrincipal;
+ }
+
+ public String getSharedSecret() {
+ return sharedSecret;
+ }
+
+ /**
+ * Builder for OmDBAccessIdInfo.
+ */
+ @SuppressWarnings("checkstyle:hiddenfield")
+ public static final class Builder {
+ private String tenantId;
+ private String kerberosPrincipal;
+ private String sharedSecret;
+
+ private Builder() {
+ }
+
+ public Builder setTenantName(String tenantId) {
+ this.tenantId = tenantId;
+ return this;
+ }
+
+ public Builder setBucketNamespaceName(String kerberosPrincipal) {
+ this.kerberosPrincipal = kerberosPrincipal;
+ return this;
+ }
+
+ public Builder setAccountNamespaceName(String sharedSecret) {
Review comment:
Wrong method name?
--
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 #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r677861097
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
+ /**
+ * Kerberos principal this accessId belongs to.
+ */
+ private final String kerberosPrincipal;
+ /**
+ * Shared secret of the accessId. TODO: Encryption?
+ */
+ private final String sharedSecret;
+
+ // This implies above String fields should NOT contain the split key.
+ public static final String SERIALIZATION_SPLIT_KEY = ";";
+
+ public OmDBAccessIdInfo(String tenantId,
+ String kerberosPrincipal, String sharedSecret) {
+ this.tenantId = tenantId;
+ this.kerberosPrincipal = kerberosPrincipal;
+ this.sharedSecret = sharedSecret;
+ }
+
+ private OmDBAccessIdInfo(String accessIdInfoString) {
+ String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
+ Preconditions.checkState(tInfo.length == 3,
+ "Incorrect accessIdInfoString");
+
+ tenantId = tInfo[0];
+ kerberosPrincipal = tInfo[1];
+ sharedSecret = tInfo[2];
+ }
+
+ public String getTenantId() {
+ return tenantId;
+ }
+
+ private String serialize() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(tenantId).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(kerberosPrincipal).append(SERIALIZATION_SPLIT_KEY);
+ sb.append(sharedSecret);
+ return sb.toString();
+ }
+
+ /**
+ * Convert OmDBAccessIdInfo to byteArray to be persisted to DB.
+ * @return byte[]
+ */
+ public byte[] convertToByteArray() {
+ return StringUtils.string2Bytes(serialize());
Review comment:
Yes it copies the string. Underneath the call it uses `str.getBytes()`, we can't seem to circumvent that if we do want to convert String to byte[]. If there is we can improve StringUtils.
--
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] jojochuang commented on a change in pull request #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r677109331
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
+ /**
+ * Kerberos principal this accessId belongs to.
+ */
+ private final String kerberosPrincipal;
+ /**
+ * Shared secret of the accessId. TODO: Encryption?
+ */
+ private final String sharedSecret;
+
+ // This implies above String fields should NOT contain the split key.
+ public static final String SERIALIZATION_SPLIT_KEY = ";";
Review comment:
this is fine since POSIX systems are supposed to support user names composed of " lower and upper case ASCII letters, digits, period, underscore, and hyphen". Windows systems do not support ";" as a valid user name character.
--
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 #2453: HDDS 5485. Add new OM DB tables for AssignUserToTenant
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2453:
URL: https://github.com/apache/ozone/pull/2453#discussion_r677859184
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.helpers;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.StringUtils;
+
+/**
+ * This class is used for storing Ozone tenant accessId info.
+ */
+public final class OmDBAccessIdInfo {
+ /**
+ * Name of the tenant.
+ */
+ private final String tenantId;
Review comment:
No. this field simply stores the tenant name.
`OmDBTenantInfo.accountNamespaceName` is supposed to point to the home volume of the tenant.
--
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