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/10 05:58:56 UTC
[GitHub] [ozone] kerneltime opened a new pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
kerneltime opened a new pull request #2822:
URL: https://github.com/apache/ozone/pull/2822
## What changes were proposed in this pull request?
To avoid an S3G that sets S3 Auth via proto messages
talking to an older version of OM. Add a check to
validate OM's client protocol version.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5898
## How was this patch tested?
- [ ] TODO: Testing via Ozone HA deployment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750783336
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
The alternate client implementation which we decided against was for the client to discover the client version supported by OM and then change its behavior as per the spec.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750779275
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
We need the actual version string, going forward we can have higher versions so the presence is not enough.
For now we should be good. And as said old client talking to new server and new client talking to old server should be supported. So, technically clients based on version returned by server and not talking i think we donot need it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753598395
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_SECOND_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_SECOND_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ true),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ false),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "10.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_BOTH_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.9.1",
+ false),
+ VALID_EXPECTED_ONE_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ "100.0.1",
+ true),
+ VALID_EXPECTED_ONE_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ null,
+ false),
+ VALID_EXPECTED_TWO_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.0.1",
+ false);;
Review comment:
Minor NIT: looks like extra ";"
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750762977
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
We need the actual version string, going forward we can have higher versions so the presence is not enough.
Also, it is made configurable in case we need to override the version string in a deployment for any reason.
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753599057
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
Review comment:
null == no OM. This means there is a single OM.
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750780290
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
The server defines the protocol, it is saying that I am at `v2.0.0` which needs S3 auth to be set via proto messages. The client only speaks version `v2.0.0` and higher so if the OM is not reporting a version that it likes the client does not continue..
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750767587
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
Server is setting the protocol version that it supports for clients. OM client protocol is the protocol between OM and the client as supported by the OM.
OM has a hard coded value that is decided at compile time based on the config set in code. This string is also made available on the client side to change based on the `OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY`. Right now client will talk only if the version it expects is present, we should support being able to have multiple versions as part of the spec.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753598081
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_SECOND_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_SECOND_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ true),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ false),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "10.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_BOTH_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.9.1",
+ false),
+ VALID_EXPECTED_ONE_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ "100.0.1",
+ true),
+ VALID_EXPECTED_ONE_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ null,
+ false),
+ VALID_EXPECTED_TWO_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.0.1",
Review comment:
One test case needed here is
OM version return "" as proto default if no value is ""
That would test scenario of newer S3G/RpcClient older OM.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753602705
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -149,7 +149,17 @@
OZONE_OM_CLIENT_PROTOCOL_VERSION,
"1.0.1",
"1.0.1",
- false);;
+ false),
+ VALID_EXPECTED_TWO_OM_ONE_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "2.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_ONE_EMPTY(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
Review comment:
Thanks for adding.
Can we have both "", as in the case added "" is not compared, as first version is lower code returns false.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750778592
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
In general
Old client new server
New client old server are to be supported
But here we have a special case, where we are trying to protect new S3G talking to old OM. So, technically new RpcClient can talk to both old/new server. but here it is problem with new S3G talking to old OM.
So, server setting protocol version that supports sounds odd to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753602705
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -149,7 +149,17 @@
OZONE_OM_CLIENT_PROTOCOL_VERSION,
"1.0.1",
"1.0.1",
- false);;
+ false),
+ VALID_EXPECTED_TWO_OM_ONE_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "2.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_ONE_EMPTY(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
Review comment:
Thanks for adding.
Can we have both "", as in this case "" is not compared, as first version is lower code returns false.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753597936
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
Review comment:
Should nt this be false?
As one OM version is null
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753599183
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_SECOND_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_SECOND_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ true),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ false),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "10.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_BOTH_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.9.1",
+ false),
+ VALID_EXPECTED_ONE_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ "100.0.1",
+ true),
+ VALID_EXPECTED_ONE_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ null,
+ false),
+ VALID_EXPECTED_TWO_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.0.1",
+ false);;
Review comment:
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r751776893
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
I have moved to a version comparison scheme and should protect in a future safe way for only secure S3G communication.
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750763567
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -198,17 +198,7 @@
import static org.apache.hadoop.hdds.utils.HAUtils.getScmInfo;
import static org.apache.hadoop.ozone.OmUtils.MAX_TRXN_ID;
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.DFS_CONTAINER_RATIS_ENABLED_DEFAULT;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.DFS_CONTAINER_RATIS_ENABLED_KEY;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED_DEFAULT;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS_WILDCARD;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_KEY_PREALLOCATION_BLOCKS_MAX;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_KEY_PREALLOCATION_BLOCKS_MAX_DEFAULT;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.*;
Review comment:
Will fix this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750778592
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
In general
Old client -> new server
New client -> old server are to be supported
But here we have a special case, where we are trying to protect new S3G talking to old OM. So, technically new RpcClient can talk to both old/new server. but here it is problem with new S3G talking to old OM.
So, server setting protocol version that supports sounds odd to me, considering above guarranty.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750778592
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
In general
Old client new server
New client old server are to be supported
But here we have a special case, where we are trying to protect new S3G talking to old OM. So, technically new RpcClient can talk to both old/new server. but here it is problem with new S3G talking to old OM.
So, server setting protocol version that supports sounds odd to me, considering above guarranty.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753601631
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
Review comment:
Yes, missed reading the != null check when adding to serviceInfo 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] avijayanhwx commented on pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#issuecomment-975833271
Thanks for working on this @kerneltime, and the review @bharatviswa504 .
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750751475
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
And also with config it is confusing, Server is setting client protocol version.
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750767587
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
Server is setting the protocol version that it supports for clients. OM client protocol is the protocol between OM and the client as supported by the OM.
OM has a hard coded value that is decided at compile time based on the config set in code. This string is also made available on the client side to change based on the `OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY`. For now this is the presence of the string but the version itself is setup to support semantic versioning.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750779275
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
>We need the actual version string, going forward we can have higher versions so the presence is not enough.
For now we should be good. And as said old client talking to new server and new client talking to old server should be supported. So, technically clients based on version returned by server and not talking i think we donot need it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750779275
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
>We need the actual version string, going forward we can have higher versions so the presence is not enough.
For now we should be good in the scenario if it has the value. And as said old client talking to new server and new client talking to old server should be supported. So, technically clients based on version returned by server and not talking i think we donot need it. So, here this is not about client to OM, our goal was trying to protect new S3G talking to old OM.
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750779275
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
>We need the actual version string, going forward we can have higher versions so the presence is not enough.
For now we should be good in the scenario if it has the value. And as said old client talking to new server and new client talking to old server should be supported. So, technically clients based on version returned by server and not talking i think we donot need it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r750739453
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -198,17 +198,7 @@
import static org.apache.hadoop.hdds.utils.HAUtils.getScmInfo;
import static org.apache.hadoop.ozone.OmUtils.MAX_TRXN_ID;
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.DFS_CONTAINER_RATIS_ENABLED_DEFAULT;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.DFS_CONTAINER_RATIS_ENABLED_KEY;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED_DEFAULT;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS_WILDCARD;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_KEY_PREALLOCATION_BLOCKS_MAX;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_KEY_PREALLOCATION_BLOCKS_MAX_DEFAULT;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.*;
Review comment:
can we avoid import with *?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2601,6 +2592,10 @@ public OzoneManagerHttpServer getHttpServer() {
ServiceInfo.Builder peerOmServiceInfoBuilder = ServiceInfo.newBuilder()
.setNodeType(HddsProtos.NodeType.OM)
.setHostname(peerNode.getHostName())
+ // For now assume peer is at the same version.
+ // This field needs to be fetched from peer when rolling upgrades
+ // are implemented.
+ .setOmClientProtocolVersion(OZONE_OM_CLIENT_PROTOCOL_VERSION)
Review comment:
Why we need this based on config value, why can't we do this based on version which we run.
As here if OM is not returning version means, newer S3G cannot talk to it, if any version is being returned it can talk to OM.
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -200,6 +199,22 @@ public RpcClient(ConfigurationSource conf, String omServiceId)
// requests serviced by this client will need S3 Auth set.
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(
conf.getBoolean(S3Auth.S3_AUTH_CHECK, false));
+ String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
Review comment:
Do we need a config key OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY here?
Can we do something like this.
if (conf.getBoolean(S3Auth.S3_AUTH_CHECK, false)) {
// Only work when version is as expected
}
And also we need this only when RpcClient is used from S3G. So, using that flag should be good I believe.
--
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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753599131
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_SECOND_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_SECOND_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ true),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ false),
+ VALID_EXPECTED_TWO_OM_FIRST_OM_HIGHER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "10.0.1",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ VALID_EXPECTED_TWO_OM_BOTH_LOWER(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.9.1",
+ false),
+ VALID_EXPECTED_ONE_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ null,
+ true),
+ VALID_EXPECTED_TWO_OM_HIGHER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "100.0.1",
+ "100.0.1",
+ true),
+ VALID_EXPECTED_ONE_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ null,
+ false),
+ VALID_EXPECTED_TWO_OM_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "1.0.1",
Review comment:
Will add them
--
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] bharatviswa504 commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753601631
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -0,0 +1,197 @@
+/**
+ * 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.client.rpc;
+
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_CLIENT_PROTOCOL_VERSION;
+import static org.apache.hadoop.ozone.client.rpc.RpcClient.validateOmVersion;
+
+/**
+ * Run RPC Client tests.
+ */
+public class RpcClientTest {
+ private enum ValidateOmVersionTestCases {
+ NULL_EXPECTED_NO_OM(
+ null, // Expected version
+ null, // First OM Version
+ null, // Second OM Version
+ true), // Should validation pass
+ NULL_EXPECTED_ONE_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ NULL_EXPECTED_TWO_OM(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ NULL_EXPECTED_TWO_OM_SECOND_MISMATCH(
+ null,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ null,
+ "invalid",
+ "invalid",
+ true),
+ NULL_EXPECTED_TWO_OM_FIRST_MISMATCH(
+ null,
+ "invalid",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_NO_OM(
+ "",
+ null,
+ null,
+ true),
+ EMPTY_EXPECTED_ONE_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ true),
+ EMPTY_EXPECTED_TWO_OM(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ true),
+ EMPTY_EXPECTED_TWO_OM_MISMATCH(
+ "",
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "invalid",
+ true),
+ EMPTY_EXPECTED_TWO_OM_BOTH_MISMATCH(
+ "",
+ "invalid",
+ "invalid",
+ true),
+ VALID_EXPECTED_NO_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ null,
+ null,
+ false),
+ VALID_EXPECTED_ONE_OM(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
Review comment:
Yes, missed the != null check when adding to serviceInfo 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] kerneltime commented on a change in pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#discussion_r753603348
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/RpcClientTest.java
##########
@@ -149,7 +149,17 @@
OZONE_OM_CLIENT_PROTOCOL_VERSION,
"1.0.1",
"1.0.1",
- false);;
+ false),
+ VALID_EXPECTED_TWO_OM_ONE_LOWER_VERSION(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
+ "2.0.1",
+ false),
+ VALID_EXPECTED_TWO_OM_ONE_EMPTY(
+ OZONE_OM_CLIENT_PROTOCOL_VERSION,
+ "1.0.1",
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
[GitHub] [ozone] bharatviswa504 commented on pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#issuecomment-974759618
Test failures does not seem related, let me kick off another run.
--
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] bharatviswa504 commented on pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2822:
URL: https://github.com/apache/ozone/pull/2822#issuecomment-974759618
Test failures does not seem related, let me kick off another run.
--
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 merged pull request #2822: HDDS-5898 S3G in secure mode checks OM version.
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #2822:
URL: https://github.com/apache/ozone/pull/2822
--
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