From 1d4f65c0cfbad72c2c52bd62d2491f73f46b30cd Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Mon, 30 Jan 2023 14:28:11 +0800 Subject: [PATCH] feat: add some APIs to separate plugin-related Setting and ConfigMap permissions (#3142) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /kind api-change /milestone 2.2.x #### What this PR does / why we need it: 添加自定义 APIs 以分离插件对 Setting 和 ConfigMap 权限的依赖 ⚠️ 此 PR 新增了 APIs，需要 Console 对其进行适配，插件查询 Setting、ConfigMap 的 APIs 需要更改。 #### Which issue(s) this PR fixes: A part of https://github.com/halo-dev/halo/issues/3069 Fixes https://github.com/halo-dev/halo/issues/3069 #### Special notes for your reviewer: how to test it? - 测试 GET /plugins/{name}/setting 是否能正确获取到插件名称对应的主题的 Setting。 - 测试 GET /plugins/{name}/config 是否能正确获取到插件名称对应的主题的 ConfigMap。 - 测试 PUT /plugins/{name}/config 是否能更新插件的 ConfigMap，如果名称不匹配则抛出异常。 切换用户为其分配主题的查看权限可以有权限调用以上描述的 GET 请求的 endpoints，管理权限可以调用 PUT /plugins/{name}/config。 /cc @halo-dev/sig-halo #### Does this PR introduce a user-facing change? ```release-note 添加自定义 APIs 以分离插件对 Setting 和 ConfigMap 权限的依赖 ``` --- .../extension/endpoint/PluginEndpoint.java | 96 +++++++++++++++ .../extensions/role-template-plugin.yaml | 4 +- .../endpoint/PluginEndpointTest.java | 112 ++++++++++++++++++ 3 files changed, 210 insertions(+), 2 deletions(-) diff --git a/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java b/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java index 82532f93f..164792e6a 100644 --- a/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java +++ b/src/main/java/run/halo/app/core/extension/endpoint/PluginEndpoint.java @@ -104,6 +104,23 @@ public class PluginEndpoint implements CustomEndpoint { .content(contentBuilder().mediaType(MediaType.MULTIPART_FORM_DATA_VALUE) .schema(schemaBuilder().implementation(InstallRequest.class)))) ) + .PUT("plugins/{name}/config", this::updatePluginConfig, + builder -> builder.operationId("updatePluginConfig") + .description("Update the configMap of plugin setting.") + .tag(tag) + .parameter(parameterBuilder() + .name("name") + .in(ParameterIn.PATH) + .required(true) + .implementation(String.class) + ) + .requestBody(requestBodyBuilder() + .required(true) + .content(contentBuilder().mediaType(MediaType.APPLICATION_JSON_VALUE) + .schema(schemaBuilder().implementation(ConfigMap.class)))) + .response(responseBuilder() + .implementation(ConfigMap.class)) + ) .PUT("plugins/{name}/reset-config", this::resetSettingConfig, builder -> builder.operationId("ResetPluginConfig") .description("Reset the configMap of plugin setting.") @@ -124,9 +141,88 @@ public class PluginEndpoint implements CustomEndpoint { .response(responseBuilder().implementation(generateGenericClass(Plugin.class))); buildParametersFromType(builder, ListRequest.class); }) + .GET("plugins/{name}/setting", this::fetchPluginSetting, + builder -> builder.operationId("fetchPluginSetting") + .description("Fetch setting of plugin.") + .tag(tag) + .parameter(parameterBuilder() + .name("name") + .in(ParameterIn.PATH) + .required(true) + .implementation(String.class) + ) + .response(responseBuilder() + .implementation(Setting.class)) + ) + .GET("plugins/{name}/config", this::fetchPluginConfig, + builder -> builder.operationId("fetchPluginConfig") + .description("Fetch configMap of plugin by configured configMapName.") + .tag(tag) + .parameter(parameterBuilder() + .name("name") + .in(ParameterIn.PATH) + .required(true) + .implementation(String.class) + ) + .response(responseBuilder() + .implementation(ConfigMap.class)) + ) .build(); } + private Mono fetchPluginConfig(ServerRequest request) { + final var name = request.pathVariable("name"); + return client.fetch(Plugin.class, name) + .mapNotNull(plugin -> plugin.getSpec().getConfigMapName()) + .flatMap(configMapName -> client.fetch(ConfigMap.class, configMapName)) + .flatMap(configMap -> ServerResponse.ok().bodyValue(configMap)); + } + + private Mono fetchPluginSetting(ServerRequest request) { + final var name = request.pathVariable("name"); + return client.fetch(Plugin.class, name) + .mapNotNull(plugin -> plugin.getSpec().getSettingName()) + .flatMap(settingName -> client.fetch(Setting.class, settingName)) + .flatMap(setting -> ServerResponse.ok().bodyValue(setting)); + } + + private Mono updatePluginConfig(ServerRequest request) { + final var pluginName = request.pathVariable("name"); + return client.fetch(Plugin.class, pluginName) + .doOnNext(plugin -> { + String configMapName = plugin.getSpec().getConfigMapName(); + if (!StringUtils.hasText(configMapName)) { + throw new ServerWebInputException( + "Unable to complete the request because the plugin configMapName is blank"); + } + }) + .flatMap(plugin -> { + final String configMapName = plugin.getSpec().getConfigMapName(); + return request.bodyToMono(ConfigMap.class) + .doOnNext(configMapToUpdate -> { + var configMapNameToUpdate = configMapToUpdate.getMetadata().getName(); + if (!configMapName.equals(configMapNameToUpdate)) { + throw new ServerWebInputException( + "The name from the request body does not match the plugin " + + "configMapName name."); + } + }) + .flatMap(configMapToUpdate -> client.fetch(ConfigMap.class, configMapName) + .map(persisted -> { + configMapToUpdate.getMetadata() + .setVersion(persisted.getMetadata().getVersion()); + return configMapToUpdate; + }) + .switchIfEmpty(client.create(configMapToUpdate)) + ) + .flatMap(client::update) + .retryWhen(Retry.backoff(5, Duration.ofMillis(300)) + .filter(OptimisticLockingFailureException.class::isInstance) + ); + }) + .flatMap(configMap -> ServerResponse.ok().bodyValue(configMap)); + } + private Mono resetSettingConfig(ServerRequest request) { String name = request.pathVariable("name"); return client.fetch(Plugin.class, name) diff --git a/src/main/resources/extensions/role-template-plugin.yaml b/src/main/resources/extensions/role-template-plugin.yaml index 63593a629..6e433d8df 100644 --- a/src/main/resources/extensions/role-template-plugin.yaml +++ b/src/main/resources/extensions/role-template-plugin.yaml @@ -16,7 +16,7 @@ rules: resources: [ "plugins" ] verbs: [ "create", "patch", "update", "delete", "deletecollection" ] - apiGroups: [ "api.console.halo.run" ] - resources: [ "plugins/upgrade", "plugins/resetconfig" ] + resources: [ "plugins/upgrade", "plugins/resetconfig", "plugins/config" ] verbs: [ "*" ] - nonResourceURLs: [ "/apis/api.console.halo.run/v1alpha1/plugins/*" ] verbs: [ "create" ] @@ -37,5 +37,5 @@ rules: resources: [ "plugins" ] verbs: [ "get", "list" ] - apiGroups: [ "api.console.halo.run" ] - resources: [ "plugins" ] + resources: [ "plugins", "plugins/setting", "plugins/config" ] verbs: [ "get", "list" ] diff --git a/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java b/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java index c6aed940d..102213a57 100644 --- a/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java +++ b/src/test/java/run/halo/app/core/extension/endpoint/PluginEndpointTest.java @@ -4,6 +4,7 @@ import static java.util.Objects.requireNonNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; @@ -38,6 +39,8 @@ import org.springframework.http.client.MultipartBodyBuilder; import org.springframework.test.web.reactive.server.WebTestClient; import reactor.core.publisher.Mono; import run.halo.app.core.extension.Plugin; +import run.halo.app.core.extension.Setting; +import run.halo.app.extension.ConfigMap; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; @@ -306,6 +309,115 @@ class PluginEndpointTest { } + @Nested + class UpdatePluginConfigTest { + WebTestClient webClient; + + @BeforeEach + void setUp() { + webClient = WebTestClient.bindToRouterFunction(endpoint.endpoint()) + .build(); + } + + @Test + void updateWhenConfigMapNameIsNull() { + Plugin plugin = createPlugin("fake-plugin"); + plugin.getSpec().setConfigMapName(null); + + when(client.fetch(eq(Plugin.class), eq("fake-plugin"))).thenReturn(Mono.just(plugin)); + webClient.put() + .uri("/plugins/fake-plugin/config") + .exchange() + .expectStatus().isBadRequest(); + } + + @Test + void updateWhenConfigMapNameNotMatch() { + Plugin plugin = createPlugin("fake-plugin"); + plugin.getSpec().setConfigMapName("fake-config-map"); + + when(client.fetch(eq(Plugin.class), eq("fake-plugin"))).thenReturn(Mono.just(plugin)); + webClient.put() + .uri("/plugins/fake-plugin/config") + .body(Mono.fromSupplier(() -> { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new Metadata()); + configMap.getMetadata().setName("not-match"); + return configMap; + }), ConfigMap.class) + .exchange() + .expectStatus().isBadRequest(); + } + + @Test + void updateWhenConfigMapNameMatch() { + Plugin plugin = createPlugin("fake-plugin"); + plugin.getSpec().setConfigMapName("fake-config-map"); + + when(client.fetch(eq(Plugin.class), eq("fake-plugin"))).thenReturn(Mono.just(plugin)); + when(client.fetch(eq(ConfigMap.class), eq("fake-config-map"))).thenReturn(Mono.empty()); + when(client.create(any(ConfigMap.class))).thenReturn(Mono.empty()); + + webClient.put() + .uri("/plugins/fake-plugin/config") + .body(Mono.fromSupplier(() -> { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new Metadata()); + configMap.getMetadata().setName("fake-config-map"); + return configMap; + }), ConfigMap.class) + .exchange() + .expectStatus().isOk(); + } + } + + @Nested + class PluginConfigAndSettingFetchTest { + WebTestClient webClient; + + @BeforeEach + void setUp() { + webClient = WebTestClient.bindToRouterFunction(endpoint.endpoint()) + .build(); + } + + @Test + void fetchSetting() { + Plugin plugin = createPlugin("fake"); + plugin.getSpec().setSettingName("fake-setting"); + + when(client.fetch(eq(Setting.class), eq("fake-setting"))) + .thenReturn(Mono.just(new Setting())); + + when(client.fetch(eq(Plugin.class), eq("fake"))).thenReturn(Mono.just(plugin)); + webClient.get() + .uri("/plugins/fake/setting") + .exchange() + .expectStatus().isOk(); + + verify(client).fetch(eq(Setting.class), eq("fake-setting")); + verify(client).fetch(eq(Plugin.class), eq("fake")); + } + + @Test + void fetchConfig() { + Plugin plugin = createPlugin("fake"); + plugin.getSpec().setConfigMapName("fake-config"); + + when(client.fetch(eq(ConfigMap.class), eq("fake-config"))) + .thenReturn(Mono.just(new ConfigMap())); + + when(client.fetch(eq(Plugin.class), eq("fake"))).thenReturn(Mono.just(plugin)); + webClient.get() + .uri("/plugins/fake/config") + .exchange() + .expectStatus().isOk(); + + verify(client).fetch(eq(ConfigMap.class), eq("fake-config")); + verify(client).fetch(eq(Plugin.class), eq("fake")); + } + } + Plugin createPlugin(String name) { return createPlugin(name, "fake display name", "fake description", null); }