NIFI-3004 Added failing unit test to SSLContextServiceTest to demonstrate that customValidate caches result from previous validation.

NIFI-3004 Added logic to expire StandardSSLContextService customValidate cache after 5 invocations.
Updated unit test to demonstrate this logic.

This closes #1375.

Signed-off-by: Andy LoPresto <alopresto@apache.org>

nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/main/java/org/apache/nifi/ssl/StandardSSLContextService.java

@@ -101,7 +101,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme
             .name("key-password")
             .displayName("Key Password")
             .description("The password for the key. If this is not specified, but the Keystore Filename, Password, and Type are specified, "
-                + "then the Keystore Password will be assumed to be the same as the Key Password.")
+                    + "then the Keystore Password will be assumed to be the same as the Key Password.")
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .sensitive(true)
             .required(false)
@@ -120,6 +120,10 @@ public class StandardSSLContextService extends AbstractControllerService impleme
     private ConfigurationContext configContext;
     private boolean isValidated;
 
+    // TODO: This can be made configurable if necessary
+    private static final int VALIDATION_CACHE_EXPIRATION = 5;
+    private int validationCacheCount = 0;
+
     static {
         List<PropertyDescriptor> props = new ArrayList<>();
         props.add(KEYSTORE);
@@ -165,7 +169,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme
     @Override
     public void onPropertyModified(PropertyDescriptor descriptor, String oldValue, String newValue) {
         super.onPropertyModified(descriptor, oldValue, newValue);
-        isValidated = false;
+        resetValidationCache();
     }
 
     private static Validator createFileExistsAndReadableValidator() {
@@ -208,8 +212,13 @@ public class StandardSSLContextService extends AbstractControllerService impleme
     protected Collection<ValidationResult> customValidate(ValidationContext validationContext) {
         final Collection<ValidationResult> results = new ArrayList<>();
 
-        if(isValidated) {
+        if (isValidated) {
-            return results;
+            validationCacheCount++;
+            if (validationCacheCount > VALIDATION_CACHE_EXPIRATION) {
+                resetValidationCache();
+            } else {
+                return results;
+            }
         }
 
         results.addAll(validateStore(validationContext.getProperties(), KeystoreValidationGroup.KEYSTORE));
@@ -246,6 +255,15 @@ public class StandardSSLContextService extends AbstractControllerService impleme
         return results;
     }
 
+    private void resetValidationCache() {
+        validationCacheCount = 0;
+        isValidated = false;
+    }
+
+    protected int getValidationCacheExpiration() {
+        return VALIDATION_CACHE_EXPIRATION;
+    }
+
     private void verifySslConfig(final ValidationContext validationContext) throws ProcessException {
         final String protocol = validationContext.getProperty(SSL_ALGORITHM).getValue();
         try {
@@ -266,7 +284,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme
                 SslContextFactory.createSslContext(
                         validationContext.getProperty(KEYSTORE).getValue(),
                         validationContext.getProperty(KEYSTORE_PASSWORD).getValue().toCharArray(),
-                    keyPassword,
+                        keyPassword,
                         validationContext.getProperty(KEYSTORE_TYPE).getValue(),
                         protocol);
                 return;
@@ -275,7 +293,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme
             SslContextFactory.createSslContext(
                     validationContext.getProperty(KEYSTORE).getValue(),
                     validationContext.getProperty(KEYSTORE_PASSWORD).getValue().toCharArray(),
-                keyPassword,
+                    keyPassword,
                     validationContext.getProperty(KEYSTORE_TYPE).getValue(),
                     validationContext.getProperty(TRUSTSTORE).getValue(),
                     validationContext.getProperty(TRUSTSTORE_PASSWORD).getValue().toCharArray(),
@@ -447,7 +465,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme
         return count;
     }
 
-    public static enum KeystoreValidationGroup {
+    public enum KeystoreValidationGroup {
 
         KEYSTORE, TRUSTSTORE
     }

nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/java/org/apache/nifi/ssl/SSLContextServiceTest.java

@@ -16,17 +16,36 @@
  */
 package org.apache.nifi.ssl;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
 import org.apache.nifi.reporting.InitializationException;
 import org.apache.nifi.ssl.SSLContextService.ClientAuth;
+import org.apache.nifi.util.MockProcessContext;
+import org.apache.nifi.util.MockValidationContext;
 import org.apache.nifi.util.TestRunner;
 import org.apache.nifi.util.TestRunners;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class SSLContextServiceTest {
+    private static final Logger logger = LoggerFactory.getLogger(SSLContextServiceTest.class);
+
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder(new File("src/test/resources"));
 
     @Test
     public void testBad1() throws InitializationException {
@@ -132,7 +151,7 @@ public class SSLContextServiceTest {
         runner.assertValid(service);
 
         runner.disableControllerService(service);
-        runner.setProperty(service,StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/DOES-NOT-EXIST.jks");
+        runner.setProperty(service, StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/DOES-NOT-EXIST.jks");
         runner.assertNotValid(service);
 
         runner.setProperty(service, StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/localhost-ks.jks");
@@ -144,6 +163,67 @@ public class SSLContextServiceTest {
         runner.assertValid(service);
     }
 
+    @Test
+    public void testValidationResultsCacheShouldExpire() throws InitializationException, IOException {
+        // Arrange
+
+        // Copy the keystore and truststore to a tmp directory so the originals are not modified
+        File originalKeystore = new File("src/test/resources/localhost-ks.jks");
+        File originalTruststore = new File("src/test/resources/localhost-ts.jks");
+
+        File tmpKeystore = tmp.newFile("keystore-tmp.jks");
+        File tmpTruststore = tmp.newFile("truststore-tmp.jks");
+
+        Files.copy(originalKeystore.toPath(), tmpKeystore.toPath(), StandardCopyOption.REPLACE_EXISTING);
+        Files.copy(originalTruststore.toPath(), tmpTruststore.toPath(), StandardCopyOption.REPLACE_EXISTING);
+
+        final TestRunner runner = TestRunners.newTestRunner(TestProcessor.class);
+        SSLContextService service = new StandardSSLContextService();
+        final String serviceIdentifier = "test-should-expire";
+        runner.addControllerService(serviceIdentifier, service);
+        runner.setProperty(service, StandardSSLContextService.KEYSTORE.getName(), tmpKeystore.getAbsolutePath());
+        runner.setProperty(service, StandardSSLContextService.KEYSTORE_PASSWORD.getName(), "localtest");
+        runner.setProperty(service, StandardSSLContextService.KEYSTORE_TYPE.getName(), "JKS");
+        runner.setProperty(service, StandardSSLContextService.TRUSTSTORE.getName(), tmpTruststore.getAbsolutePath());
+        runner.setProperty(service, StandardSSLContextService.TRUSTSTORE_PASSWORD.getName(), "localtest");
+        runner.setProperty(service, StandardSSLContextService.TRUSTSTORE_TYPE.getName(), "JKS");
+        runner.enableControllerService(service);
+
+        runner.setProperty("SSL Context Svc ID", serviceIdentifier);
+        runner.assertValid(service);
+
+        final StandardSSLContextService sslContextService = (StandardSSLContextService) service;
+
+        // Act
+        boolean isDeleted = tmpKeystore.delete();
+        assert isDeleted;
+        assert !tmpKeystore.exists();
+        logger.info("Deleted keystore file");
+
+        // Manually validate the service (expecting cached result to be returned)
+        final MockProcessContext processContext = (MockProcessContext) runner.getProcessContext();
+        // This service does not use the state manager or variable registry
+        final ValidationContext validationContext = new MockValidationContext(processContext, null, null);
+
+        // Even though the keystore file is no longer present, because no property changed, the cached result is still valid
+        Collection<ValidationResult> validationResults = sslContextService.customValidate(validationContext);
+        assertTrue("validation results is not empty", validationResults.isEmpty());
+        logger.info("(1) StandardSSLContextService#customValidate() returned true even though the keystore file is no longer available");
+
+        // Assert
+
+        // Have to exhaust the cached result by checking n-1 more times
+        for (int i = 2; i < sslContextService.getValidationCacheExpiration(); i++) {
+            validationResults = sslContextService.customValidate(validationContext);
+            assertTrue("validation results is not empty", validationResults.isEmpty());
+            logger.info("(" + i + ") StandardSSLContextService#customValidate() returned true even though the keystore file is no longer available");
+        }
+
+        validationResults = sslContextService.customValidate(validationContext);
+        assertFalse("validation results is empty", validationResults.isEmpty());
+        logger.info("(" + sslContextService.getValidationCacheExpiration() + ") StandardSSLContextService#customValidate() returned false because the cache expired");
+    }
+
     @Test
     public void testGoodTrustOnly() {
         try {
@@ -159,7 +239,7 @@ public class SSLContextServiceTest {
             runner.setProperty("SSL Context Svc ID", "test-good2");
             runner.assertValid();
             Assert.assertNotNull(service);
-            Assert.assertTrue(service instanceof StandardSSLContextService);
+            assertTrue(service instanceof StandardSSLContextService);
             service.createSSLContext(ClientAuth.NONE);
         } catch (InitializationException e) {
         }
@@ -180,7 +260,7 @@ public class SSLContextServiceTest {
             runner.setProperty("SSL Context Svc ID", "test-good3");
             runner.assertValid();
             Assert.assertNotNull(service);
-            Assert.assertTrue(service instanceof StandardSSLContextService);
+            assertTrue(service instanceof StandardSSLContextService);
             SSLContextService sslService = service;
             sslService.createSSLContext(ClientAuth.NONE);
         } catch (Exception e) {
@@ -205,7 +285,7 @@ public class SSLContextServiceTest {
             runner.setProperty("SSL Context Svc ID", "test-diff-keys");
             runner.assertValid();
             Assert.assertNotNull(service);
-            Assert.assertTrue(service instanceof StandardSSLContextService);
+            assertTrue(service instanceof StandardSSLContextService);
             SSLContextService sslService = service;
             sslService.createSSLContext(ClientAuth.NONE);
         } catch (Exception e) {