-
Couldn't load subscription status.
- Fork 1.1k
[GenAI] Fix 11799: binding a list to endpoints.env.keys to ensure only 'activeEnvironments' remains. #12145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.10.x
Are you sure you want to change the base?
[GenAI] Fix 11799: binding a list to endpoints.env.keys to ensure only 'activeEnvironments' remains. #12145
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| package io.micronaut.reproduce | ||
|
|
||
| import com.fasterxml.jackson.databind.JsonNode | ||
| import com.fasterxml.jackson.databind.ObjectMapper | ||
| import io.micronaut.context.ApplicationContext | ||
| import io.micronaut.http.HttpRequest | ||
| import io.micronaut.http.HttpResponse | ||
| import io.micronaut.http.client.HttpClient | ||
| import io.micronaut.runtime.server.EmbeddedServer | ||
| import org.junit.jupiter.api.AfterEach | ||
| import org.junit.jupiter.api.Assertions.* | ||
| import org.junit.jupiter.api.Test | ||
|
|
||
| class EnvEndpointKeysConfigTest { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be in Java or Groovy like all our tests and be placed in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tests in |
||
|
|
||
| private val mapper = ObjectMapper() | ||
|
|
||
| private var server: EmbeddedServer? = null | ||
| private var client: HttpClient? = null | ||
|
|
||
| @AfterEach | ||
| fun cleanup() { | ||
| try { client?.close() } catch (e: Exception) { /* ignore */ } | ||
| try { server?.close() } catch (e: Exception) { /* ignore */ } | ||
| } | ||
|
|
||
| @Test | ||
| fun `default env endpoint contains packages and propertySources`() { | ||
| val props = HashMap<String, Any?>() | ||
| props["endpoints.env.enabled"] = true | ||
| // ensure the endpoint is exposed over HTTP for newer Micronaut versions | ||
| props["management.endpoints.web.exposure.include"] = "env" | ||
| // try to avoid security blocking the endpoint in test environments | ||
| props["micronaut.security.enabled"] = false | ||
| props["micronaut.server.port"] = 0 | ||
|
|
||
| server = ApplicationContext.run(EmbeddedServer::class.java, props) | ||
| client = HttpClient.create(server!!.url) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the client here is never closed and will leak resources |
||
|
|
||
| val node = retrieveEnvJsonNode() | ||
|
|
||
| assertTrue(node.has("activeEnvironments"), "activeEnvironments should be present by default. Body: $node") | ||
| assertTrue(node.has("packages"), "packages should be present by default. Body: $node") | ||
| assertTrue(node.has("propertySources"), "propertySources should be present by default. Body: $node") | ||
| } | ||
|
|
||
| @Test | ||
| fun `env endpoint respects endpoints env keys when configured to only activeEnvironments (list binding)`() { | ||
| val props = HashMap<String, Any?>() | ||
| props["endpoints.env.enabled"] = true | ||
| props["management.endpoints.web.exposure.include"] = "env" | ||
| props["micronaut.security.enabled"] = false | ||
| // bind keys as a list containing only activeEnvironments | ||
| props["endpoints.env.keys"] = listOf("activeEnvironments") | ||
| props["micronaut.server.port"] = 0 | ||
|
|
||
| server = ApplicationContext.run(EmbeddedServer::class.java, props) | ||
| client = HttpClient.create(server!!.url) | ||
|
|
||
| val node = retrieveEnvJsonNode() | ||
|
|
||
| // Expect only the configured key to be present | ||
| val fieldNames = node.fieldNames().asSequence().toList() | ||
| assertEquals(1, fieldNames.size, "Expected exactly one top-level key when endpoints.env.keys is configured. Body: $node") | ||
| assertTrue(node.has("activeEnvironments"), "activeEnvironments should be present when configured. Body: $node") | ||
| assertFalse(node.has("packages"), "packages should not be present when not configured. Body: $node") | ||
| assertFalse(node.has("propertySources"), "propertySources should not be present when not configured. Body: $node") | ||
| } | ||
|
|
||
| private fun retrieveEnvJsonNode(): JsonNode { | ||
| val pathsToTry = arrayOf( | ||
| "/env", | ||
| "/env/", | ||
| "/endpoints/env", | ||
| "/endpoints/env/", | ||
| "/management/env", | ||
| "/management/env/" | ||
| ) | ||
| var lastBody: String? = null | ||
| for (path in pathsToTry) { | ||
| try { | ||
| val resp: HttpResponse<String> = client!!.toBlocking().exchange(HttpRequest.GET<Any>(path).accept("application/json"), String::class.java) | ||
| val code = resp.status.code | ||
| lastBody = resp.body.orElse(null) | ||
| if (code in 200..299) { | ||
| val root = mapper.readTree(lastBody ?: "{}") | ||
| // some Micronaut versions wrap endpoint output (e.g. {"result":{...}} or {"value":{...}}) | ||
| findNodeWithActiveEnvironments(root)?.let { return it } | ||
| // otherwise if root itself is an object, return it | ||
| if (root.isObject) { | ||
| return root | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| lastBody = e.message | ||
| } | ||
| } | ||
|
|
||
| // fallback: try to locate a known env endpoint bean and serialize it | ||
| try { | ||
| val ctx = server!!.applicationContext | ||
| val candidateClassNames = listOf( | ||
| "io.micronaut.management.endpoint.env.EnvironmentEndpoint", | ||
| "io.micronaut.management.endpoint.env.EnvEndpoint", | ||
| "io.micronaut.management.endpoint.env.EnvironmentController" | ||
| ) | ||
|
|
||
| for (className in candidateClassNames) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is very weird logic, why is reflection being used? |
||
| try { | ||
| val clazz = Class.forName(className) | ||
| val bean = ctx.getBean(clazz) | ||
| // try to find a no-arg method that returns the payload | ||
| val method = clazz.methods.firstOrNull { m -> m.parameterCount == 0 } | ||
| val result = if (method != null) { | ||
| method.invoke(bean) | ||
| } else bean | ||
| val json = mapper.writeValueAsString(result ?: mapOf<String, Any>()) | ||
| val root = mapper.readTree(json) | ||
| findNodeWithActiveEnvironments(root)?.let { return it } | ||
| if (root.isObject) return root | ||
| } catch (cnf: ClassNotFoundException) { | ||
| // try next | ||
| } | ||
| } | ||
|
|
||
| throw AssertionError("Could not locate env endpoint via HTTP or known bean classes. Last HTTP message: $lastBody") | ||
| } catch (e: Exception) { | ||
| throw AssertionError("Failed to retrieve env endpoint payload. Last HTTP message: $lastBody", e) | ||
| } | ||
| } | ||
|
|
||
| private fun findNodeWithActiveEnvironments(root: JsonNode): JsonNode? { | ||
| // If the node itself contains the key, return it | ||
| if (root.has("activeEnvironments")) return root | ||
| // Common wrappers: value, result, data | ||
| val wrapperNames = listOf("value", "result", "data", "environment") | ||
| for (name in wrapperNames) { | ||
| val child = root.get(name) | ||
| if (child != null && child.has("activeEnvironments")) return child | ||
| } | ||
| // search children shallowly | ||
| if (root.isObject) { | ||
| val fields = root.fieldNames() | ||
| while (fields.hasNext()) { | ||
| val f = root.get(fields.next()) | ||
| if (f != null && f.has("activeEnvironments")) return f | ||
| } | ||
| } | ||
| return null | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fix seems unnecessary and the functionality already implemented by
setActiveKeysso it appears this is just a documentation issue and no fix is needed.