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/06/14 07:27:21 UTC
[GitHub] [ozone] kerneltime opened a new pull request #2329: Hdds 5315
kerneltime opened a new pull request #2329:
URL: https://github.com/apache/ozone/pull/2329
## What changes were proposed in this pull request?
Adding a basic unit test to validate tokens are skipped when persisting.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5315
## How was this patch tested?
Unit tested
--
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.
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] cxorm commented on pull request #2329: Hdds 5315 Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
cxorm commented on pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#issuecomment-860111078
Hi @kerneltime, thank you for the work.
Could you please file a new jira to address this unit work ?
I think it make sense to file a new jira if you don't mind.
--
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.
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 closed pull request #2329: HDDS-5315. Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
kerneltime closed pull request #2329:
URL: https://github.com/apache/ozone/pull/2329
--
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 #2329: Hdds 5315 Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#discussion_r650724083
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyCreation.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
+import org.apache.hadoop.hdds.security.token.OzoneBlockTokenIdentifier;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.DBConfigFromFile;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.ozone.om.helpers.*;
+import org.apache.hadoop.ozone.om.request.TestOMRequestUtils;
+import org.apache.hadoop.ozone.security.OzoneBlockTokenSecretManager;
+import org.apache.hadoop.security.token.Token;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.UUID;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test key creation.
+ * This file hosts tests purely focused on key creation.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class TestKeyCreation {
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+
+ private OzoneConfiguration createConfAndInitValues() throws IOException {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ File newFolder = folder.newFolder();
+ if (!newFolder.exists()) {
+ Assert.assertTrue(newFolder.mkdirs());
+ }
+ System.setProperty(DBConfigFromFile.CONFIG_DIR, "/");
+ ServerUtils.setOzoneMetaDirPath(conf, newFolder.toString());
+ conf.setBoolean(HDDS_BLOCK_TOKEN_ENABLED, true);
+ conf.setQuietMode(false);
+ return conf;
+ }
+
+ /**
+ * checkSkippingOfBlockTokens tests if block security tokens are not persisted
+ * along with the key.
+ *
+ * @throws IOException
+ */
+ @Test(timeout = 5000)
+ public void checkSkippingOfBlockTokens()
+ throws IOException {
+ // 1. Create an object with security manager and token set
+ // 2. Read the object back and any token should be generated new
+ // and not persisted.
+ OzoneConfiguration conf = createConfAndInitValues();
+ OmMetadataManagerImpl metaMgr = new OmMetadataManagerImpl(conf);
+ OzoneBlockTokenSecretManager secretManager =
+ mock(OzoneBlockTokenSecretManager.class);
+ Token<OzoneBlockTokenIdentifier> token = new Token<>();
+ token.setKind(new Text("to-be-persisted"));
+ when(secretManager.
+ generateToken(anyString(), any(), any(), anyLong())).
+ thenReturn(token);
+ KeyManager keyManager =
+ new KeyManagerImpl(
+ new ScmBlockLocationTestingClient(null, null, 0),
+ metaMgr, conf, UUID.randomUUID().toString(), secretManager);
+ keyManager.start(conf);
+ String volumeName = String.format("volume%s",
+ RandomStringUtils.randomAlphanumeric(5));
+ String bucketName = String.format("bucket%s",
+ RandomStringUtils.randomAlphanumeric(5));
+ String keyName = String.format("key%s",
+ RandomStringUtils.randomAlphanumeric(5));
+ TestOMRequestUtils.addVolumeToOM(keyManager.getMetadataManager(),
+ OmVolumeArgs.newBuilder()
+ .setOwnerName("o")
+ .setAdminName("a")
+ .setVolume(volumeName)
+ .build());
+
+ TestOMRequestUtils.addBucketToOM(keyManager.getMetadataManager(),
+ OmBucketInfo.newBuilder().setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .build());
+
+ OmKeyArgs arg =
+ new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setAcls(Collections.emptyList())
+ .setReplicationConfig(new StandaloneReplicationConfig(
+ HddsProtos.ReplicationFactor.ONE))
+ .setLocationInfoList(new ArrayList<>())
+ .build();
+ OpenKeySession session = keyManager.openKey(arg);
+ for (int i = 0; i < 100; i++) {
+ arg.addLocationInfo(
+ keyManager.allocateBlock(arg, session.getId(), new ExcludeList()));
+ }
+ keyManager.commitKey(arg, session.getId());
Review comment:
Can you avoid using these API's for write requests we should use OMKey*Request? (For write requests we don't use any of the code from (Volume/Bucket/Key)Manager classes.
These methods need to be removed, but we have never got to that work.
--
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.
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] cxorm edited a comment on pull request #2329: Hdds 5315 Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
cxorm edited a comment on pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#issuecomment-860111078
Hi @kerneltime, thank you for the work.
Could you please file a new jira to address this unit test ?
I think it make sense to file a new jira if you don't mind.
--
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.
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 pull request #2329: HDDS-5315. Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
kerneltime commented on pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#issuecomment-948156966
Closing this PR for now. Will revisit it once the code clean up is 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] kerneltime commented on pull request #2329: Hdds 5315 Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
kerneltime commented on pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#issuecomment-860017187
> @kerneltime
>
> * You can find checkstyle violations in the [output of the failing check](https://github.com/apache/ozone/pull/2329/checks?check_run_id=2806213374#step:5:7).
> * Instead of force pushing please prefer incremental commits to fix issues after you have opened the PR. These make CI history more easily accessible.
I incorporated the check style scripts that are part of the source code and the checks pass locally as well as here.
Not sure I follow which part of the CI history would you like to refer to.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #2329: HDDS-5315. Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#issuecomment-938348239
/pending avoid using KeyManager for write request
--
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 #2329: Hdds 5315 Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#discussion_r650950953
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyCreation.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
+import org.apache.hadoop.hdds.security.token.OzoneBlockTokenIdentifier;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.DBConfigFromFile;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.ozone.om.helpers.*;
+import org.apache.hadoop.ozone.om.request.TestOMRequestUtils;
+import org.apache.hadoop.ozone.security.OzoneBlockTokenSecretManager;
+import org.apache.hadoop.security.token.Token;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.UUID;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test key creation.
+ * This file hosts tests purely focused on key creation.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class TestKeyCreation {
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+
+ private OzoneConfiguration createConfAndInitValues() throws IOException {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ File newFolder = folder.newFolder();
+ if (!newFolder.exists()) {
+ Assert.assertTrue(newFolder.mkdirs());
+ }
+ System.setProperty(DBConfigFromFile.CONFIG_DIR, "/");
+ ServerUtils.setOzoneMetaDirPath(conf, newFolder.toString());
+ conf.setBoolean(HDDS_BLOCK_TOKEN_ENABLED, true);
+ conf.setQuietMode(false);
+ return conf;
+ }
+
+ /**
+ * checkSkippingOfBlockTokens tests if block security tokens are not persisted
+ * along with the key.
+ *
+ * @throws IOException
+ */
+ @Test(timeout = 5000)
+ public void checkSkippingOfBlockTokens()
+ throws IOException {
+ // 1. Create an object with security manager and token set
+ // 2. Read the object back and any token should be generated new
+ // and not persisted.
+ OzoneConfiguration conf = createConfAndInitValues();
+ OmMetadataManagerImpl metaMgr = new OmMetadataManagerImpl(conf);
+ OzoneBlockTokenSecretManager secretManager =
+ mock(OzoneBlockTokenSecretManager.class);
+ Token<OzoneBlockTokenIdentifier> token = new Token<>();
+ token.setKind(new Text("to-be-persisted"));
+ when(secretManager.
+ generateToken(anyString(), any(), any(), anyLong())).
+ thenReturn(token);
+ KeyManager keyManager =
+ new KeyManagerImpl(
+ new ScmBlockLocationTestingClient(null, null, 0),
+ metaMgr, conf, UUID.randomUUID().toString(), secretManager);
+ keyManager.start(conf);
+ String volumeName = String.format("volume%s",
+ RandomStringUtils.randomAlphanumeric(5));
+ String bucketName = String.format("bucket%s",
+ RandomStringUtils.randomAlphanumeric(5));
+ String keyName = String.format("key%s",
+ RandomStringUtils.randomAlphanumeric(5));
+ TestOMRequestUtils.addVolumeToOM(keyManager.getMetadataManager(),
+ OmVolumeArgs.newBuilder()
+ .setOwnerName("o")
+ .setAdminName("a")
+ .setVolume(volumeName)
+ .build());
+
+ TestOMRequestUtils.addBucketToOM(keyManager.getMetadataManager(),
+ OmBucketInfo.newBuilder().setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .build());
+
+ OmKeyArgs arg =
+ new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setAcls(Collections.emptyList())
+ .setReplicationConfig(new StandaloneReplicationConfig(
+ HddsProtos.ReplicationFactor.ONE))
+ .setLocationInfoList(new ArrayList<>())
+ .build();
+ OpenKeySession session = keyManager.openKey(arg);
+ for (int i = 0; i < 100; i++) {
+ arg.addLocationInfo(
+ keyManager.allocateBlock(arg, session.getId(), new ExcludeList()));
+ }
+ keyManager.commitKey(arg, session.getId());
Review comment:
Ok
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #2329: Hdds 5315 Add basic unit tests for skipping tokens
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2329:
URL: https://github.com/apache/ozone/pull/2329#issuecomment-859902650
--
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.
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