ashishkumar50 commented on code in PR #4747:
URL: https://github.com/apache/ozone/pull/4747#discussion_r1203769732
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java:
##########
@@ -377,6 +386,23 @@ private static void setSecureConfig() throws IOException {
ozoneKeytab.getAbsolutePath());
}
+ @Test
+ public void testRotateKeySCMAdminCommand()
+ throws InterruptedException, TimeoutException, IOException {
+ GenericTestUtils.waitFor(() -> cluster.getScmLeader() != null, 100, 1000);
+ InetSocketAddress address =
+ cluster.getScmLeader().getClientRpcAddress();
+ String hostPort = address.getHostName() + ":" + address.getPort();
+ String[] args = {"scm", "rotate", "--scm", hostPort};
+
+ String oldKey =
+ scmClient.getSecretKeyClient().getCurrentSecretKey().toString();
Review Comment:
May be we can add a small test here, run twice "getCurrentSecretKey" and
assert whether both are same before rotating.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.admin.scm;
+
+import java.io.IOException;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient;
+import org.apache.hadoop.hdds.scm.cli.ScmSubcommand;
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+import picocli.CommandLine;
+
+/**
+ * Handler of ozone admin scm rotate command.
+ */
[email protected](
+ name = "rotate",
+ description = "CLI command to force generate new keys (rotate)",
+ mixinStandardHelpOptions = true,
+ versionProvider = HddsVersionProvider.class)
+public class RotateKeySubCommand extends ScmSubcommand {
+
+ @CommandLine.ParentCommand
+ private ScmAdmin parent;
+
+ @Override
+ protected void execute(ScmClient scmClient) throws IOException {
+ try (ScmClient client = new ContainerOperationClient(
+ parent.getParent().getOzoneConf())) {
+ boolean status = false;
+ try {
+ status = client.checkAndRotate(true);
+ } catch (TimeoutException e) {
+ e.printStackTrace();
+ }
+ System.out.println(
+ "Secret key rotation is complete, a new key has been generated. " +
+ "Rotate Status: " + status);
Review Comment:
When there is TimeoutException, This message will not be correct to show.
Instead we can print different message in catch and return from there.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/protocol/SecretKeyProtocolScm.java:
##########
@@ -28,4 +31,13 @@
clientPrincipal = HDDS_SCM_KERBEROS_PRINCIPAL_KEY
)
public interface SecretKeyProtocolScm extends SecretKeyProtocol {
+
+ /**
+ * Force generates new secret keys (rotate).
+ *
+ * @param force boolean flag that forcefully rotates the key on demand
+ * @return
+ * @throws TimeoutException
Review Comment:
Please Correct javadoc for @return and @throws.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/symmetric/SecretKeyManager.java:
##########
@@ -109,12 +109,13 @@ public boolean isInitialized() {
*
* @return true if rotation actually happens, false if it doesn't.
*/
- public synchronized boolean checkAndRotate() throws TimeoutException {
+ public synchronized boolean checkAndRotate(boolean force)
+ throws TimeoutException {
// Initialize the state if it's not initialized already.
checkAndInitialize();
ManagedSecretKey currentKey = state.getCurrentKey();
- if (shouldRotate(currentKey)) {
+ if (shouldRotate(currentKey) || force) {
Review Comment:
nit: we can reverse condition "if (force || shouldRotate(currentKey))", to
avoid method call if force is true.
##########
hadoop-hdds/interface-server/src/main/proto/ScmSecretKeyProtocol.proto:
##########
@@ -98,6 +103,11 @@ message SCMGetSecretKeyRequest {
required UUID secretKeyId = 1;
}
+message SCMGetCheckAndRotateRequest {
+ optional bool force = 1;
Review Comment:
We can change to optional bool force = 1 [default = false];
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.admin.scm;
+
+import java.io.IOException;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient;
+import org.apache.hadoop.hdds.scm.cli.ScmSubcommand;
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+import picocli.CommandLine;
+
+/**
+ * Handler of ozone admin scm rotate command.
+ */
[email protected](
+ name = "rotate",
+ description = "CLI command to force generate new keys (rotate)",
+ mixinStandardHelpOptions = true,
+ versionProvider = HddsVersionProvider.class)
+public class RotateKeySubCommand extends ScmSubcommand {
+
+ @CommandLine.ParentCommand
+ private ScmAdmin parent;
+
+ @Override
+ protected void execute(ScmClient scmClient) throws IOException {
+ try (ScmClient client = new ContainerOperationClient(
+ parent.getParent().getOzoneConf())) {
+ boolean status = false;
+ try {
+ status = client.checkAndRotate(true);
+ } catch (TimeoutException e) {
Review Comment:
checkAndRotate can even throw IOException, we can handle and show proper
message too.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.admin.scm;
+
+import java.io.IOException;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient;
+import org.apache.hadoop.hdds.scm.cli.ScmSubcommand;
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+import picocli.CommandLine;
+
+/**
+ * Handler of ozone admin scm rotate command.
+ */
[email protected](
+ name = "rotate",
+ description = "CLI command to force generate new keys (rotate)",
+ mixinStandardHelpOptions = true,
+ versionProvider = HddsVersionProvider.class)
+public class RotateKeySubCommand extends ScmSubcommand {
+
+ @CommandLine.ParentCommand
+ private ScmAdmin parent;
+
+ @Override
+ protected void execute(ScmClient scmClient) throws IOException {
+ try (ScmClient client = new ContainerOperationClient(
+ parent.getParent().getOzoneConf())) {
+ boolean status = false;
+ try {
+ status = client.checkAndRotate(true);
+ } catch (TimeoutException e) {
+ e.printStackTrace();
+ }
+ System.out.println(
+ "Secret key rotation is complete, a new key has been generated. " +
+ "Rotate Status: " + status);
+ }
+ }
+}
Review Comment:
Add a new line at the end of file.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
