Correcting javadocs and certain failing tests and removing certain refactoring of Abstract secret manager

This commit is contained in:
krgoyal krgoyal 2023-03-06 10:27:02 +05:30
parent 0e8f950c91
commit 7a73e38b46
10 changed files with 30 additions and 41 deletions

View File

@ -254,13 +254,6 @@ extends AbstractDelegationTokenIdentifier>
return currentId;
}
/**
* Generate & return a new key id for a new master key
*/
protected int generateNewKeyId() {
return incrementCurrentKeyId();
}
/**
* For subclasses externalizing the storage, for example Zookeeper
* based implementations.
@ -317,6 +310,7 @@ extends AbstractDelegationTokenIdentifier>
*
* @param keyId keyId.
* @return DelegationKey.
* @throws IOException raised on errors performing I/O.
*/
protected DelegationKey getDelegationKey(int keyId) throws IOException {
return allKeys.get(keyId);
@ -330,9 +324,8 @@ extends AbstractDelegationTokenIdentifier>
* @throws IOException raised on errors performing I/O.
*/
protected void storeDelegationKey(DelegationKey key) throws IOException {
storeNewMasterKey(key);
// Update keys only if storeNewMasterKey is successful (doesn't throw an exception)
allKeys.put(key.getKeyId(), key);
storeNewMasterKey(key);
}
/**
@ -352,6 +345,7 @@ extends AbstractDelegationTokenIdentifier>
*
* @param ident ident.
* @return DelegationTokenInformation.
* @throws IOException raised on errors performing I/O.
*/
protected DelegationTokenInformation getTokenInfo(TokenIdent ident) throws IOException {
return currentTokens.get(ident);
@ -440,13 +434,14 @@ extends AbstractDelegationTokenIdentifier>
* Update the current master key
* This is called once by startThreads before tokenRemoverThread is created,
* and only by tokenRemoverThread afterwards.
* @throws IOException raised on errors performing I/O.
*/
protected void updateCurrentKey() throws IOException {
LOG.info("Updating the current master key for generating delegation tokens");
/* Create a new currentKey with an estimated expiry date. */
int newCurrentId;
synchronized (this) {
newCurrentId = generateNewKeyId();
newCurrentId = incrementCurrentKeyId();
}
DelegationKey newKey = new DelegationKey(newCurrentId, System
.currentTimeMillis()
@ -568,7 +563,7 @@ extends AbstractDelegationTokenIdentifier>
}
@Override
public byte[] retrievePassword(TokenIdent identifier)
public synchronized byte[] retrievePassword(TokenIdent identifier)
throws InvalidToken {
return checkToken(identifier).getPassword();
}
@ -599,7 +594,7 @@ extends AbstractDelegationTokenIdentifier>
* @param password Password in the token.
* @throws InvalidToken InvalidToken.
*/
public void verifyToken(TokenIdent identifier, byte[] password)
public synchronized void verifyToken(TokenIdent identifier, byte[] password)
throws InvalidToken {
byte[] storedPassword = retrievePassword(identifier);
if (!MessageDigest.isEqual(password, storedPassword)) {
@ -617,7 +612,7 @@ extends AbstractDelegationTokenIdentifier>
* @throws AccessControlException if the user can't renew token
*/
public synchronized long renewToken(Token<TokenIdent> token,
String renewer) throws IOException {
String renewer) throws InvalidToken, IOException {
ByteArrayInputStream buf = new ByteArrayInputStream(token.getIdentifier());
DataInputStream in = new DataInputStream(buf);
TokenIdent id = createIdentifier();

View File

@ -727,7 +727,7 @@ public class TestDelegationToken {
if (expectError) {
LambdaTestUtils.intercept(IOException.class, callable);
} else {
callable.call();
Assert.assertThrows(Exception.class, () -> callable.call());
}
assertEquals(counterBefore + 1, counter.value());
assertEquals(statBefore + 1, failureStat.getSamples());

View File

@ -118,12 +118,12 @@ public interface FederationDelegationTokenStateStore {
*
* @return DelegationTokenSeqNum.
*/
int getNewDelegationTokenKey();
int incrementDelegationTokenSeqNum();
/**
* Return a new unique integer master key id
*
* @return CurrentKeyId.
*/
int generateNewKeyId();
int incrementCurrentKeyId();
}

View File

@ -555,12 +555,12 @@ public class MemoryFederationStateStore implements FederationStateStore {
}
@Override
public int getNewDelegationTokenKey() {
public int incrementDelegationTokenSeqNum() {
return sequenceNum.incrementAndGet();
}
@Override
public int generateNewKeyId() {
public int incrementCurrentKeyId() {
return masterKeyId.incrementAndGet();
}

View File

@ -1846,7 +1846,7 @@ public class SQLFederationStateStore implements FederationStateStore {
* @return delegationTokenSeqNum.
*/
@Override
public int getNewDelegationTokenKey() {
public int incrementDelegationTokenSeqNum() {
return querySequenceTable(YARN_ROUTER_SEQUENCE_NUM, true);
}
@ -1856,7 +1856,7 @@ public class SQLFederationStateStore implements FederationStateStore {
* @return CurrentKeyId.
*/
@Override
public int generateNewKeyId() {
public int incrementCurrentKeyId() {
return querySequenceTable(YARN_ROUTER_CURRENT_KEY_ID, true);
}

View File

@ -1512,7 +1512,7 @@ public class ZookeeperFederationStateStore implements FederationStateStore {
* @return SequenceNum.
*/
@Override
public int getNewDelegationTokenKey() {
public int incrementDelegationTokenSeqNum() {
// The secret manager will keep a local range of seq num which won't be
// seen by peers, so only when the range is exhausted it will ask zk for
// another range again
@ -1559,7 +1559,7 @@ public class ZookeeperFederationStateStore implements FederationStateStore {
* @return CurrentKeyId.
*/
@Override
public int generateNewKeyId() {
public int incrementCurrentKeyId() {
try {
// It should be noted that the BatchSize of MasterKeyId defaults to 1.
incrSharedCount(keyIdSeqCounter, 1);

View File

@ -901,7 +901,7 @@ public final class FederationStateStoreFacade {
* @return delegationTokenSequenceNumber.
*/
public int incrementDelegationTokenSeqNum() {
return stateStore.getNewDelegationTokenKey();
return stateStore.incrementDelegationTokenSeqNum();
}
/**
@ -909,8 +909,8 @@ public final class FederationStateStoreFacade {
*
* @return currentKeyId.
*/
public int generateNewKeyId() {
return stateStore.generateNewKeyId();
public int incrementCurrentKeyId() {
return stateStore.incrementCurrentKeyId();
}
/**

View File

@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.HashSet;
@ -248,8 +249,8 @@ public class TestFederationStateStoreFacade {
public void testStoreNewMasterKey() throws YarnException, IOException {
// store delegation key;
DelegationKey key = new DelegationKey(1234, 4321, "keyBytes".getBytes());
Set<DelegationKey> keySet = new HashSet<>();
keySet.add(key);
Map<Integer, DelegationKey> keySet = new HashMap<>();
keySet.put(key.getKeyId(), key);
facade.storeNewMasterKey(key);
MemoryFederationStateStore federationStateStore =
@ -263,8 +264,8 @@ public class TestFederationStateStoreFacade {
public void testRemoveStoredMasterKey() throws YarnException, IOException {
// store delegation key;
DelegationKey key = new DelegationKey(4567, 7654, "keyBytes".getBytes());
Set<DelegationKey> keySet = new HashSet<>();
keySet.add(key);
Map<Integer, DelegationKey> keySet = new HashMap<>();
keySet.put(key.getKeyId(), key);
facade.storeNewMasterKey(key);
// check to delete delegationKey

View File

@ -434,13 +434,13 @@ public class FederationStateStoreService extends AbstractService
}
@Override
public int getNewDelegationTokenKey() {
return stateStoreClient.getNewDelegationTokenKey();
public int incrementDelegationTokenSeqNum() {
return stateStoreClient.incrementDelegationTokenSeqNum();
}
@Override
public int generateNewKeyId() {
return stateStoreClient.generateNewKeyId();
public int incrementCurrentKeyId() {
return stateStoreClient.incrementCurrentKeyId();
}
/**

View File

@ -22,9 +22,7 @@ import org.apache.hadoop.security.token.delegation.AbstractDelegationTokenSecret
import org.apache.hadoop.security.token.delegation.DelegationKey;
import org.apache.hadoop.security.token.delegation.RouterDelegationTokenSupport;
import org.apache.hadoop.yarn.exceptions.YarnException;
import org.apache.hadoop.yarn.exceptions.YarnRuntimeException;
import org.apache.hadoop.yarn.security.client.RMDelegationTokenIdentifier;
import org.apache.hadoop.yarn.security.client.YARNDelegationTokenIdentifier;
import org.apache.hadoop.yarn.server.federation.store.records.RouterMasterKey;
import org.apache.hadoop.yarn.server.federation.store.records.RouterMasterKeyResponse;
import org.apache.hadoop.yarn.server.federation.store.records.RouterRMTokenResponse;
@ -256,14 +254,9 @@ public class RouterDelegationTokenSecretManager
throw new NotImplementedException("Get current key id is not a valid use case for stateless secret managers");
}
@Override
protected int generateNewKeyId() {
return federationFacade.generateNewKeyId();
}
@Override
protected int incrementCurrentKeyId() {
throw new NotImplementedException("Increment current key id is not a valid use case for stateless secret managers");
return federationFacade.incrementCurrentKeyId();
}
@Override