IGNITE-28091 Validate configuration in ConfigurationExtension

modules/client-handler/src/integrationTest/java/org/apache/ignite/client/handler/ItClientHandlerTest.java

@@ -19,6 +19,7 @@
 
 import static org.apache.ignite.client.handler.ItClientHandlerTestUtils.MAGIC;
 import static org.apache.ignite.configuration.annotation.ConfigurationType.DISTRIBUTED;
+import static org.apache.ignite.internal.security.authentication.SecurityConfigurationModule.DEFAULT_PROVIDER_NAME;
 import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
 import static org.apache.ignite.lang.ErrorGroups.Authentication.INVALID_CREDENTIALS_ERR;
 import static org.apache.ignite.lang.ErrorGroups.Authentication.UNSUPPORTED_AUTHENTICATION_TYPE_ERR;
@@ -592,7 +593,7 @@ private static void writeAndFlushLoop(Socket socket) throws Exception {
     private void setupAuthentication(String username, String password) {
         securityConfiguration.change(change -> {
             change.changeEnabled(true);
-            change.changeAuthentication().changeProviders().create("basic", authenticationProviderChange -> {
+            change.changeAuthentication().changeProviders().update(DEFAULT_PROVIDER_NAME, authenticationProviderChange -> {
                 authenticationProviderChange.convert(BasicAuthenticationProviderChange.class)
                         .changeUsers(users -> users.create(username, user -> user.changePassword(password)));
             });

modules/client/src/test/java/org/apache/ignite/client/ClientAuthenticationTest.java

@@ -18,14 +18,17 @@
 package org.apache.ignite.client;
 
 import static org.apache.ignite.configuration.annotation.ConfigurationType.DISTRIBUTED;
+import static org.apache.ignite.internal.security.authentication.SecurityConfigurationModule.DEFAULT_PASSWORD;
+import static org.apache.ignite.internal.security.authentication.SecurityConfigurationModule.DEFAULT_USERNAME;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
 import static org.apache.ignite.internal.util.IgniteUtils.closeAll;
+import static org.hamcrest.MatcherAssert.assertThat;
 
 import java.util.UUID;
 import org.apache.ignite.client.fakes.FakeIgnite;
 import org.apache.ignite.internal.configuration.ClusterConfiguration;
 import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
 import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
-import org.apache.ignite.internal.security.authentication.basic.BasicAuthenticationProviderChange;
 import org.apache.ignite.internal.security.configuration.SecurityConfiguration;
 import org.apache.ignite.internal.security.configuration.SecurityExtensionConfiguration;
 import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
@@ -85,9 +88,9 @@ public void testAuthnOnServerBadAuthnOnClient() {
 
     @Test
     public void testAuthnOnClientAuthnOnServer() {
-        server = startServer(false);
+        server = startServer(true);
 
-        client = startClient(BasicAuthenticator.builder().username("usr").password("pwd").build());
+        client = startClient(BasicAuthenticator.builder().username(DEFAULT_USERNAME).password(DEFAULT_PASSWORD).build());
     }
 
     private IgniteClient startClient(@Nullable IgniteClientAuthenticator authenticator) {
@@ -111,15 +114,7 @@ private TestServer startServer(boolean basicAuthn) {
                 null);
 
         if (basicAuthn) {
-            securityConfiguration.change(securityChange -> {
-                securityChange.changeEnabled(true);
-                securityChange.changeAuthentication().changeProviders().create("basic", change ->
-                        change.convert(BasicAuthenticationProviderChange.class)
-                                .changeUsers(users -> users.create("usr", user ->
-                                        user.changePassword("pwd"))
-                                )
-                );
-            }).join();
+            assertThat(securityConfiguration.change(securityChange -> securityChange.changeEnabled(true)), willCompleteSuccessfully());
         }
 
         return server;

modules/configuration/build.gradle

@@ -34,9 +34,17 @@ dependencies {
     testAnnotationProcessor project(':ignite-configuration-annotation-processor')
     testImplementation project(':ignite-core')
     testImplementation testFixtures(project(':ignite-core'))
+    testImplementation libs.junit.testkit
 
     testFixturesAnnotationProcessor project(':ignite-configuration-annotation-processor')
     testFixturesImplementation project(':ignite-core')
     testFixturesImplementation testFixtures(project(':ignite-core'))
     testFixturesImplementation libs.typesafe.config
 }
+
+// Exclude tests from inner static classes that use JUnit Platform Test Kit.
+test {
+    filter {
+        excludeTestsMatching "*ConfigurationExtensionTest\$*"
+    }
+}

modules/configuration/src/test/java/org/apache/ignite/internal/configuration/testframework/ConfigurationExtensionTest.java

@@ -17,21 +17,30 @@
 
 package org.apache.ignite.internal.configuration.testframework;
 
-import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.ignite.internal.testframework.JunitExtensionTestUtils.assertExecutesWithFailure;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrowFast;
 import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
 import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.configuration.validation.ConfigurationValidationException;
 import org.apache.ignite.internal.configuration.sample.DiscoveryConfiguration;
 import org.apache.ignite.internal.configuration.sample.ExtendedDiscoveryConfiguration;
 import org.apache.ignite.internal.configuration.sample.ExtendedDiscoveryConfigurationSchema;
 import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.apache.ignite.internal.testframework.JunitExtensionTestUtils;
+import org.apache.ignite.internal.util.ExceptionUtils;
+import org.assertj.core.api.Condition;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -56,25 +65,23 @@ static void staticParameterInjection(
 
     /** Test that contains injected parameter. */
     @Test
-    public void injectConfiguration(
-            @InjectConfiguration("mock.joinTimeout=100") DiscoveryConfiguration paramCfg
-    ) throws Exception {
+    void injectConfiguration(@InjectConfiguration("mock.joinTimeout=100") DiscoveryConfiguration paramCfg) {
         assertEquals(5000, fieldCfg.joinTimeout().value());
 
         assertEquals(100, paramCfg.joinTimeout().value());
 
-        paramCfg.change(d -> d.changeJoinTimeout(200)).get(1, SECONDS);
+        assertThat(paramCfg.change(d -> d.changeJoinTimeout(200)), willCompleteSuccessfully());
 
         assertEquals(200, paramCfg.joinTimeout().value());
 
-        paramCfg.joinTimeout().update(300).get(1, SECONDS);
+        assertThat(paramCfg.joinTimeout().update(300), willCompleteSuccessfully());
 
         assertEquals(300, paramCfg.joinTimeout().value());
     }
 
     /** Tests that notifications work on injected configuration instance. */
     @Test
-    public void notifications() throws Exception {
+    void notifications() {
         List<String> log = new ArrayList<>();
 
         fieldCfg.listen(ctx -> {
@@ -95,35 +102,33 @@ public void notifications() throws Exception {
             return nullCompletedFuture();
         });
 
-        fieldCfg.change(change -> change.changeJoinTimeout(1000_000)).get(1, SECONDS);
+        assertThat(fieldCfg.change(change -> change.changeJoinTimeout(1000_000)), willCompleteSuccessfully());
 
         assertEquals(List.of("update", "join"), log);
 
         log.clear();
 
-        fieldCfg.failureDetectionTimeout().update(2000_000).get(1, SECONDS);
+        assertThat(fieldCfg.failureDetectionTimeout().update(2000_000), willCompleteSuccessfully());
 
         assertEquals(List.of("update", "failure"), log);
     }
 
     /** Tests that internal configuration extensions work properly on injected configuration instance. */
     @Test
-    public void internalConfiguration(
-            @InjectConfiguration(extensions = {ExtendedConfigurationSchema.class}) BasicConfiguration cfg
-    ) throws Exception {
+    void internalConfiguration(@InjectConfiguration(extensions = ExtendedConfigurationSchema.class) BasicConfiguration cfg) {
         assertThat(cfg, is(instanceOf(ExtendedConfiguration.class)));
 
         assertEquals(1, cfg.visible().value());
 
         assertEquals(2, ((ExtendedConfiguration) cfg).invisible().value());
 
-        cfg.change(change -> {
+        assertThat(cfg.change(change -> {
             assertThat(change, is(instanceOf(ExtendedChange.class)));
 
             change.changeVisible(3);
 
             ((ExtendedChange) change).changeInvisible(4);
-        }).get(1, SECONDS);
+        }), willCompleteSuccessfully());
 
         assertEquals(3, cfg.visible().value());
 
@@ -132,12 +137,133 @@ public void internalConfiguration(
 
     /** Test UUID generation in mocks. */
     @Test
-    public void testInjectInternalId(
+    void testInjectInternalId(
             @InjectConfiguration(
                     extensions = ExtendedDiscoveryConfigurationSchema.class,
                     name = "test"
             ) DiscoveryConfiguration discoveryConfig
     ) {
         assertNotNull(((ExtendedDiscoveryConfiguration) discoveryConfig).id().value());
     }
+
+    /** Tests that changing a value to one within the valid range succeeds. */
+    @Test
+    void rangeValidationAcceptsValidChange(@InjectConfiguration ValidatedConfiguration cfg) {
+        assertThat(cfg.change(c -> c.changeRangeValue(80)), willCompleteSuccessfully());
+
+        assertEquals(80, cfg.rangeValue().value());
+    }
+
+    /** Tests that changing a value to one outside the valid range throws {@link ConfigurationValidationException}. */
+    @Test
+    void rangeValidationRejectsInvalidChange(@InjectConfiguration ValidatedConfiguration cfg) {
+        assertThat(cfg.change(c -> c.changeRangeValue(0)), willThrowFast(ConfigurationValidationException.class));
+    }
+
+    /** Tests that changing an {@code @Immutable} field throws {@link ConfigurationValidationException}. */
+    @Test
+    void immutableValidationRejectsChange(@InjectConfiguration ValidatedConfiguration cfg) {
+        assertThat(cfg.change(c -> c.changeConstValue("changed")), willThrowFast(ConfigurationValidationException.class));
+    }
+
+    /** Tests that a failed validation does not mutate the configuration value. */
+    @Test
+    void failedValidationLeavesValueUnchanged(@InjectConfiguration ValidatedConfiguration cfg) {
+        assertThat(cfg.change(c -> c.changeRangeValue(0)), willThrowFast(ConfigurationValidationException.class));
+
+        assertEquals(50, cfg.rangeValue().value());
+    }
+
+    /** Tests that all violated constraints are collected and reported together. */
+    @Test
+    void multipleValidationIssuesAreReported(@InjectConfiguration ValidatedConfiguration cfg) {
+        CompletableFuture<Void> future = cfg.change(c -> c.changeRangeValue(0).changeConstValue("changed"));
+
+        assertThat(future, willThrowFast(ConfigurationValidationException.class));
+
+        ExecutionException ex = assertThrows(ExecutionException.class, future::get);
+
+        assertThat(((ConfigurationValidationException) ex.getCause()).getIssues(), hasSize(2));
+    }
+
+    /**
+     * Tests that {@code @Value(hasDefault = true)} schema defaults are applied when no explicit HOCON values
+     * are provided.
+     */
+    @Test
+    void defaultValuesAreApplied(@InjectConfiguration ValidatedConfiguration cfg) {
+        assertEquals(50, cfg.rangeValue().value());
+        assertEquals("constant", cfg.constValue().value());
+    }
+
+    /**
+     * Tests that initial configuration values are validated at injection time, and that a value
+     * violating a constraint causes {@link ConfigurationValidationException} to be thrown.
+     */
+    @Test
+    void initialValidationRejectsInvalidValue() {
+        assertInjectionFails(InvalidInitialValueTest.class);
+    }
+
+    @Test
+    void missingDefaultValueIsRejectedByValidation() {
+        assertInjectionFails(MissingDefaultValueTest.class);
+    }
+
+    @Test
+    void invalidDefaultValueIsRejectedByValidation() {
+        assertInjectionFails(InvalidDefaultValueTest.class);
+    }
+
+    private static void assertInjectionFails(Class<?> testClass) {
+        assertExecutesWithFailure(
+                testClass,
+                new Condition<>(
+                        t -> ExceptionUtils.hasCause(t, ConfigurationValidationException.class),
+                        "ConfigurationValidationException"
+                )
+        );
+    }
+
+    /**
+     * A test class whose {@code @Value(hasDefault = true)} is not initialized.
+     * It is not a normal test and is only executed programmatically via {@link JunitExtensionTestUtils}.
+     */
+    @ExtendWith(ConfigurationExtension.class)
+    static class MissingDefaultValueTest extends BaseIgniteAbstractTest {
+        @SuppressWarnings("unused")
+        @InjectConfiguration
+        InvalidDefaultConfiguration cfg;
+
+        @Test
+        void test() {}
+    }
+
+    /**
+     * A test class whose field injection intentionally violates {@code @Range(min=1)}.
+     * It is not a normal test and is only executed programmatically via {@link JunitExtensionTestUtils}.
+     */
+    @ExtendWith(ConfigurationExtension.class)
+    static class InvalidInitialValueTest extends BaseIgniteAbstractTest {
+        @SuppressWarnings("unused")
+        @InjectConfiguration("mock.rangeValue=0")
+        ValidatedConfiguration cfg;
+
+        @Test
+        void test() {}
+    }
+
+    /**
+     * A test class whose {@code @Value(hasDefault = true)} is inialized with an invalid value.
+     * It is not a normal test and is only executed programmatically via {@link JunitExtensionTestUtils}.
+     */
+    @ExtendWith(ConfigurationExtension.class)
+    static class InvalidDefaultValueTest extends BaseIgniteAbstractTest {
+        @SuppressWarnings("unused")
+        @InjectConfiguration
+        InvalidDefaultConfiguration cfg;
+
+        @Test
+        void test() {}
+    }
 }

modules/configuration/src/test/java/org/apache/ignite/internal/configuration/testframework/InvalidDefaultConfigurationSchema.java

@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.configuration.testframework;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.Range;
+
+/**
+ * Configuration schema whose {@code @Value(hasDefault = true)} field is initialized with an invalid value.
+ */
+@Config
+public class InvalidDefaultConfigurationSchema {
+    @Value(hasDefault = true)
+    @Range(min = 1, max = 100)
+    public int invalidDefaultValue = -1;
+}

modules/configuration/src/test/java/org/apache/ignite/internal/configuration/testframework/MissingDefaultConfigurationSchema.java

@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.configuration.testframework;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+
+/**
+ * Configuration schema whose {@code @Value(hasDefault = true)} field is not initialized.
+ * Used by {@link ConfigurationExtensionTest} to verify that schema defaults are subject to validation.
+ */
+@Config
+public class MissingDefaultConfigurationSchema {
+    @Value(hasDefault = true)
+    public String defaultValue;
+}