From 806d8d61658c03537a38132eb5764900cb7b4e8c Mon Sep 17 00:00:00 2001 From: David Handermann Date: Fri, 19 Jan 2024 07:30:45 -0600 Subject: [PATCH] NIFI-12634 Ignored Blank Prefix Values in Kubernetes Components (#8268) - Updated KubernetesConfigMapStateProvider and KubernetesLeaderElectionManager to ignore blank prefix values as provided in default configuration files --- .../KubernetesLeaderElectionManager.java | 3 +- .../KubernetesLeaderElectionManagerTest.java | 35 ++++++++++++++----- .../pom.xml | 4 +++ .../KubernetesConfigMapStateProvider.java | 7 ++-- .../KubernetesConfigMapStateProviderTest.java | 26 ++++++++++---- 5 files changed, 56 insertions(+), 19 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/main/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManager.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/main/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManager.java index ecefed0323..0272149470 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/main/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManager.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/main/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManager.java @@ -80,7 +80,8 @@ public class KubernetesLeaderElectionManager extends TrackedLeaderElectionManage * Kubernetes Leader Election Manager constructor with NiFi Properties */ public KubernetesLeaderElectionManager(final NiFiProperties nifiProperties) { - this.roleIdPrefix = nifiProperties.getProperty(NiFiProperties.CLUSTER_LEADER_ELECTION_KUBERNETES_LEASE_PREFIX); + final String leasePrefix = nifiProperties.getProperty(NiFiProperties.CLUSTER_LEADER_ELECTION_KUBERNETES_LEASE_PREFIX); + this.roleIdPrefix = leasePrefix == null || leasePrefix.isBlank() ? null : leasePrefix; executorService = createExecutorService(); leaderElectionCommandProvider = createLeaderElectionCommandProvider(); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/test/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManagerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/test/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManagerTest.java index 3457332127..05c6f636c5 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/test/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManagerTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-leader-election/src/test/java/org/apache/nifi/kubernetes/leader/election/KubernetesLeaderElectionManagerTest.java @@ -53,6 +53,8 @@ class KubernetesLeaderElectionManagerTest { private static final String PREFIX = "label"; + private static final String EMPTY_PREFIX = ""; + @Mock LeaderElectionStateChangeListener changeListener; @@ -68,16 +70,11 @@ class KubernetesLeaderElectionManagerTest { ManagedLeaderElectionCommandProvider leaderElectionCommandProvider; KubernetesLeaderElectionManager manager; - KubernetesLeaderElectionManager managerWithProperties; @BeforeEach void setManager() { leaderElectionCommandProvider = new ManagedLeaderElectionCommandProvider(); manager = new MockKubernetesLeaderElectionManager(new NiFiProperties()); - - final Properties properties = new Properties(); - properties.setProperty(NiFiProperties.CLUSTER_LEADER_ELECTION_KUBERNETES_LEASE_PREFIX, PREFIX); - managerWithProperties = new MockKubernetesLeaderElectionManager(new NiFiProperties(properties)); } @Test @@ -195,15 +192,37 @@ class KubernetesLeaderElectionManagerTest { @Test void testRoleIdWithPrefix() { - managerWithProperties.start(); + final Properties properties = new Properties(); + properties.setProperty(NiFiProperties.CLUSTER_LEADER_ELECTION_KUBERNETES_LEASE_PREFIX, PREFIX); + final MockKubernetesLeaderElectionManager electionManager = new MockKubernetesLeaderElectionManager(new NiFiProperties(properties)); + + electionManager.start(); setSubmitStartLeading(); - managerWithProperties.register(ROLE, changeListener, PARTICIPANT_ID); + electionManager.register(ROLE, changeListener, PARTICIPANT_ID); captureRunCommand(); - assertEquals(PREFIX + "-" + LEADER_ELECTION_ROLE.getRoleId(), leaderElectionCommandProvider.name); + final String expected = String.format("%s-%s", PREFIX, LEADER_ELECTION_ROLE.getRoleId()); + assertEquals(expected, leaderElectionCommandProvider.name); + } + + @Test + void testRoleIdWithEmptyPrefix() { + final Properties properties = new Properties(); + properties.setProperty(NiFiProperties.CLUSTER_LEADER_ELECTION_KUBERNETES_LEASE_PREFIX, EMPTY_PREFIX); + final MockKubernetesLeaderElectionManager electionManager = new MockKubernetesLeaderElectionManager(new NiFiProperties(properties)); + + electionManager.start(); + + setSubmitStartLeading(); + + electionManager.register(ROLE, changeListener, PARTICIPANT_ID); + + captureRunCommand(); + + assertEquals(LEADER_ELECTION_ROLE.getRoleId(), leaderElectionCommandProvider.name); } private void setSubmitStartLeading() { diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/pom.xml index 1ee073e5c4..a93989ddf0 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/pom.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/pom.xml @@ -48,6 +48,10 @@ kubernetes-server-mock test + + org.apache.nifi + nifi-mock + org.apache.nifi nifi-expression-language diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/main/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/main/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProvider.java index 6c8d742e00..a4d90d5421 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/main/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProvider.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/main/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProvider.java @@ -41,6 +41,7 @@ import java.util.regex.Pattern; import org.apache.nifi.components.AbstractConfigurableComponent; import org.apache.nifi.components.PropertyDescriptor; import org.apache.nifi.components.PropertyValue; +import org.apache.nifi.components.Validator; import org.apache.nifi.components.state.Scope; import org.apache.nifi.components.state.StateMap; import org.apache.nifi.components.state.StateProvider; @@ -48,7 +49,6 @@ import org.apache.nifi.components.state.StateProviderInitializationContext; import org.apache.nifi.kubernetes.client.ServiceAccountNamespaceProvider; import org.apache.nifi.kubernetes.client.StandardKubernetesClientProvider; import org.apache.nifi.logging.ComponentLog; -import org.apache.nifi.processor.util.StandardValidators; /** * State Provider implementation based on Kubernetes ConfigMaps with Base64 encoded keys to meet Kubernetes constraints @@ -57,7 +57,7 @@ public class KubernetesConfigMapStateProvider extends AbstractConfigurableCompon static final PropertyDescriptor CONFIG_MAP_NAME_PREFIX = new PropertyDescriptor.Builder() .name("ConfigMap Name Prefix") .description("Optional prefix that the Provider will prepend to Kubernetes ConfigMap names. The resulting ConfigMap name will contain nifi-component and the component identifier.") - .addValidator(StandardValidators.NON_BLANK_VALIDATOR) + .addValidator(Validator.VALID) .required(false) .build(); @@ -126,8 +126,9 @@ public class KubernetesConfigMapStateProvider extends AbstractConfigurableCompon this.namespace = new ServiceAccountNamespaceProvider().getNamespace(); final PropertyValue configMapNamePrefixProperty = context.getProperty(CONFIG_MAP_NAME_PREFIX); - final String configMapNamePrefix = configMapNamePrefixProperty.isSet() ? configMapNamePrefixProperty.getValue() + PREFIX_SEPARATOR : EMPTY_PREFIX; + final String prefixPropertyValue = configMapNamePrefixProperty.getValue(); + final String configMapNamePrefix = prefixPropertyValue == null || prefixPropertyValue.isBlank() ? EMPTY_PREFIX : prefixPropertyValue + PREFIX_SEPARATOR; configMapNameFormat = String.format(CONFIG_MAP_NAME_FORMAT, configMapNamePrefix); configMapNamePattern = Pattern.compile(String.format(CONFIG_MAP_NAME_PATTERN_FORMAT, configMapNamePrefix)); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/test/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProviderTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/test/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProviderTest.java index 54b40e5833..d833113b97 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/test/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProviderTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/test/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProviderTest.java @@ -24,11 +24,14 @@ import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer; import io.fabric8.kubernetes.client.server.mock.KubernetesMockServerExtension; import io.fabric8.mockwebserver.dsl.HttpMethod; import okhttp3.mockwebserver.RecordedRequest; +import org.apache.nifi.components.ValidationResult; import org.apache.nifi.components.state.Scope; import org.apache.nifi.components.state.StateMap; import org.apache.nifi.components.state.StateProviderInitializationContext; import org.apache.nifi.logging.ComponentLog; import org.apache.nifi.parameter.ParameterLookup; +import org.apache.nifi.util.MockProcessContext; +import org.apache.nifi.util.MockValidationContext; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -72,6 +75,8 @@ class KubernetesConfigMapStateProviderTest { private static final String CONFIG_MAP_NAME_PREFIX_VALUE = "label"; + private static final String EMPTY = ""; + @Mock StateProviderInitializationContext context; @@ -97,12 +102,19 @@ class KubernetesConfigMapStateProviderTest { } @Test - void testInitializeShutdown() { - setContext(); - provider.initialize(context); + void testInitializeValidateShutdown() { + setContextWithConfigMapNamePrefix(EMPTY); + provider.initialize(context); assertEquals(IDENTIFIER, provider.getIdentifier()); + final MockProcessContext processContext = new MockProcessContext(provider); + processContext.setProperty(KubernetesConfigMapStateProvider.CONFIG_MAP_NAME_PREFIX, EMPTY); + final MockValidationContext validationContext = new MockValidationContext(processContext, null); + final Collection results = provider.validate(validationContext); + + assertTrue(results.isEmpty()); + provider.shutdown(); } @@ -299,7 +311,7 @@ class KubernetesConfigMapStateProviderTest { @Test void testSetStateGetStateWithPrefix() throws IOException { - setContextWithProperties(); + setContextWithConfigMapNamePrefix(CONFIG_MAP_NAME_PREFIX_VALUE); provider.initialize(context); final Map state = Collections.singletonMap(STATE_PROPERTY, STATE_VALUE); @@ -317,7 +329,7 @@ class KubernetesConfigMapStateProviderTest { @Test void testSetStateGetStoredComponentIdsWithPrefix() throws IOException { - setContextWithProperties(); + setContextWithConfigMapNamePrefix(CONFIG_MAP_NAME_PREFIX_VALUE); provider.initialize(context); final Collection initialStoredComponentIds = provider.getStoredComponentIds(); @@ -340,10 +352,10 @@ class KubernetesConfigMapStateProviderTest { .thenReturn(new StandardPropertyValue(null, null, ParameterLookup.EMPTY)); } - private void setContextWithProperties() { + private void setContextWithConfigMapNamePrefix(final String configMapNamePrefix) { setContext(); when(context.getProperty(KubernetesConfigMapStateProvider.CONFIG_MAP_NAME_PREFIX)) - .thenReturn(new StandardPropertyValue(CONFIG_MAP_NAME_PREFIX_VALUE, null, ParameterLookup.EMPTY)); + .thenReturn(new StandardPropertyValue(configMapNamePrefix, null, ParameterLookup.EMPTY)); } private void assertStateEquals(final Map expected, final StateMap stateMap) {