NIFI-5368 controller services validated prior to enabling; referenced controller services must be enabled for referencing component to be valid (mock framework)

This closes #2873.

Signed-off-by: Mark Payne <markap14@hotmail.com>

nifi-mock/src/main/java/org/apache/nifi/util/MockControllerServiceLookup.java

@@ -29,6 +29,10 @@ public abstract class MockControllerServiceLookup implements ControllerServiceLo
 
     private final Map<String, ControllerServiceConfiguration> controllerServiceMap = new ConcurrentHashMap<>();
 
+    public Map<String, ControllerServiceConfiguration> getControllerServices() {
+        return controllerServiceMap;
+    }
+
     public ControllerServiceConfiguration addControllerService(final ControllerService service, final String identifier) {
         final ControllerServiceConfiguration config = new ControllerServiceConfiguration(service);
         controllerServiceMap.put(identifier, config);

nifi-mock/src/main/java/org/apache/nifi/util/MockProcessContext.java

@@ -249,6 +249,16 @@ public class MockProcessContext extends MockControllerServiceLookup implements S
 
         final Collection<ValidationResult> serviceResults = validateReferencedControllerServices(validationContext);
         results.addAll(serviceResults);
+
+        // verify all controller services are enabled
+        for (Map.Entry<String, ControllerServiceConfiguration> service : getControllerServices().entrySet()) {
+            if (!service.getValue().isEnabled()) {
+                results.add(new ValidationResult.Builder()
+                        .explanation("Controller service " + service.getKey() + " for " + this.getName() + " is not enabled")
+                        .valid(false)
+                        .build());
+            }
+        }
         return results;
     }
 

nifi-mock/src/main/java/org/apache/nifi/util/StandardProcessorTestRunner.java

@@ -686,6 +686,16 @@ public class StandardProcessorTestRunner implements TestRunner {
             throw new IllegalStateException("Cannot enable Controller Service " + service + " because it is not disabled");
         }
 
+        // ensure controller service is valid before enabling
+        final ValidationContext validationContext = new MockValidationContext(context).getControllerServiceValidationContext(service);
+        final Collection<ValidationResult> results = context.getControllerService(service.getIdentifier()).validate(validationContext);
+
+        for (final ValidationResult result : results) {
+            if (!result.isValid()) {
+                throw new IllegalStateException("Cannot enable Controller Service " + service + " because it is in an invalid state: " + result.toString());
+            }
+        }
+
         try {
             final ConfigurationContext configContext = new MockConfigurationContext(service, configuration.getProperties(), context,variableRegistry);
             ReflectionUtils.invokeMethodsWithAnnotation(OnEnabled.class, service, configContext);
@@ -712,7 +722,9 @@ public class StandardProcessorTestRunner implements TestRunner {
 
     @Override
     public void removeControllerService(final ControllerService service) {
-        disableControllerService(service);
+        if (context.getControllerServiceLookup().isControllerServiceEnabled(service)) {
+            disableControllerService(service);
+        }
 
         try {
             ReflectionUtils.invokeMethodsWithAnnotation(OnRemoved.class, service);

nifi-mock/src/test/java/org/apache/nifi/util/TestStandardProcessorTestRunner.java

@@ -207,6 +207,35 @@ public class TestStandardProcessorTestRunner {
         assertTrue("onPropertyModified has not been called", ((SimpleTestService) testService).isOpmCalled());
     }
 
+    @Test
+    public void testProcessorInvalidWhenControllerServiceDisabled() {
+        final ControllerService testService = new RequiredPropertyTestService();
+        final AddAttributeProcessor proc = new AddAttributeProcessor();
+        final TestRunner runner = TestRunners.newTestRunner(proc);
+        final String serviceIdentifier = "test";
+        final String pdName = "name";
+        final String pdValue = "exampleName";
+        try {
+            runner.addControllerService(serviceIdentifier, testService);
+        } catch (InitializationException e) {
+            fail(e.getMessage());
+        }
+
+        // controller service invalid due to no value on required property; processor must also be invalid
+        runner.assertNotValid(testService);
+        runner.assertNotValid();
+
+        // add required property; controller service valid but not enabled; processor must be invalid
+        runner.setProperty(testService, RequiredPropertyTestService.namePropertyDescriptor, pdValue);
+        runner.assertValid(testService);
+        runner.assertNotValid();
+
+        // enable controller service; processor now valid
+        runner.enableControllerService(testService);
+        runner.assertValid(testService);
+        runner.assertValid();
+    }
+
     private static class ProcessorWithOnStop extends AbstractProcessor {
 
         private int callsWithContext = 0;
@@ -330,4 +359,19 @@ public class TestStandardProcessorTestRunner {
             return opmCalled;
         }
     }
+
+    private static class RequiredPropertyTestService extends AbstractControllerService {
+        private static final String PD_NAME = "name";
+        protected static final  PropertyDescriptor namePropertyDescriptor = new PropertyDescriptor.Builder()
+                .name(PD_NAME)
+                .displayName("Controller Service Name")
+                .required(true)
+                .sensitive(false)
+                .allowableValues("exampleName", "anotherExampleName")
+                .build();
+
+        protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+            return Arrays.asList(namePropertyDescriptor);
+        }
+    }
 }

nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/credentials/provider/service/AWSCredentialsProviderControllerServiceTest.java

@@ -144,7 +144,7 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.assertValid(serviceImpl);
     }
 
-    @Test(expected = AssertionError.class)
+    @Test
     public void testKeysCredentialsProviderWithRoleAndNameAndSessionTimeoutLessThan900() throws Throwable {
         final TestRunner runner = TestRunners.newTestRunner(FetchS3Object.class);
         final AWSCredentialsProviderControllerService serviceImpl = new AWSCredentialsProviderControllerService();
@@ -154,11 +154,10 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.ASSUME_ROLE_ARN, "Role");
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.ASSUME_ROLE_NAME, "RoleName");
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.MAX_SESSION_TIME, "899");
-        runner.enableControllerService(serviceImpl);
         runner.assertNotValid(serviceImpl);
     }
 
-    @Test(expected = AssertionError.class)
+    @Test
     public void testKeysCredentialsProviderWithRoleAndNameAndSessionTimeoutGreaterThan3600() throws Throwable {
         final TestRunner runner = TestRunners.newTestRunner(FetchS3Object.class);
         final AWSCredentialsProviderControllerService serviceImpl = new AWSCredentialsProviderControllerService();
@@ -168,7 +167,7 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.ASSUME_ROLE_ARN, "Role");
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.ASSUME_ROLE_NAME, "RoleName");
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.MAX_SESSION_TIME, "899");
-        runner.enableControllerService(serviceImpl);
+        runner.assertNotValid(serviceImpl);
     }
 
     @Test
@@ -179,7 +178,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.setProperty(serviceImpl, AbstractAWSProcessor.ACCESS_KEY, "awsAccessKey");
         runner.setProperty(serviceImpl, AbstractAWSProcessor.SECRET_KEY, "awsSecretKey");
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.ASSUME_ROLE_ARN, "Role");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -192,7 +190,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.setProperty(serviceImpl, AbstractAWSProcessor.ACCESS_KEY, "awsAccessKey");
         runner.setProperty(serviceImpl, AbstractAWSProcessor.SECRET_KEY, "awsSecretKey");
         runner.setProperty(serviceImpl, AWSCredentialsProviderControllerService.ASSUME_ROLE_NAME, "RoleName");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -244,7 +241,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.addControllerService("awsCredentialsProvider", serviceImpl);
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.CREDENTIALS_FILE,
                 "src/test/resources/bad-mock-aws-credentials.properties");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -258,7 +254,6 @@ public class AWSCredentialsProviderControllerServiceTest {
                 "src/test/resources/mock-aws-credentials.properties");
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.ACCESS_KEY, "awsAccessKey");
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.SECRET_KEY, "awsSecretKey");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -271,7 +266,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.CREDENTIALS_FILE,
                 "src/test/resources/mock-aws-credentials.properties");
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.ACCESS_KEY, "awsAccessKey");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -284,7 +278,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.CREDENTIALS_FILE,
                 "src/test/resources/mock-aws-credentials.properties");
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.SECRET_KEY, "awsSecretKey");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -295,7 +288,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         final AWSCredentialsProviderControllerService serviceImpl = new AWSCredentialsProviderControllerService();
         runner.addControllerService("awsCredentialsProvider", serviceImpl);
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.ACCESS_KEY, "awsAccessKey");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }
@@ -306,7 +298,6 @@ public class AWSCredentialsProviderControllerServiceTest {
         final AWSCredentialsProviderControllerService serviceImpl = new AWSCredentialsProviderControllerService();
         runner.addControllerService("awsCredentialsProvider", serviceImpl);
         runner.setProperty(serviceImpl, CredentialPropertyDescriptors.SECRET_KEY, "awsSecretKey");
-        runner.enableControllerService(serviceImpl);
 
         runner.assertNotValid(serviceImpl);
     }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/ServiceStateTransition.java

@@ -17,13 +17,12 @@
 
 package org.apache.nifi.controller.service;
 
+import org.apache.nifi.controller.ComponentNode;
+
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 
-import org.apache.nifi.controller.ComponentNode;
-
 public class ServiceStateTransition {
     private ControllerServiceState state = ControllerServiceState.DISABLED;
     private final List<CompletableFuture<?>> enabledFutures = new ArrayList<>();

nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/dbcp/TestDBCPConnectionPoolLookup.java

@@ -125,15 +125,12 @@ public class TestDBCPConnectionPoolLookup {
         // enable lookup service with no services registered, verify not valid
         runner = TestRunners.newTestRunner(TestProcessor.class);
         runner.addControllerService("dbcp-lookup", dbcpLookupService);
-        runner.enableControllerService(dbcpLookupService);
         runner.assertNotValid(dbcpLookupService);
 
         final String dbcpServiceAIdentifier = "dbcp-a";
         runner.addControllerService(dbcpServiceAIdentifier, dbcpServiceA);
-        runner.enableControllerService(dbcpServiceA);
 
         // register a service and now verify valid
-        runner.disableControllerService(dbcpLookupService);
         runner.setProperty(dbcpLookupService, "a", dbcpServiceAIdentifier);
         runner.enableControllerService(dbcpLookupService);
         runner.assertValid(dbcpLookupService);
@@ -144,7 +141,6 @@ public class TestDBCPConnectionPoolLookup {
         runner = TestRunners.newTestRunner(TestProcessor.class);
         runner.addControllerService("dbcp-lookup", dbcpLookupService);
         runner.setProperty(dbcpLookupService, "dbcp-lookup", "dbcp-lookup");
-        runner.enableControllerService(dbcpLookupService);
         runner.assertNotValid(dbcpLookupService);
     }
 

nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/test/java/org/apache/nifi/hbase/TestHBase_1_1_2_ClientService.java

@@ -83,7 +83,6 @@ public class TestHBase_1_1_2_ClientService {
         // no conf file or zk properties so should be invalid
         MockHBaseClientService service = new MockHBaseClientService(table, COL_FAM, kerberosPropsWithFile);
         runner.addControllerService("hbaseClientService", service);
-        runner.enableControllerService(service);
 
         runner.assertNotValid(service);
         runner.removeControllerService(service);
@@ -106,7 +105,6 @@ public class TestHBase_1_1_2_ClientService {
         service = new MockHBaseClientService(table, COL_FAM, kerberosPropsWithFile);
         runner.addControllerService("hbaseClientService", service);
         runner.setProperty(service, HBase_1_1_2_ClientService.ZOOKEEPER_QUORUM, "${zk-quorum}");
-        runner.enableControllerService(service);
 
         runner.assertNotValid(service);
         runner.removeControllerService(service);
@@ -116,7 +114,6 @@ public class TestHBase_1_1_2_ClientService {
         runner.addControllerService("hbaseClientService", service);
         runner.setProperty(service, HBase_1_1_2_ClientService.ZOOKEEPER_QUORUM, "${zk-quorum}");
         runner.setProperty(service, HBase_1_1_2_ClientService.ZOOKEEPER_CLIENT_PORT, "${zk-client-port}");
-        runner.enableControllerService(service);
 
         runner.assertNotValid(service);
         runner.removeControllerService(service);
@@ -155,11 +152,9 @@ public class TestHBase_1_1_2_ClientService {
         runner.disableControllerService(service);
         runner.setProperty(service, HBase_1_1_2_ClientService.HADOOP_CONF_FILES,
                 "src/test/resources/hbase-site-security.xml, src/test/resources/core-site-security.xml");
-        runner.enableControllerService(service);
         runner.assertNotValid(service);
 
         // Kerberos - add valid options
-        runner.disableControllerService(service);
         runner.setProperty(service, kerberosPropsWithFile.getKerberosKeytab(), "src/test/resources/fake.keytab");
         runner.setProperty(service, kerberosPropsWithFile.getKerberosPrincipal(), "test@REALM");
         runner.enableControllerService(service);
@@ -168,14 +163,11 @@ public class TestHBase_1_1_2_ClientService {
         // Kerberos - add invalid non-existent keytab file
         runner.disableControllerService(service);
         runner.setProperty(service, kerberosPropsWithFile.getKerberosKeytab(), "src/test/resources/missing.keytab");
-        runner.enableControllerService(service);
         runner.assertNotValid(service);
 
         // Kerberos - add invalid principal
-        runner.disableControllerService(service);
         runner.setProperty(service, kerberosPropsWithFile.getKerberosKeytab(), "src/test/resources/fake.keytab");
         runner.setProperty(service, kerberosPropsWithFile.getKerberosPrincipal(), "");
-        runner.enableControllerService(service);
         runner.assertNotValid(service);
 
         // Kerberos - valid props but the KerberosProperties has a null Kerberos config file so be invalid
@@ -185,7 +177,6 @@ public class TestHBase_1_1_2_ClientService {
                 "src/test/resources/hbase-site-security.xml, src/test/resources/core-site-security.xml");
         runner.setProperty(service, kerberosPropsWithoutFile.getKerberosKeytab(), "src/test/resources/fake.keytab");
         runner.setProperty(service, kerberosPropsWithoutFile.getKerberosPrincipal(), "test@REALM");
-        runner.enableControllerService(service);
         runner.assertNotValid(service);
     }
 

nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/test/java/org/apache/nifi/xml/TestXMLRecordSetWriter.java

@@ -174,6 +174,10 @@ public class TestXMLRecordSetWriter {
         runner.setProperty(writer, XMLRecordSetWriter.RECORD_TAG_NAME, "record");
 
         runner.setProperty(writer, XMLRecordSetWriter.ARRAY_WRAPPING, XMLRecordSetWriter.USE_PROPERTY_AS_WRAPPER);
+        runner.assertNotValid(writer);
+
+        runner.setProperty(writer, XMLRecordSetWriter.ARRAY_TAG_NAME, "array-tag-name");
+        runner.assertValid(writer);
 
         runner.enableControllerService(writer);
         runner.enqueue("");

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

@@ -117,7 +117,7 @@ class StandardSSLContextServiceTest {
         }
 
         // Assert
-        assert msg =~ "invalid because Cannot access file"
+        assert msg =~ "Cannot enable Controller Service SSLContextService.* because it is in an invalid state: 'Truststore Filename'.* is invalid because File.* does not exist or cannot be read";
         runner.assertNotValid(sslContextService)
     }
 

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

@@ -224,7 +224,7 @@ public class SSLContextServiceTest {
         // Assert
 
         // Have to exhaust the cached result by checking n-1 more times
-        for (int i = 2; i <= sslContextService.getValidationCacheExpiration(); i++) {
+        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");