diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/config/KubernetesConfiguration.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/config/KubernetesConfiguration.java index 21059a6a34fb..0022913842c8 100644 --- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/config/KubernetesConfiguration.java +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/config/KubernetesConfiguration.java @@ -21,25 +21,40 @@ import io.kubernetes.client.openapi.apis.AppsV1Api; import io.kubernetes.client.util.Config; import org.apache.shenyu.admin.config.properties.DeploymentProperties; -import org.apache.shenyu.admin.scale.scaler.KubernetesScaler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import java.io.FileInputStream; +import java.io.ByteArrayInputStream; import java.io.IOException; -import java.util.Optional; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; @Configuration -@EnableConfigurationProperties(DeploymentProperties.class) public class KubernetesConfiguration { private final Logger logger = LoggerFactory.getLogger(KubernetesConfiguration.class); + /** + * kubernetes deployment properties. Registered as a conditional bean rather + * than through {@code @EnableConfigurationProperties}, so a user-supplied + * {@link DeploymentProperties} bean cleanly wins instead of competing with the + * one registered here for the {@code apiClient} argument (#7912). The prefix + * comes from {@link DeploymentProperties} itself: repeating it on this method + * is a duplicate definition the configuration processor rejects at compile time. + * + * @return the deployment properties bound from {@code shenyu.k8s.deployment} + */ + @Bean + @ConditionalOnMissingBean + public DeploymentProperties deploymentProperties() { + return new DeploymentProperties(); + } + /** * kubernetes apiClient. * @param deploymentProperties deploymentProperties @@ -47,35 +62,28 @@ public class KubernetesConfiguration { */ @Bean @ConditionalOnProperty(value = "shenyu.k8s.scale.enabled", havingValue = "true") - @ConditionalOnMissingBean(DeploymentProperties.class) + @ConditionalOnMissingBean(AppsV1Api.class) public AppsV1Api apiClient(final DeploymentProperties deploymentProperties) { try { ApiClient client = Config.fromToken( deploymentProperties.getApiServer(), deploymentProperties.getToken(), - false + // verify the api server certificate against the configured CA + true ); - try (FileInputStream caCertStream = new FileInputStream(deploymentProperties.getCaCertPath())) { - client.setSslCaCert(caCertStream); + try (InputStream caCertStream = Files.newInputStream(Paths.get(deploymentProperties.getCaCertPath()))) { + // the api client keeps the stream and reads it when it builds the + // http client, so hand it a re-readable copy instead of a closed file + client.setSslCaCert(new ByteArrayInputStream(caCertStream.readAllBytes())); } return new AppsV1Api(client); - } catch (IOException e) { + } catch (IOException | RuntimeException e) { + // an unreachable cluster or an unusable CA must not stop Admin from + // starting: the scaler degrades to a logged no-op without a client logger.error("kubernetes apiClient create error", e); } return null; } - /** - * kubernetes scaler. - * @param appsV1Api appsV1Api - * @param deploymentProperties deploymentProperties - * @return KubernetesScaler KubernetesScaler - */ - @Bean - @ConditionalOnMissingBean({AppsV1Api.class, DeploymentProperties.class}) - public KubernetesScaler kubernetesScaler(final Optional appsV1Api, final DeploymentProperties deploymentProperties) { - return new KubernetesScaler(appsV1Api, deploymentProperties); - } - } diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/observer/ScaleObserver.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/observer/ScaleObserver.java index cd43f853544e..3337c2cfbfee 100644 --- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/observer/ScaleObserver.java +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/observer/ScaleObserver.java @@ -46,8 +46,8 @@ public ScaleObserver(final KubernetesScaler kubernetesScaler, final ScaleRuleEva public void update(final MetricData metricData, final ScaleRuleDO rule) { try { ScaleAction action = ruleEvaluator.evaluate(metricData, rule); - if (Objects.nonNull(action)) { - kubernetesScaler.scaleByAction(action); + if (Objects.nonNull(action) && !kubernetesScaler.scaleByAction(action)) { + LOG.warn("scale action {} was not applied to the deployment", action.getActionType()); } } catch (Exception e) { LOG.error("update observer error. cause: {} ", e.getMessage()); diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/scaler/KubernetesScaler.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/scaler/KubernetesScaler.java index 8af934449934..828dab3c1088 100644 --- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/scaler/KubernetesScaler.java +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/scaler/KubernetesScaler.java @@ -49,8 +49,15 @@ public KubernetesScaler(final Optional appsV1Api, final DeploymentPro * scale by action. * * @param action action + * @return true when the scale request succeeds + * @throws Exception when the current scale cannot be read from the cluster */ - public void scaleByAction(final ScaleAction action) throws Exception { + public boolean scaleByAction(final ScaleAction action) throws Exception { + if (Objects.isNull(appsV1Api)) { + LOG.warn("kubernetes scale client is unavailable (scaling disabled or client creation failed)," + + " ignoring scale action {}", action.getActionType()); + return false; + } V1Scale scale = appsV1Api.readNamespacedDeploymentScale( deploymentProperties.getName(), deploymentProperties.getNamespace(), @@ -60,7 +67,7 @@ public void scaleByAction(final ScaleAction action) throws Exception { ? spec.getReplicas() : 0; int newReplicas = calculateNewReplicaCount(currentReplicas, action); - scaleByNum(newReplicas); + return scaleByNum(newReplicas); } /** @@ -70,6 +77,11 @@ public void scaleByAction(final ScaleAction action) throws Exception { * @return true when the scale request succeeds */ public boolean scaleByNum(final int replicaCount) { + if (Objects.isNull(appsV1Api)) { + LOG.warn("kubernetes scale client is unavailable (scaling disabled or client creation failed)," + + " ignoring request to scale to {} replicas", replicaCount); + return false; + } try { V1Scale scale = new V1Scale(); scale.setSpec(new V1ScaleSpec().replicas(replicaCount)); diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/config/KubernetesConfigurationTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/config/KubernetesConfigurationTest.java new file mode 100644 index 000000000000..37ba8481f0a8 --- /dev/null +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/config/KubernetesConfigurationTest.java @@ -0,0 +1,184 @@ +/* + * 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.shenyu.admin.config; + +import io.kubernetes.client.openapi.apis.AppsV1Api; +import org.apache.shenyu.admin.config.properties.DeploymentProperties; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.context.ConfigurationPropertiesAutoConfiguration; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.function.Supplier; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +/** + * Bean-condition tests for the kubernetes scaling configuration (#7912). + * The client's back-off condition must key on the {@link AppsV1Api} bean + * type, not on the always-present {@link DeploymentProperties} bean. + */ +public final class KubernetesConfigurationTest { + + /** + * A throwaway self-signed certificate: the client parses the CA when it is + * created, so the tests need real PEM data rather than placeholder bytes. + */ + private static final String SELF_SIGNED_CA = "-----BEGIN CERTIFICATE-----\n" + + "MIIDEzCCAfugAwIBAgIUH4u8FdeymXrwh9z0yN6dp2sem34wDQYJKoZIhvcNAQEL\n" + + "BQAwGTEXMBUGA1UEAwwOc2hlbnl1LXRlc3QtY2EwHhcNMjYxMDIxMTMyODM5WhcN\n" + + "MzYxMDE4MTMyODM5WjAZMRcwFQYDVQQDDA5zaGVueXUtdGVzdC1jYTCCASIwDQYJ\n" + + "KoZIhvcNAQEBBQADggEPADCCAQoCggEBALbaq3kRwM5qaRiksHzGiJcFXk8WZg13\n" + + "wN0tZA4feGXfTXrnKfDGuCtDs6eVaboXB0JGUS/zuWSeL7bjmZQzGvF2NaO9rWj8\n" + + "eI9NvxoRZERMqOae5m+2ZHtFUqThjzXNDVLSmbd71z3i1eXPmHvIk2BM+fGdiEuL\n" + + "09Jhq2sX6/T2/lUeGLxKJHop2gB5L3SQoNS23keEeJbxYw7oH/IZNStT6zfkeMn8\n" + + "YNNVjXPpIsWLkDKMDS7ltxsIvtNhE+uQkpm6EID/XNPvJpsppCSv8g8oiNXOUZMV\n" + + "dN81WkAUAAteIktw1GSc4tm37QExCUy6d3KX8XWYhqWPwtRoCbcGP7sCAwEAAaNT\n" + + "MFEwHQYDVR0OBBYEFPELa31848xyVMlVgjaLYqrLK+9EMB8GA1UdIwQYMBaAFPEL\n" + + "a31848xyVMlVgjaLYqrLK+9EMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQEL\n" + + "BQADggEBAEb70NyFvDOHWUb+UMZdjF+AQ5MZpVIS4vqBDGMTpI56CfIt13J0vjUZ\n" + + "SOs9zZ2bBwtbhfWNW6+c/i66CDONdXLDoYktLy5dFoHbjDUx+78QpWXh2vBPrr9i\n" + + "lVp5TP2B654JMp0cwzS1TmpsBMU5lM1vUhwtxPmClYURJ2M6/nTOg2bHT2/PXUAe\n" + + "QDS9WvENlLMTT+rPVrAP2q1ljKUVYQwnYYd9Z48dYPDvLzIFFKgXINWXoH35daWu\n" + + "HTNnLozedrj3zlU8G3ubEOzmkdapDz7R3zd5ohqlF65c05NDUB4o6I22uOJMpRJ6\n" + + "p07cnl7D8goTqlxI+j6CW0MZui9vmf8=\n" + + "-----END CERTIFICATE-----\n"; + + @TempDir + private Path tempDir; + + private final ApplicationContextRunner runner = new ApplicationContextRunner() + // @ConfigurationProperties binding needs the post-processor, which a plain + // runner does not register (a real Boot app always has it) + .withConfiguration(AutoConfigurations.of(ConfigurationPropertiesAutoConfiguration.class)) + .withUserConfiguration(KubernetesConfiguration.class); + + @Test + public void shouldCreateClientWithStandardBindingWhenEnabled() { + runner.withPropertyValues(enabledProps()) + .run(context -> assertHasSingleClient(context, "the api client must be created when scaling is enabled")); + } + + /** + * The prefix still binds with only the class-level annotation on the way: the + * bound values are what the api client is built from, so an unbound bean would + * surface as a null api server. + */ + @Test + public void shouldBindDeploymentPropertiesFromTheConfiguredPrefix() { + runner.withPropertyValues(enabledProps()) + .run(context -> { + final DeploymentProperties properties = context.getBean(DeploymentProperties.class); + assertEquals("shenyu-bootstrap", properties.getName()); + assertEquals("https://127.0.0.1:6443", properties.getApiServer()); + assertEquals("token", properties.getToken()); + }); + } + + /** + * The reported configuration: a deployment registers its own properties bean. + * The bean name is deliberately not {@code deploymentProperties}, so the + * argument can only be resolved by type — a name coincidence must not be what + * keeps the context from failing (#7912). + */ + @Test + public void shouldCreateClientEvenWhenPropertiesBeanIsPreRegistered() { + runner.withPropertyValues(enabledProps()) + .withBean("preRegisteredProperties", DeploymentProperties.class, + (Supplier) this::deploymentProperties) + .run(context -> assertHasSingleClient(context, + "a pre-registered DeploymentProperties bean must not suppress the api client (#7912)")); + } + + /** + * A user-supplied properties bean overrides the bound one and is the one the + * client is built from. + */ + @Test + public void shouldUseThePreRegisteredProperties() { + final DeploymentProperties custom = deploymentProperties(); + custom.setApiServer("https://pre-registered:6443"); + runner.withPropertyValues(enabledProps()) + .withBean("preRegisteredProperties", DeploymentProperties.class, () -> custom) + .run(context -> { + assertHasSingleClient(context, "the api client must be built from the override"); + assertSame(custom, context.getBean(DeploymentProperties.class), + "the user-supplied properties bean must win the back-off"); + }); + } + + @Test + public void shouldBackOffWhenACustomClientIsProvided() { + final AppsV1Api custom = mock(AppsV1Api.class); + runner.withPropertyValues(enabledProps()) + .withBean("customAppsV1Api", AppsV1Api.class, () -> custom) + .run(context -> assertSame(custom, context.getBean(AppsV1Api.class), + "a user-supplied AppsV1Api must win the back-off")); + } + + @Test + public void shouldNotCreateClientWhenScalingIsDisabled() { + runner.run(context -> assertTrue(context.getBeansOfType(AppsV1Api.class).isEmpty(), + "no api client may be created when scaling is disabled")); + } + + private void assertHasSingleClient(final AssertableApplicationContext context, final String message) { + assertEquals(1, context.getBeansOfType(AppsV1Api.class).size(), message); + } + + private String[] enabledProps() { + return new String[]{ + "shenyu.k8s.scale.enabled=true", + "shenyu.k8s.deployment.name=shenyu-bootstrap", + "shenyu.k8s.deployment.namespace=shenyu", + "shenyu.k8s.deployment.api-server=https://127.0.0.1:6443", + "shenyu.k8s.deployment.token=token", + "shenyu.k8s.deployment.ca-cert-path=" + certFile(), + }; + } + + private String certFile() { + try { + final Path cert = tempDir.resolve("ca.crt"); + if (Files.notExists(cert)) { + Files.write(cert, SELF_SIGNED_CA.getBytes(StandardCharsets.UTF_8)); + } + return cert.toString(); + } catch (Exception e) { + throw new IllegalStateException("cannot prepare the ca cert file", e); + } + } + + private DeploymentProperties deploymentProperties() { + final DeploymentProperties properties = new DeploymentProperties(); + properties.setName("shenyu-bootstrap"); + properties.setNamespace("shenyu"); + properties.setApiServer("https://127.0.0.1:6443"); + properties.setToken("token"); + properties.setCaCertPath(certFile()); + return properties; + } +} diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/scaler/KubernetesScalerTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/scaler/KubernetesScalerTest.java index ed30e0a97b67..93a3c51188da 100644 --- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/scaler/KubernetesScalerTest.java +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/scaler/KubernetesScalerTest.java @@ -34,7 +34,9 @@ import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -106,4 +108,30 @@ public void testScaleByActionReadsCurrentAndAppliesDelta() throws Exception { isNull()); assertEquals(3, captor.getValue().getSpec().getReplicas()); } + + /** + * Scaling stays enabled but the client bean is absent (disabled property + * or a failed client creation left a null behind): the scaler must degrade + * to a logged no-op instead of throwing on the hot path (#7912). + */ + @Test + public void testScaleByNumWithoutClientIsSafeNoOp() { + final KubernetesScaler clientlessScaler = new KubernetesScaler(Optional.empty(), props()); + + assertFalse(clientlessScaler.scaleByNum(3)); + } + + @Test + public void testScaleByActionWithoutClientIsSafeNoOp() throws Exception { + final KubernetesScaler clientlessScaler = new KubernetesScaler(Optional.empty(), props()); + + assertDoesNotThrow(() -> clientlessScaler.scaleByAction(new ScaleAction(ScaleAction.ActionType.SCALE_UP))); + } + + private DeploymentProperties props() { + final DeploymentProperties deploymentProperties = new DeploymentProperties(); + deploymentProperties.setName(DEPLOY_NAME); + deploymentProperties.setNamespace(DEPLOY_NAMESPACE); + return deploymentProperties; + } }