diff --git a/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnection.java b/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnection.java index 4cf18994e33..de85fdde48e 100644 --- a/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnection.java +++ b/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnection.java @@ -282,7 +282,7 @@ public boolean deleteX509CertRecord(String provider, String instanceId, String s } @Override - public int deleteExpiredX509CertRecords(int expiryTimeMins) { + public int deleteExpiredX509CertRecords(int expiryTimeMins, int limit) { // with dynamo db there is no need to manually expunge expired // record since we have the TTL option enabled for our table, diff --git a/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnection.java b/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnection.java index 9f6a8cfb812..6f606af5a23 100644 --- a/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnection.java +++ b/libs/java/server_aws_common/src/main/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnection.java @@ -178,7 +178,7 @@ public boolean deleteSSHCertRecord(String instanceId, String service) { } @Override - public int deleteExpiredSSHCertRecords(int expiryTimeMins) { + public int deleteExpiredSSHCertRecords(int expiryTimeMins, int limit) { // with dynamo db there is no need to manually expunge expired // record since we have the TTL option enabled for our table, diff --git a/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnectionTest.java b/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnectionTest.java index ca0184d9882..1915c513f59 100644 --- a/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnectionTest.java +++ b/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBCertRecordStoreConnectionTest.java @@ -482,8 +482,8 @@ public void testDeleteX509RecordException() { @Test public void testdeleteExpiredX509CertRecords() { DynamoDBCertRecordStoreConnection dbConn = getDBConnection(); - assertEquals(dbConn.deleteExpiredX509CertRecords(100), 0); - assertEquals(dbConn.deleteExpiredX509CertRecords(100000), 0); + assertEquals(dbConn.deleteExpiredX509CertRecords(100, 0), 0); + assertEquals(dbConn.deleteExpiredX509CertRecords(100000, 0), 0); dbConn.close(); } diff --git a/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnectionTest.java b/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnectionTest.java index 0f5f1b16e0c..eab02f99ae5 100644 --- a/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnectionTest.java +++ b/libs/java/server_aws_common/src/test/java/io/athenz/server/aws/common/cert/impl/DynamoDBSSHRecordStoreConnectionTest.java @@ -258,8 +258,8 @@ public void testDeleteSSHRecordException() { @Test public void testDeleteExpiredSSHCertRecords() { DynamoDBSSHRecordStoreConnection dbConn = new DynamoDBSSHRecordStoreConnection(dynamoDB, tableName); - assertEquals(dbConn.deleteExpiredSSHCertRecords(100), 0); - assertEquals(dbConn.deleteExpiredSSHCertRecords(100000), 0); + assertEquals(dbConn.deleteExpiredSSHCertRecords(100, 0), 0); + assertEquals(dbConn.deleteExpiredSSHCertRecords(100000, 0), 0); dbConn.close(); } } diff --git a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/CertRecordStoreConnection.java b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/CertRecordStoreConnection.java index 29a695b202b..aeeb64e8f1e 100644 --- a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/CertRecordStoreConnection.java +++ b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/CertRecordStoreConnection.java @@ -70,9 +70,10 @@ public interface CertRecordStoreConnection extends Closeable { * considered expired if it hasn't been updated within the * specified number of minutes * @param expiryTimeMins expiry time in minutes + * @param limit delete limit * @return number of records deleted */ - int deleteExpiredX509CertRecords(int expiryTimeMins) throws ServerResourceException; + int deleteExpiredX509CertRecords(int expiryTimeMins, int limit) throws ServerResourceException; /** * Return all certificate records that failed to refresh after updating them with the current notification time and server. diff --git a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnection.java b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnection.java index 6945a8be511..6a982edc324 100644 --- a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnection.java +++ b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnection.java @@ -51,6 +51,8 @@ public class JDBCCertRecordStoreConnection implements CertRecordStoreConnection "WHERE provider=? AND instanceId=? AND service=?;"; private static final String SQL_DELETE_EXPIRED_X509_RECORDS = "DELETE FROM certificates " + "WHERE currentTime < ADDDATE(NOW(), INTERVAL -? MINUTE);"; + private static final String SQL_DELETE_EXPIRED_X509_RECORDS_WITH_LIMIT = "DELETE FROM certificates " + + "WHERE currentTime < ADDDATE(NOW(), INTERVAL -? MINUTE) LIMIT ?;"; // Get all records that didn't refresh and update notification time. // Query explanation: @@ -277,7 +279,7 @@ public boolean deleteX509CertRecord(String provider, String instanceId, String s } @Override - public int deleteExpiredX509CertRecords(int expiryTimeMins) throws ServerResourceException { + public int deleteExpiredX509CertRecords(int expiryTimeMins, int limit) throws ServerResourceException { int affectedRows; final String caller = "deleteExpiredX509CertRecords"; @@ -287,9 +289,16 @@ public int deleteExpiredX509CertRecords(int expiryTimeMins) throws ServerResourc if (expiryTimeMins <= 0) { return 0; } - - try (PreparedStatement ps = con.prepareStatement(SQL_DELETE_EXPIRED_X509_RECORDS)) { + + String sql = SQL_DELETE_EXPIRED_X509_RECORDS; + if (limit > 0) { + sql = SQL_DELETE_EXPIRED_X509_RECORDS_WITH_LIMIT; + } + try (PreparedStatement ps = con.prepareStatement(sql)) { ps.setInt(1, expiryTimeMins); + if (limit > 0) { + ps.setInt(2, limit); + } affectedRows = executeUpdate(ps, caller); } catch (SQLException ex) { throw sqlError(ex, caller); diff --git a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnection.java b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnection.java index 22541ca4e99..48003baf67d 100644 --- a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnection.java +++ b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnection.java @@ -40,6 +40,8 @@ public class JDBCSSHRecordStoreConnection implements SSHRecordStoreConnection { "WHERE instanceId=? AND service=?;"; private static final String SQL_DELETE_EXPIRED_X509_RECORDS = "DELETE FROM ssh_certificates " + "WHERE issueTime < ADDDATE(NOW(), INTERVAL -? MINUTE);"; + private static final String SQL_DELETE_EXPIRED_X509_RECORDS_WITH_LIMIT = "DELETE FROM ssh_certificates " + + "WHERE issueTime < ADDDATE(NOW(), INTERVAL -? MINUTE) LIMIT ?;"; public static final String DB_COLUMN_SERVICE = "service"; public static final String DB_COLUMN_CLIENT_IP = "clientIP"; @@ -190,7 +192,7 @@ public boolean deleteSSHCertRecord(String instanceId, String service) throws Ser } @Override - public int deleteExpiredSSHCertRecords(int expiryTimeMins) throws ServerResourceException { + public int deleteExpiredSSHCertRecords(int expiryTimeMins, int limit) throws ServerResourceException { int affectedRows; final String caller = "deleteExpiredSSHCertRecords"; @@ -200,9 +202,16 @@ public int deleteExpiredSSHCertRecords(int expiryTimeMins) throws ServerResource if (expiryTimeMins <= 0) { return 0; } - - try (PreparedStatement ps = con.prepareStatement(SQL_DELETE_EXPIRED_X509_RECORDS)) { + + String sql = SQL_DELETE_EXPIRED_X509_RECORDS; + if (limit > 0) { + sql = SQL_DELETE_EXPIRED_X509_RECORDS_WITH_LIMIT; + } + try (PreparedStatement ps = con.prepareStatement(sql)) { ps.setInt(1, expiryTimeMins); + if (limit > 0) { + ps.setInt(2, limit); + } affectedRows = executeUpdate(ps, caller); } catch (SQLException ex) { throw sqlError(ex, caller); diff --git a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/ssh/SSHRecordStoreConnection.java b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/ssh/SSHRecordStoreConnection.java index d7d888b42e8..1c81f2eb5f2 100644 --- a/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/ssh/SSHRecordStoreConnection.java +++ b/libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/ssh/SSHRecordStoreConnection.java @@ -67,7 +67,8 @@ public interface SSHRecordStoreConnection extends Closeable { * considered expired if it hasn't been updated within the * specified number of minutes * @param expiryTimeMins expiry time in minutes + * @param limit maximum number of records to delete * @return number of records deleted */ - int deleteExpiredSSHCertRecords(int expiryTimeMins) throws ServerResourceException; + int deleteExpiredSSHCertRecords(int expiryTimeMins, int limit) throws ServerResourceException; } diff --git a/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnectionTest.java b/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnectionTest.java index 048c79f0770..df47a35b239 100644 --- a/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnectionTest.java +++ b/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCCertRecordStoreConnectionTest.java @@ -418,15 +418,34 @@ public void testConnectionCloseException() throws SQLException { @Test public void testdeleteExpiredX509CertRecords() throws Exception { + final String SQL_NO_LIMIT = "DELETE FROM certificates " + + "WHERE currentTime < ADDDATE(NOW(), INTERVAL -? MINUTE);"; JDBCCertRecordStoreConnection jdbcConn = new JDBCCertRecordStoreConnection(mockConn); Mockito.doReturn(1).when(mockPrepStmt).executeUpdate(); - jdbcConn.deleteExpiredX509CertRecords(360); + jdbcConn.deleteExpiredX509CertRecords(360, 0); + Mockito.verify(mockConn, times(1)).prepareStatement(SQL_NO_LIMIT); Mockito.verify(mockPrepStmt, times(1)).setInt(1, 360); jdbcConn.close(); } + + @Test + public void testdeleteExpiredX509CertRecordsWithLimit() throws Exception { + final String SQL_WITH_LIMIT = "DELETE FROM certificates " + + "WHERE currentTime < ADDDATE(NOW(), INTERVAL -? MINUTE) LIMIT ?;"; + + JDBCCertRecordStoreConnection jdbcConn = new JDBCCertRecordStoreConnection(mockConn); + + Mockito.doReturn(1000).when(mockPrepStmt).executeUpdate(); + jdbcConn.deleteExpiredX509CertRecords(360, 1000); + + Mockito.verify(mockConn, times(1)).prepareStatement(SQL_WITH_LIMIT); + Mockito.verify(mockPrepStmt, times(1)).setInt(1, 360); + Mockito.verify(mockPrepStmt, times(1)).setInt(2, 1000); + jdbcConn.close(); + } @Test public void testdeleteExpiredX509CertRecordsInvalidValue() throws Exception { @@ -434,7 +453,7 @@ public void testdeleteExpiredX509CertRecordsInvalidValue() throws Exception { JDBCCertRecordStoreConnection jdbcConn = new JDBCCertRecordStoreConnection(mockConn); Mockito.doReturn(1).when(mockPrepStmt).executeUpdate(); - jdbcConn.deleteExpiredX509CertRecords(0); + jdbcConn.deleteExpiredX509CertRecords(0, 0); Mockito.verify(mockPrepStmt, times(0)).setInt(1, 0); jdbcConn.close(); @@ -447,7 +466,7 @@ public void testdeleteExpiredX509CertRecordsException() throws SQLException { Mockito.when(mockPrepStmt.executeUpdate()).thenThrow(new SQLException("exc", "exc", 101)); try { - jdbcConn.deleteExpiredX509CertRecords(360); + jdbcConn.deleteExpiredX509CertRecords(360, 0); fail(); } catch (ServerResourceException ex) { Assert.assertEquals(ex.getCode(), ServerResourceException.INTERNAL_SERVER_ERROR); diff --git a/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnectionTest.java b/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnectionTest.java index a6fa44aa228..78213aab3d6 100644 --- a/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnectionTest.java +++ b/libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/cert/impl/JDBCSSHRecordStoreConnectionTest.java @@ -339,15 +339,35 @@ public void testConnectionCloseException() throws SQLException { @Test public void testdeleteExpiredSSHCertRecords() throws Exception { + final String SQL_NO_LIMIT = "DELETE FROM ssh_certificates " + + "WHERE issueTime < ADDDATE(NOW(), INTERVAL -? MINUTE);"; JDBCSSHRecordStoreConnection jdbcConn = new JDBCSSHRecordStoreConnection(mockConn); Mockito.doReturn(1).when(mockPrepStmt).executeUpdate(); - jdbcConn.deleteExpiredSSHCertRecords(360); + jdbcConn.deleteExpiredSSHCertRecords(360, 0); + Mockito.verify(mockConn, times(1)).prepareStatement(SQL_NO_LIMIT); Mockito.verify(mockPrepStmt, times(1)).setInt(1, 360); jdbcConn.close(); } + + @Test + public void testdeleteExpiredSSHCertRecordsWithLimit() throws Exception { + + final String SQL_WITH_LIMIT = "DELETE FROM ssh_certificates " + + "WHERE issueTime < ADDDATE(NOW(), INTERVAL -? MINUTE) LIMIT ?;"; + + JDBCSSHRecordStoreConnection jdbcConn = new JDBCSSHRecordStoreConnection(mockConn); + + Mockito.doReturn(1).when(mockPrepStmt).executeUpdate(); + jdbcConn.deleteExpiredSSHCertRecords(360, 1000); + + Mockito.verify(mockConn, times(1)).prepareStatement(SQL_WITH_LIMIT); + Mockito.verify(mockPrepStmt, times(1)).setInt(1, 360); + Mockito.verify(mockPrepStmt, times(1)).setInt(2, 1000); + jdbcConn.close(); + } @Test public void testdeleteExpiredSSHCertRecordsInvalidValue() throws Exception { @@ -355,7 +375,7 @@ public void testdeleteExpiredSSHCertRecordsInvalidValue() throws Exception { JDBCSSHRecordStoreConnection jdbcConn = new JDBCSSHRecordStoreConnection(mockConn); Mockito.doReturn(1).when(mockPrepStmt).executeUpdate(); - jdbcConn.deleteExpiredSSHCertRecords(0); + jdbcConn.deleteExpiredSSHCertRecords(0, 0); Mockito.verify(mockPrepStmt, times(0)).setInt(1, 0); jdbcConn.close(); @@ -368,7 +388,7 @@ public void testdeleteExpiredSSHCertRecordsException() throws SQLException { Mockito.when(mockPrepStmt.executeUpdate()).thenThrow(new SQLException("exc", "exc", 101)); try { - jdbcConn.deleteExpiredSSHCertRecords(360); + jdbcConn.deleteExpiredSSHCertRecords(360, 0); fail(); } catch (ServerResourceException ex) { Assert.assertEquals(ex.getCode(), ServerResourceException.INTERNAL_SERVER_ERROR); diff --git a/servers/zts/conf/zts.properties b/servers/zts/conf/zts.properties index a4fdc1e7205..7ca45f937e8 100644 --- a/servers/zts/conf/zts.properties +++ b/servers/zts/conf/zts.properties @@ -797,3 +797,16 @@ athenz.zts.k8s_provider_distribution_validator_factory_class=com.yahoo.athenz.in # the server allows the certificate signer module to impose the limit. The certificate # signer will either honor that value or lower it based on its own configuration. #athenz.zts.service_cert_default_expiry_mins=0 + +# This property specifies the maximum number of records that the CertRecordCleaner +# can delete in a single execution. If set to 0, there is no limit on the number of records +# that can be deleted. +#athenz.zts.cert_record_cleaner_limit=0 + +# These properties define the interval at which the CertRecordCleaner is executed. +# The cert_record_cleaner_duration property specifies the duration as an integer value, +# and the cert_record_cleaner_timeunit property determines the unit of time. +# The supported time units are: second, minute, hour, or day. The interval is calculated +# as duration * timeunit, and the CertRecordCleaner will run at this defined interval. +#athenz.zts.cert_record_cleaner_duration=1 +#athenz.zts.cert_record_cleaner_timeunit=day diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java index 27bdb5e797a..4dbe4934dc7 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java @@ -137,6 +137,9 @@ public final class ZTSConsts { public static final String ZTS_PROP_SSH_USER_CA_CERT_KEYID_FNAME = "athenz.zts.ssh_user_ca_cert_keyid_fname"; public static final String ZTS_PROP_RESP_X509_SIGNER_CERTS = "athenz.zts.resp_x509_signer_certs"; public static final String ZTS_PROP_RESP_SSH_SIGNER_CERTS = "athenz.zts.resp_ssh_signer_certs"; + public static final String ZTS_PROP_CERT_RECORD_CLEANER_LIMIT = "athenz.zts.cert_record_cleaner_limit"; + public static final String ZTS_PROP_CERT_RECORD_CLEANER_DURATION = "athenz.zts.cert_record_cleaner_duration"; + public static final String ZTS_PROP_CERT_RECORD_CLEANER_TIMEUNIT = "athenz.zts.cert_record_cleaner_timeunit"; public static final String DB_PROP_USER = "user"; public static final String DB_PROP_PASSWORD = "password"; diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/InstanceCertManager.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/InstanceCertManager.java index de36433debb..b8982ec8a91 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/InstanceCertManager.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/InstanceCertManager.java @@ -150,26 +150,44 @@ public InstanceCertManager(final PrivateKeyStore keyStore, Authorizer authorizer // start our thread to delete expired cert records once a day // unless we're running in read-only mode thus no modifications // to the database + final int limit = Integer.parseInt(System.getProperty(ZTSConsts.ZTS_PROP_CERT_RECORD_CLEANER_LIMIT, "0")); + final int duration = Integer.parseInt(System.getProperty(ZTSConsts.ZTS_PROP_CERT_RECORD_CLEANER_DURATION, "1")); + final TimeUnit timeUnit = parseTimeUnit(System.getProperty(ZTSConsts.ZTS_PROP_CERT_RECORD_CLEANER_TIMEUNIT, "day")); if (certStore != null && certSigner != null) { certScheduledExecutor = Executors.newScheduledThreadPool(1); certScheduledExecutor.scheduleAtFixedRate( - new ExpiredX509CertRecordCleaner(certStore, certSigner.getMaxCertExpiryTimeMins(), readOnlyMode), - 0, 1, TimeUnit.DAYS); + new ExpiredX509CertRecordCleaner(certStore, certSigner.getMaxCertExpiryTimeMins(), limit, readOnlyMode), + 0, duration, timeUnit); } if (sshStore != null) { int expiryTimeMins = (int) TimeUnit.MINUTES.convert(30, TimeUnit.DAYS); sshScheduledExecutor = Executors.newScheduledThreadPool(1); sshScheduledExecutor.scheduleAtFixedRate( - new ExpiredSSHCertRecordCleaner(sshStore, expiryTimeMins, readOnlyMode), - 0, 1, TimeUnit.DAYS); + new ExpiredSSHCertRecordCleaner(sshStore, expiryTimeMins, limit, readOnlyMode), + 0, duration, timeUnit); } // check to see if we have it configured to validate IP addresses validateIPAddress = new DynamicConfigBoolean(CONFIG_MANAGER, ZTSConsts.ZTS_PROP_SSH_CERT_VALIDATE_IP, false); } + + static TimeUnit parseTimeUnit(String timeUnitStr) { + switch (timeUnitStr) { + case "second": + return TimeUnit.SECONDS; + case "minute": + return TimeUnit.MINUTES; + case "hour": + return TimeUnit.HOURS; + case "day": + return TimeUnit.DAYS; + default: + return TimeUnit.DAYS; + } + } void shutdown() { if (certScheduledExecutor != null) { @@ -1417,11 +1435,13 @@ static class ExpiredX509CertRecordCleaner implements Runnable { private final CertRecordStore store; private final int expiryTimeMins; + private final int limit; private final DynamicConfigBoolean readOnlyMode; - public ExpiredX509CertRecordCleaner(CertRecordStore store, int expiryTimeMins, DynamicConfigBoolean readOnlyMode) { + public ExpiredX509CertRecordCleaner(CertRecordStore store, int expiryTimeMins, int limit, DynamicConfigBoolean readOnlyMode) { this.store = store; this.expiryTimeMins = expiryTimeMins; + this.limit = limit; this.readOnlyMode = readOnlyMode; } @@ -1452,7 +1472,7 @@ int cleanupExpiredX509CertRecords() { int deletedRecords; try (CertRecordStoreConnection storeConnection = store.getConnection()) { - deletedRecords = storeConnection.deleteExpiredX509CertRecords(expiryTimeMins); + deletedRecords = storeConnection.deleteExpiredX509CertRecords(expiryTimeMins, limit); } catch (ServerResourceException ex) { throw ZTSUtils.error(ex); } @@ -1464,11 +1484,13 @@ static class ExpiredSSHCertRecordCleaner implements Runnable { private final SSHRecordStore store; private final int expiryTimeMins; + private final int limit; private final DynamicConfigBoolean readOnlyMode; - public ExpiredSSHCertRecordCleaner(SSHRecordStore store, int expiryTimeMins, DynamicConfigBoolean readOnlyMode) { + public ExpiredSSHCertRecordCleaner(SSHRecordStore store, int expiryTimeMins, int limit, DynamicConfigBoolean readOnlyMode) { this.store = store; this.expiryTimeMins = expiryTimeMins; + this.limit = limit; this.readOnlyMode = readOnlyMode; } @@ -1499,7 +1521,7 @@ int cleanupExpiredSSHCertRecords() { int deletedRecords; try (SSHRecordStoreConnection storeConnection = store.getConnection()) { - deletedRecords = storeConnection.deleteExpiredSSHCertRecords(expiryTimeMins); + deletedRecords = storeConnection.deleteExpiredSSHCertRecords(expiryTimeMins, limit); } catch (ServerResourceException ex) { throw ZTSUtils.error(ex); } diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnection.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnection.java index 82993cd9c95..b6940b851e4 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnection.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnection.java @@ -79,7 +79,7 @@ public boolean deleteX509CertRecord(String provider, String instanceId, String s } @Override - public int deleteExpiredX509CertRecords(int expiryTimeMins) { + public int deleteExpiredX509CertRecords(int expiryTimeMins, int limit) { String[] fnames = rootDir.list(); if (fnames == null) { return 0; @@ -98,6 +98,9 @@ public int deleteExpiredX509CertRecords(int expiryTimeMins) { //noinspection ResultOfMethodCallIgnored file.delete(); count += 1; + if (limit == count) { + break; + } } return count; } diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnection.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnection.java index 96164355619..0d16dbf8c70 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnection.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnection.java @@ -77,7 +77,7 @@ public boolean deleteSSHCertRecord(String instanceId, String service) { } @Override - public int deleteExpiredSSHCertRecords(int expiryTimeMins) { + public int deleteExpiredSSHCertRecords(int expiryTimeMins, int limit) { String[] fnames = rootDir.list(); if (fnames == null) { return 0; @@ -96,6 +96,9 @@ public int deleteExpiredSSHCertRecords(int expiryTimeMins) { //noinspection ResultOfMethodCallIgnored file.delete(); count += 1; + if (count == limit) { + break; + } } return count; } diff --git a/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/InstanceCertManagerTest.java b/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/InstanceCertManagerTest.java index 12d2268eaa5..76be4c0d88f 100644 --- a/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/InstanceCertManagerTest.java +++ b/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/InstanceCertManagerTest.java @@ -8,6 +8,7 @@ import java.security.cert.X509Certificate; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import com.fasterxml.jackson.databind.ObjectMapper; @@ -1464,7 +1465,7 @@ public void testExpiredX509CertRecordCleaner() throws ServerResourceException { when(store.getConnection()).thenThrow(new RuntimeException("invalid connection")); InstanceCertManager.ExpiredX509CertRecordCleaner cleaner = - new InstanceCertManager.ExpiredX509CertRecordCleaner(store, 100, new DynamicConfigBoolean(false)); + new InstanceCertManager.ExpiredX509CertRecordCleaner(store, 100, 0, new DynamicConfigBoolean(false)); // make sure no exceptions are thrown @@ -1479,7 +1480,7 @@ public void testExpiredSSHCertRecordCleaner() { assertNotNull(store); InstanceCertManager.ExpiredSSHCertRecordCleaner cleaner = - new InstanceCertManager.ExpiredSSHCertRecordCleaner(store, 100, new DynamicConfigBoolean(false)); + new InstanceCertManager.ExpiredSSHCertRecordCleaner(store, 100, 0, new DynamicConfigBoolean(false)); // make sure no exceptions are thrown @@ -1493,7 +1494,7 @@ public void testExpiredSSHCertRecordCleanerException() throws ServerResourceExce when(store.getConnection()).thenThrow(new RuntimeException("invalid connection")); InstanceCertManager.ExpiredSSHCertRecordCleaner cleaner = - new InstanceCertManager.ExpiredSSHCertRecordCleaner(store, 100, new DynamicConfigBoolean(false)); + new InstanceCertManager.ExpiredSSHCertRecordCleaner(store, 100, 0, new DynamicConfigBoolean(false)); // make sure no exceptions are thrown @@ -2358,14 +2359,14 @@ public void testX509CertOperationFailures() throws ServerResourceException, IOEx .thenThrow(new ServerResourceException(400, "Invalid delete request")); when(certConnection.updateUnrefreshedCertificatesNotificationTimestamp(anyString(), anyLong(), anyString())) .thenThrow(new ServerResourceException(400, "Invalid update unrefreshed cert request")); - when(certConnection.deleteExpiredX509CertRecords(anyInt())) + when(certConnection.deleteExpiredX509CertRecords(anyInt(), anyInt())) .thenThrow(new ServerResourceException(400, "Invalid delete expired certs request")); instance.setCertStore(certStore); // verify cleaner runs without any exceptions InstanceCertManager.ExpiredX509CertRecordCleaner cleaner = - new InstanceCertManager.ExpiredX509CertRecordCleaner(certStore, 100, new DynamicConfigBoolean(false)); + new InstanceCertManager.ExpiredX509CertRecordCleaner(certStore, 100, 0, new DynamicConfigBoolean(false)); cleaner.run(); try { @@ -2432,14 +2433,14 @@ public void testSSHCertOperationFailures() throws ServerResourceException { .thenThrow(new ServerResourceException(400, "Invalid get request")); when(sshRecordStoreConnection.updateSSHCertRecord(any())) .thenThrow(new ServerResourceException(400, "Invalid update request")); - when(sshRecordStoreConnection.deleteExpiredSSHCertRecords(anyInt())) + when(sshRecordStoreConnection.deleteExpiredSSHCertRecords(anyInt(), anyInt())) .thenThrow(new ServerResourceException(400, "Invalid delete expired certs request")); instance.setSSHStore(sshRecordStore); // verify cleaner runs without any exceptions InstanceCertManager.ExpiredSSHCertRecordCleaner cleaner = - new InstanceCertManager.ExpiredSSHCertRecordCleaner(sshRecordStore, 100, new DynamicConfigBoolean(false)); + new InstanceCertManager.ExpiredSSHCertRecordCleaner(sshRecordStore, 100, 0, new DynamicConfigBoolean(false)); cleaner.run(); try { @@ -2494,4 +2495,14 @@ public void testWorkloadOperationFailures() throws ServerResourceException { instance.shutdown(); } + + @Test + public void testParseTimeUnit() { + assertEquals(InstanceCertManager.parseTimeUnit("second"), TimeUnit.SECONDS); + assertEquals(InstanceCertManager.parseTimeUnit("minute"), TimeUnit.MINUTES); + assertEquals(InstanceCertManager.parseTimeUnit("hour"), TimeUnit.HOURS); + assertEquals(InstanceCertManager.parseTimeUnit("days"), TimeUnit.DAYS); + assertEquals(InstanceCertManager.parseTimeUnit("invalidstring"), TimeUnit.DAYS); + } + } diff --git a/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnectionTest.java b/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnectionTest.java index 81c698081d7..fb1f5efab31 100644 --- a/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnectionTest.java +++ b/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileCertRecordStoreConnectionTest.java @@ -174,18 +174,67 @@ public void testdeleteExpiredX509CertRecords() throws Exception { assertNotNull(certRecordCheck); // Verify that certificates are not expired immediately - con.deleteExpiredX509CertRecords(43200); //30 days + con.deleteExpiredX509CertRecords(43200, 0); //30 days certRecordCheck = con.getX509CertRecord("ostk", "instance-id", "cn"); assertNotNull(certRecordCheck); Thread.sleep(1000); - con.deleteExpiredX509CertRecords(0); + con.deleteExpiredX509CertRecords(0, 0); certRecordCheck = con.getX509CertRecord("ostk", "instance-id", "cn"); assertNull(certRecordCheck); con.close(); } + @Test + public void testdeleteExpiredX509CertRecordsWithLimit() throws Exception { + + // make sure the directory does not exist + + ZTSTestUtils.deleteDirectory(new File("/tmp/zts-cert-tests")); + + FileCertRecordStore store = new FileCertRecordStore(new File("/tmp/zts-cert-tests")); + FileCertRecordStoreConnection con = (FileCertRecordStoreConnection) store.getConnection(); + assertNotNull(con); + + Date now = new Date(); + X509CertRecord certRecord = new X509CertRecord(); + + certRecord.setService("cn"); + certRecord.setProvider("ostk"); + certRecord.setInstanceId("instance-id-001"); + certRecord.setCurrentIP("current-ip"); + certRecord.setCurrentSerial("current-serial"); + certRecord.setCurrentTime(now); + certRecord.setPrevIP("prev-ip"); + certRecord.setPrevSerial("prev-serial"); + certRecord.setPrevTime(now); + + assertTrue(con.insertX509CertRecord(certRecord)); + + certRecord = new X509CertRecord(); + certRecord.setService("cn"); + certRecord.setProvider("ostk"); + certRecord.setInstanceId("instance-id-002"); + certRecord.setCurrentIP("current-ip"); + certRecord.setCurrentSerial("current-serial"); + certRecord.setCurrentTime(now); + certRecord.setPrevIP("prev-ip"); + certRecord.setPrevSerial("prev-serial"); + certRecord.setPrevTime(now); + + assertTrue(con.insertX509CertRecord(certRecord)); + + Thread.sleep(1000); + int deleted = con.deleteExpiredX509CertRecords(0, 1); + assertEquals(deleted, 1); + if (con.getX509CertRecord("ostk", "instance-id-001", "cn") == null && con.getX509CertRecord("ostk", "instance-id-002", "cn") == null) { + fail(); + } + + con.close(); + } + @Test public void testDeleteExpiredX509CertRecords() { @@ -201,7 +250,7 @@ public void testDeleteExpiredX509CertRecords() { Mockito.when(dir.list()).thenReturn(null); con.rootDir = dir; - assertEquals(con.deleteExpiredX509CertRecords(0), 0); + assertEquals(con.deleteExpiredX509CertRecords(0, 0), 0); } @Test @@ -235,7 +284,7 @@ public void testdeleteExpiredX509CertRecordsDelete() throws Exception { assertNotNull(certRecordCheck); Thread.sleep(1000); - store.deleteExpiredX509CertRecords(0); + store.deleteExpiredX509CertRecords(0, 0); certRecordCheck = store.getX509CertRecord("ostk", "instance-id", "cn"); assertNotNull(certRecordCheck); diff --git a/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnectionTest.java b/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnectionTest.java index 729650ac2c6..3f00d678521 100644 --- a/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnectionTest.java +++ b/servers/zts/src/test/java/com/yahoo/athenz/zts/cert/impl/FileSSHRecordStoreConnectionTest.java @@ -144,13 +144,52 @@ public void testDeleteExpiredSSHCertRecords() throws Exception { assertNotNull(certRecordCheck); Thread.sleep(1000); - con.deleteExpiredSSHCertRecords(0); + con.deleteExpiredSSHCertRecords(0, 0); certRecordCheck = con.getSSHCertRecord("instance-id", "cn"); assertNull(certRecordCheck); con.close(); } + @Test + public void testDeleteExpiredSSHCertRecordsWithLimit() throws Exception { + + // make sure the directory does not exist + + ZTSTestUtils.deleteDirectory(new File("/tmp/zts-ssh-tests")); + + FileSSHRecordStore store = new FileSSHRecordStore(new File("/tmp/zts-ssh-tests")); + FileSSHRecordStoreConnection con = (FileSSHRecordStoreConnection) store.getConnection(); + assertNotNull(con); + + SSHCertRecord certRecord = new SSHCertRecord(); + certRecord.setInstanceId("instance-id-001"); + certRecord.setService("cn"); + certRecord.setPrincipals("host1,host2"); + certRecord.setClientIP("10.10.10.11"); + certRecord.setPrivateIP("10.10.10.12"); + + assertTrue(con.insertSSHCertRecord(certRecord)); + + certRecord = new SSHCertRecord(); + certRecord.setInstanceId("instance-id-002"); + certRecord.setService("cn"); + certRecord.setPrincipals("host1,host2"); + certRecord.setClientIP("10.10.10.11"); + certRecord.setPrivateIP("10.10.10.12"); + + assertTrue(con.insertSSHCertRecord(certRecord)); + + + Thread.sleep(1000); + int deleted = con.deleteExpiredSSHCertRecords(0, 1); + assertEquals(deleted, 1); + if (con.getSSHCertRecord("instance-id-001", "cn") == null && con.getSSHCertRecord("instance-id-002", "cn") == null) { + fail(); + } + con.close(); + } + @Test public void testDeleteExpiredSSHCertRecordsDelete() throws Exception { @@ -175,7 +214,7 @@ public void testDeleteExpiredSSHCertRecordsDelete() throws Exception { assertNotNull(certRecordCheck); Thread.sleep(1000); - store.deleteExpiredSSHCertRecords(0); + store.deleteExpiredSSHCertRecords(0, 0); certRecordCheck = store.getSSHCertRecord("instance-id", "cn"); assertNotNull(certRecordCheck); @@ -196,6 +235,6 @@ public void testDeleteExpiredSSHCertRecordsNoDir() { Mockito.when(dir.list()).thenReturn(null); con.rootDir = dir; - assertEquals(con.deleteExpiredSSHCertRecords(0), 0); + assertEquals(con.deleteExpiredSSHCertRecords(0, 0), 0); } }