You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ivandika3 (via GitHub)" <gi...@apache.org> on 2024/01/22 23:52:23 UTC
[PR] HDDS-10181. Remove duplicate ACL protobuf definitions in OmPrefixInfo [ozone]
ivandika3 opened a new pull request, #6068:
URL: https://github.com/apache/ozone/pull/6068
## What changes were proposed in this pull request?
It's observed that the serialization (toProtobuf) and deserialization (fromProtobuf) of OmPrefixInfo uses OzoneAclStorageUtil, whereas OmKeyInfo, OmBucketInfo, OmVolumeArgs uses OzoneAclUtil. The logic for both classes looks the same.
There are also duplicate proto definitions for OzoneAclInfo in OmStorageProtocol.proto and OmClientProtocol.proto.
We need to remove the duplication to prevent any divergence.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10181
## How was this patch tested?
Existing tests.
Clean CI: https://github.com/ivandika3/ozone/actions/runs/7611899320
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6068:
URL: https://github.com/apache/ozone/pull/6068#issuecomment-1907691449
@adoroszlai Thank you for the info. In that case, let me close this first.
I'll take a look at HDDS-4465 for the outcome and see whether we can proceed with the separation.
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6068:
URL: https://github.com/apache/ozone/pull/6068#discussion_r1463228884
##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPrefixInfo.java:
##########
@@ -184,7 +184,7 @@ public static OmPrefixInfo getFromProtobuf(PersistedPrefixInfo prefixInfo) {
.getFromProtobuf(prefixInfo.getMetadataList()));
}
if (prefixInfo.getAclsList() != null) {
- opib.setAcls(OzoneAclStorageUtil.fromProtobuf(prefixInfo.getAclsList()));
+ opib.setAcls(OzoneAclUtil.fromProtobuf(prefixInfo.getAclsList()));
Review Comment:
There is a change in Class name hierarcy for OzoneAclInfo from OzoneManagerStorageProtos --> OzoneManagerProtocolProtos
and Om.StorageProtocol.proto used for content saved in rocksdb for encode/decode.
Please check if this have any `compatibility issue`, i.e. before upgrade, having encoded with old way in db, and after upgrade, it should able to retrive same information from db with the change.
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6068:
URL: https://github.com/apache/ozone/pull/6068#discussion_r1464553878
##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPrefixInfo.java:
##########
@@ -184,7 +184,7 @@ public static OmPrefixInfo getFromProtobuf(PersistedPrefixInfo prefixInfo) {
.getFromProtobuf(prefixInfo.getMetadataList()));
}
if (prefixInfo.getAclsList() != null) {
- opib.setAcls(OzoneAclStorageUtil.fromProtobuf(prefixInfo.getAclsList()));
+ opib.setAcls(OzoneAclUtil.fromProtobuf(prefixInfo.getAclsList()));
Review Comment:
Yes, both type of prefixes can be read and printed properly.
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6068:
URL: https://github.com/apache/ozone/pull/6068#discussion_r1464380672
##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPrefixInfo.java:
##########
@@ -184,7 +184,7 @@ public static OmPrefixInfo getFromProtobuf(PersistedPrefixInfo prefixInfo) {
.getFromProtobuf(prefixInfo.getMetadataList()));
}
if (prefixInfo.getAclsList() != null) {
- opib.setAcls(OzoneAclStorageUtil.fromProtobuf(prefixInfo.getAclsList()));
+ opib.setAcls(OzoneAclUtil.fromProtobuf(prefixInfo.getAclsList()));
Review Comment:
Thank you for the review. I have tested it, and there was no compatibility issue.
Testing steps:
1. Deploy OM using 1.4.0-rc1 (without this patch)
2. Create a prefix (`ozone sh prefix addacl --acls="group:ozone:w" //dev/vol1/test-bucket/prefix/`)
3. Deploy OM using this patch
4. Read the same prefix (`ozone sh prefix getacl o3://dev/vol1/test-bucket/prefix/`)
5. Add another prefix (`ozone sh prefix addacl --acls="user:testuser:w" //dev/vol1/test-bucket/prefix/`)
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6068:
URL: https://github.com/apache/ozone/pull/6068#discussion_r1464471312
##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPrefixInfo.java:
##########
@@ -184,7 +184,7 @@ public static OmPrefixInfo getFromProtobuf(PersistedPrefixInfo prefixInfo) {
.getFromProtobuf(prefixInfo.getMetadataList()));
}
if (prefixInfo.getAclsList() != null) {
- opib.setAcls(OzoneAclStorageUtil.fromProtobuf(prefixInfo.getAclsList()));
+ opib.setAcls(OzoneAclUtil.fromProtobuf(prefixInfo.getAclsList()));
Review Comment:
can we read both prefix and print if comming properly?
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6068:
URL: https://github.com/apache/ozone/pull/6068#issuecomment-1907622066
> Note: This seems the separation was done as part of [HDDS-4242](https://issues.apache.org/jira/browse/HDDS-4242), under [HDDS-4216](https://issues.apache.org/jira/browse/HDDS-4216) which are supposed to separate RPC and storage proto files for compatibility reasons by introducing interface-storage package. But it seems only PrefixInfo and UserVolumeInfo have been done for this. If these changes are intended, this can be closed and we can instead migrate the other OzoneAcls to use the one in interface-storage.
Yes, separation / duplication was intended. I guess we should proceed with HDDS-4465 instead of merging them back.
--
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
Re: [PR] HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 closed pull request #6068: HDDS-10181. Remove duplicate ACL protobuf definitions in interface-storage
URL: https://github.com/apache/ozone/pull/6068
--
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