chihsuan commented on code in PR #10873:
URL: https://github.com/apache/ozone/pull/10873#discussion_r3694541370
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSecretKeysApi.java:
##########
@@ -183,11 +98,11 @@ public void testSecretKeyApiSuccess() throws Exception {
enableBlockToken();
// set a low rotation period, of 1s, expiry is 3s, expect 3 active keys
// at any moment.
- conf.set(HDDS_SECRET_KEY_ROTATE_CHECK_DURATION, "100ms");
- conf.set(HDDS_SECRET_KEY_ROTATE_DURATION, "1s");
- conf.set(HDDS_SECRET_KEY_EXPIRY_DURATION, "3000ms");
- conf.set(DELEGATION_TOKEN_MAX_LIFETIME_KEY, "1500ms");
- conf.set(DELEGATION_REMOVER_SCAN_INTERVAL_KEY, "100ms");
+ getConf().set(HDDS_SECRET_KEY_ROTATE_CHECK_DURATION, "100ms");
+ getConf().set(HDDS_SECRET_KEY_ROTATE_DURATION, "1s");
+ getConf().set(HDDS_SECRET_KEY_EXPIRY_DURATION, "3000ms");
+ getConf().set(DELEGATION_TOKEN_MAX_LIFETIME_KEY, "1500ms");
+ getConf().set(DELEGATION_REMOVER_SCAN_INTERVAL_KEY, "100ms");
Review Comment:
nit: Suggest assigning `getConf()` to a local conf variable to reduce the
mechanical diff here and in similar places when it is used repeatedly.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/AbstractKerberosTest.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * 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;
+
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_PRINCIPAL_KEY;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_KEYTAB_FILE;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod.KERBEROS;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.nio.file.Files;
+import java.util.Properties;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.UserGroupInformation;
+
+/**
+ * Shared MiniKdc / Kerberos setup for secure cluster integration tests
(HDDS-15913).
+ */
+public abstract class AbstractKerberosTest {
Review Comment:
Maybe rename this to `KerberosTests` to follow the convention introduced in
[#10672](https://github.com/apache/ozone/pull/10672)?
I wonder whether it should also live under `org.apache.ozone.test`,
alongside the other shared test fixtures. cc @adoroszlai for your thoughts. 🙏
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/AbstractKerberosTest.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * 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;
+
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_PRINCIPAL_KEY;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_KEYTAB_FILE;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod.KERBEROS;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.nio.file.Files;
+import java.util.Properties;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.UserGroupInformation;
+
+/**
+ * Shared MiniKdc / Kerberos setup for secure cluster integration tests
(HDDS-15913).
+ */
+public abstract class AbstractKerberosTest {
+
+ private MiniKdc miniKdc;
+ private OzoneConfiguration conf;
+
+ private File workDir;
+
+ /** Service keytab (scm.keytab); shared by SCM/OM/DN when
useSharedServicePrincipal(). */
+ private File ozoneKeytab;
+ private File spnegoKeytab;
+ private File testUserKeytab;
+ private String testUserPrincipal;
+ /** e.g. scm/host@REALM when useSharedServicePrincipal(). */
+ private String ozonePrincipal;
+
+ /** Separate OM keytab, only used when useSharedServicePrincipal() is false.
*/
+ private File omKeytab;
+
+ protected OzoneConfiguration getConf() {
+ return conf;
+ }
+
+ protected void setConf(OzoneConfiguration conf) {
+ this.conf = conf;
+ }
+
+ protected File getOzoneKeytab() {
+ return ozoneKeytab;
+ }
+
+ protected String getOzonePrincipal() {
+ return ozonePrincipal;
+ }
+
+ protected File getTestUserKeytab() {
+ return testUserKeytab;
+ }
+
+ protected String getTestUserPrincipal() {
+ return testUserPrincipal;
+ }
+
+ protected void initKerberos() throws Exception {
+ startMiniKdc();
+ setSecureConfig();
+ createCredentialsInKDC();
+ }
+
+ protected void startMiniKdc() throws Exception {
+ if (workDir == null) {
+ workDir = Files.createTempDirectory("kerberos").toFile();
+ }
+ Properties securityProperties = MiniKdc.createConf();
+ miniKdc = new MiniKdc(securityProperties, workDir);
+ miniKdc.start();
+ }
+
+ protected void stopMiniKdc() {
+ if (miniKdc != null) {
+ miniKdc.stop();
+ }
+ FileUtils.deleteQuietly(workDir);
+ }
+
+ protected void createPrincipal(File keytab, String... principal)
+ throws Exception {
+ miniKdc.createPrincipal(keytab, principal);
+ }
+
+ protected void setSecureConfig() throws IOException {
+ conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
+ String host = InetAddress.getLocalHost().getCanonicalHostName()
+ .toLowerCase();
+
+ conf.set(HADOOP_SECURITY_AUTHENTICATION, kerberosAuthenticationValue());
+
+ String curUser = UserGroupInformation.getCurrentUser().getUserName();
+ conf.set(OZONE_ADMINISTRATORS, curUser);
+
+ String realm = miniKdc.getRealm();
+ String hostAndRealm = host + "@" + realm;
+
+ ozoneKeytab = new File(workDir, "scm.keytab");
+ spnegoKeytab = new File(workDir, "http.keytab");
+
+ if (useSharedServicePrincipal()) {
+ ozonePrincipal = "scm/" + hostAndRealm;
+ conf.set(HDDS_SCM_KERBEROS_PRINCIPAL_KEY, ozonePrincipal);
+ conf.set(OZONE_OM_KERBEROS_PRINCIPAL_KEY, ozonePrincipal);
+ conf.set(HDDS_SCM_KERBEROS_KEYTAB_FILE_KEY,
ozoneKeytab.getAbsolutePath());
+ conf.set(OZONE_OM_KERBEROS_KEYTAB_FILE_KEY,
ozoneKeytab.getAbsolutePath());
+ conf.set(HDDS_DATANODE_KERBEROS_PRINCIPAL_KEY, ozonePrincipal);
+ conf.set(HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY,
+ ozoneKeytab.getAbsolutePath());
+ } else {
+ ozonePrincipal = "scm/" + hostAndRealm;
+ conf.set(HDDS_SCM_KERBEROS_PRINCIPAL_KEY, ozonePrincipal);
+ conf.set(HDDS_SCM_KERBEROS_KEYTAB_FILE_KEY,
ozoneKeytab.getAbsolutePath());
+ omKeytab = new File(workDir, "om.keytab");
+ conf.set(OZONE_OM_KERBEROS_PRINCIPAL_KEY, "om/" + hostAndRealm);
+ conf.set(OZONE_OM_KERBEROS_KEYTAB_FILE_KEY, omKeytab.getAbsolutePath());
+ }
+
+ conf.set(HDDS_SCM_HTTP_KERBEROS_PRINCIPAL_KEY, "HTTP_SCM/" + hostAndRealm);
+ conf.set(OZONE_OM_HTTP_KERBEROS_PRINCIPAL_KEY, "HTTP_OM/" + hostAndRealm);
+ conf.set(HDDS_SCM_HTTP_KERBEROS_KEYTAB_FILE_KEY,
+ spnegoKeytab.getAbsolutePath());
+ conf.set(OZONE_OM_HTTP_KERBEROS_KEYTAB_FILE,
+ spnegoKeytab.getAbsolutePath());
+
+ if (createTestUserPrincipal()) {
+ testUserKeytab = new File(workDir, "testuser.keytab");
+ testUserPrincipal = "test@" + realm;
+ }
+
+ if (enableSecurityAuthorizationByDefault()) {
+ conf.setBoolean(HADOOP_SECURITY_AUTHORIZATION, true);
+ }
+ }
+
+ protected void createCredentialsInKDC() throws Exception {
+ SCMHTTPServerConfig httpServerConfig =
+ conf.getObject(SCMHTTPServerConfig.class);
+ createPrincipal(ozoneKeytab, conf.get(HDDS_SCM_KERBEROS_PRINCIPAL_KEY));
+ if (!useSharedServicePrincipal()) {
+ createPrincipal(omKeytab, conf.get(OZONE_OM_KERBEROS_PRINCIPAL_KEY));
+ }
+ createPrincipal(spnegoKeytab, httpServerConfig.getKerberosPrincipal());
+ if (createTestUserPrincipal()) {
+ createPrincipal(testUserKeytab, testUserPrincipal);
+ }
+ }
+
+ /** Whether SCM/OM (and optionally DN) share a single "scm/..." principal
and keytab. */
+ protected boolean useSharedServicePrincipal() {
+ return true;
+ }
+
+ protected boolean createTestUserPrincipal() {
+ return true;
+ }
+
+ protected boolean enableSecurityAuthorizationByDefault() {
+ return true;
+ }
+
+ protected String kerberosAuthenticationValue() {
+ return KERBEROS.name();
Review Comment:
nit: I understand the intent here is to keep the migration byte-for-byte,
but since we're updating this, it might be worth dropping this hook and using
`KERBEROS.name()` unconditionally, since authentication parsing is
case-insensitive. Fine to keep the existing literals in this PR.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/AbstractKerberosTest.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * 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;
+
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.scm.ScmConfig.ConfigStrings.HDDS_SCM_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_PRINCIPAL_KEY;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_KEYTAB_FILE;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTP_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY;
+import static
org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod.KERBEROS;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.nio.file.Files;
+import java.util.Properties;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.UserGroupInformation;
+
+/**
+ * Shared MiniKdc / Kerberos setup for secure cluster integration tests
(HDDS-15913).
+ */
+public abstract class AbstractKerberosTest {
+
+ private MiniKdc miniKdc;
+ private OzoneConfiguration conf;
+
+ private File workDir;
+
+ /** Service keytab (scm.keytab); shared by SCM/OM/DN when
useSharedServicePrincipal(). */
+ private File ozoneKeytab;
+ private File spnegoKeytab;
+ private File testUserKeytab;
+ private String testUserPrincipal;
+ /** e.g. scm/host@REALM when useSharedServicePrincipal(). */
+ private String ozonePrincipal;
+
+ /** Separate OM keytab, only used when useSharedServicePrincipal() is false.
*/
+ private File omKeytab;
+
+ protected OzoneConfiguration getConf() {
+ return conf;
+ }
+
+ protected void setConf(OzoneConfiguration conf) {
Review Comment:
Could the base class create the configuration via a `createOzoneConfig()`
hook instead? `setConf()` requires every subclass to initialize it correctly
and may otherwise cause an NPE.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java:
##########
@@ -163,14 +156,16 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Test class to for security enabled Ozone cluster.
*/
-final class TestSecureOzoneCluster {
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
Review Comment:
`PER_CLASS` also shares scm, om, and omClient across tests. Could we reset
these fields after each test, or keep only the KDC class-scoped?
--
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]
