From 25c2cd0f72e3017614be7e420e158b6df48b021f Mon Sep 17 00:00:00 2001 From: Vamsi-klu Date: Sat, 17 Oct 2026 12:34:20 +0000 Subject: [PATCH 1/3] fix: keep service lists available after connection failures (#31887) --- .../it/tests/McpServiceResourceIT.java | 109 +++- .../tests/ServiceConnectionResilienceIT.java | 502 ++++++++++++++++++ .../it/tests/ServicesOverviewResourceIT.java | 2 + .../services/ServiceEntityResource.java | 22 +- .../services/ServiceEntityResourceTest.java | 195 +++++++ 5 files changed, 825 insertions(+), 5 deletions(-) create mode 100644 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServiceConnectionResilienceIT.java create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ServiceEntityResourceTest.java diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/McpServiceResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/McpServiceResourceIT.java index c7c5eaf5f4dd..d569ab0d23ba 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/McpServiceResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/McpServiceResourceIT.java @@ -1,5 +1,108 @@ package org.openmetadata.it.tests; -// TODO: Implement McpServiceResourceIT extending BaseServiceIT -// Requires adding McpServiceService to openmetadata-sdk first (only McpServerService exists). -// See LLMServiceResourceIT.java for the pattern to follow. +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.openmetadata.it.util.SdkClients; +import org.openmetadata.it.util.TestNamespace; +import org.openmetadata.it.util.TestNamespaceExtension; +import org.openmetadata.schema.api.domains.CreateDomain; +import org.openmetadata.schema.api.services.CreateMcpService; +import org.openmetadata.schema.entity.domains.Domain; +import org.openmetadata.schema.entity.services.McpService; +import org.openmetadata.schema.utils.JsonUtils; +import org.openmetadata.sdk.network.HttpClient; +import org.openmetadata.sdk.network.HttpMethod; +import org.openmetadata.sdk.network.RequestOptions; +import org.openmetadata.service.Entity; +import org.openmetadata.service.resources.services.mcp.McpServiceResource.McpServiceList; +import org.openmetadata.service.secrets.masker.PasswordEntityMasker; + +@Execution(ExecutionMode.CONCURRENT) +@ExtendWith(TestNamespaceExtension.class) +public class McpServiceResourceIT { + private static final String COLLECTION = "/v1/services/mcpServices"; + private static final String API_KEY = "synthetic-mcp-smoke-api-key"; + + @Test + void list_preservesIdentityFieldsAndCredentialMasking(TestNamespace ns) { + final Domain domain = + ns.trackRoot( + Entity.DOMAIN, + SdkClients.adminClient() + .domains() + .create( + new CreateDomain() + .withName(ns.prefix("mcp_domain")) + .withDescription("MCP service list isolation") + .withDomainType(CreateDomain.DomainType.AGGREGATE))); + final CreateMcpService request = + JsonUtils.readValue( + """ + {"serviceType":"Mcp","connection":{"config":{"type":"Mcp","servers":[{ + "name":"http-server","transport":"SSE","url":"http://localhost:9000/sse", + "apiKey":"synthetic-mcp-smoke-api-key"}]}}} + """, + CreateMcpService.class) + .withName(ns.prefix("mcp_service")) + .withDescription("Healthy MCP service") + .withDomains(List.of(domain.getFullyQualifiedName())); + final HttpClient human = SdkClients.adminClient().getHttpClient(); + final McpService created = + human.execute(HttpMethod.POST, COLLECTION, request, McpService.class); + try { + final McpServiceList listed = + human.execute( + HttpMethod.GET, + COLLECTION, + null, + McpServiceList.class, + RequestOptions.builder() + .queryParam("domain", domain.getFullyQualifiedName()) + .queryParam("fields", "domains,owners") + .queryParam("includePermissions", "true") + .build()); + assertEquals(1, listed.getData().size()); + final McpService row = listed.getData().getFirst(); + assertEquals(created.getId(), row.getId()); + assertEquals(request.getDescription(), row.getDescription()); + assertEquals( + List.of(domain.getId()), + row.getDomains().stream().map(reference -> reference.getId()).toList()); + assertTrue(listed.getEntityPermissions().containsKey(created.getId().toString())); + assertEquals( + PasswordEntityMasker.PASSWORD_MASK, + JsonUtils.readTree(JsonUtils.pojoToJson(row.getConnection().getConfig())) + .path("servers") + .get(0) + .path("apiKey") + .asText()); + final McpService bot = + SdkClients.ingestionBotClient() + .getHttpClient() + .execute(HttpMethod.GET, COLLECTION + "/" + created.getId(), null, McpService.class); + assertEquals( + API_KEY, + JsonUtils.readTree(JsonUtils.pojoToJson(bot.getConnection().getConfig())) + .path("servers") + .get(0) + .path("apiKey") + .asText()); + } finally { + human.execute( + HttpMethod.DELETE, + COLLECTION + "/" + created.getId(), + null, + Void.class, + RequestOptions.builder() + .queryParam("hardDelete", "true") + .queryParam("recursive", "true") + .build()); + } + } +} diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServiceConnectionResilienceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServiceConnectionResilienceIT.java new file mode 100644 index 000000000000..0560d0bcda04 --- /dev/null +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServiceConnectionResilienceIT.java @@ -0,0 +1,502 @@ +package org.openmetadata.it.tests; + +import static org.junit.jupiter.api.Assertions.assertAll; +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 static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.macasaet.fernet.Key; +import com.macasaet.fernet.Token; +import java.security.SecureRandom; +import java.util.List; +import java.util.UUID; +import java.util.function.Consumer; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.parallel.Isolated; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.openmetadata.it.util.SdkClients; +import org.openmetadata.it.util.TestNamespace; +import org.openmetadata.it.util.TestNamespaceExtension; +import org.openmetadata.schema.api.domains.CreateDomain; +import org.openmetadata.schema.entity.domains.Domain; +import org.openmetadata.schema.type.Include; +import org.openmetadata.schema.utils.JsonUtils; +import org.openmetadata.sdk.client.OpenMetadataClient; +import org.openmetadata.sdk.exceptions.OpenMetadataException; +import org.openmetadata.sdk.network.HttpMethod; +import org.openmetadata.sdk.network.RequestOptions; +import org.openmetadata.service.Entity; +import org.openmetadata.service.jdbi3.EntityDAO; +import org.openmetadata.service.jdbi3.EntityRepository; +import org.openmetadata.service.secrets.masker.PasswordEntityMasker; +import org.openmetadata.service.util.RestUtil; + +@Execution(ExecutionMode.CONCURRENT) +@Isolated("Metadata service listing reports a global total that concurrent fixtures can change") +@ExtendWith(TestNamespaceExtension.class) +public class ServiceConnectionResilienceIT { + private static final String TEST_PASSWORD = "synthetic-service-resilience-password"; + private static final String OTHER_KEY = "ZJNP0-FkaNPWmLsuhZq1dpQ5Cr2I8bG7vRlkjJjd25A="; + private static final ServiceCase DATABASE = + new ServiceCase( + Entity.DATABASE_SERVICE, + "databaseServices", + "Postgres", + """ + {"type":"Postgres","hostPort":"localhost:5432","username":"test", + "authType":{"password":"synthetic-service-resilience-password"}} + """); + + @AfterEach + void removeFixtures(TestNamespace ns) { + assertAll( + ns.drainTrackedRoots().reversed().stream() + .map(root -> (Executable) () -> removeFixture(root))); + } + + private static void removeFixture(TestNamespace.EntityRoot root) { + final String path = + Entity.DOMAIN.equals(root.entityType()) + ? "/v1/domains" + : serviceCases() + .filter(serviceCase -> serviceCase.entityType().equals(root.entityType())) + .findFirst() + .orElseThrow() + .path(); + SdkClients.adminClient() + .getHttpClient() + .execute( + HttpMethod.DELETE, + path + "/" + root.id(), + null, + Void.class, + RequestOptions.builder() + .queryParam("hardDelete", "true") + .queryParam("recursive", "true") + .build()); + } + + @ParameterizedTest + @MethodSource("invalidTokens") + void list_withUndecryptableConnection_keepsHealthyServices( + String invalidToken, TestNamespace ns) { + final ServicePage fixture = createPage(DATABASE, ns); + final JsonNode baseline = list(fixture, SdkClients.adminClient()); + try (StoredConnection stored = snapshot(fixture)) { + stored.changeConfig( + config -> ((ObjectNode) config.path("authType")).put("password", invalidToken)); + assertMixedPage(fixture, baseline, list(fixture, SdkClients.adminClient())); + assertHealthyBotSecrets(fixture); + assertSingleEntityStillFails(fixture); + assertCursorTraversal(fixture); + stored.assertUnchangedAfterReads(); + } + assertEquals(baseline, list(fixture, SdkClients.adminClient())); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("serviceCases") + void list_withInvalidInnerConfig_keepsEveryServiceCategory( + ServiceCase serviceCase, TestNamespace ns) { + final ServicePage fixture = createPage(serviceCase, ns); + final JsonNode baseline = list(fixture, SdkClients.adminClient()); + try (StoredConnection stored = snapshot(fixture)) { + stored.changeConfig(config -> config.put("unsupportedConnectionField31887", true)); + assertMixedPage(fixture, baseline, list(fixture, SdkClients.adminClient())); + assertSingleEntityStillFails(fixture); + stored.assertUnchangedAfterReads(); + } + assertEquals(baseline, list(fixture, SdkClients.adminClient())); + } + + @Test + void list_withInvalidNestedSslSecret_clearsEntireConnection(TestNamespace ns) { + final ServicePage fixture = createPage(DATABASE, ns); + final JsonNode baseline = list(fixture, SdkClients.adminClient()); + try (StoredConnection stored = snapshot(fixture)) { + stored.changeConfig( + config -> + config.putObject("sslConfig").put("caCertificate", "fernet:invalid-certificate")); + assertMixedPage(fixture, baseline, list(fixture, SdkClients.adminClient())); + stored.assertUnchangedAfterReads(); + } + assertEquals(baseline, list(fixture, SdkClients.adminClient())); + } + + @ParameterizedTest + @MethodSource("absentConnections") + void list_withAbsentConnectionOrConfig_keepsService( + Consumer change, TestNamespace ns) { + final ServicePage fixture = createPage(DATABASE, ns); + final JsonNode baseline = list(fixture, SdkClients.adminClient()); + try (StoredConnection stored = snapshot(fixture)) { + stored.changeDocument(change); + assertMixedPage(fixture, baseline, list(fixture, SdkClients.adminClient())); + stored.assertUnchangedAfterReads(); + } + assertEquals(baseline, list(fixture, SdkClients.adminClient())); + } + + private static Stream> absentConnections() { + return Stream.of( + document -> document.remove("connection"), + document -> ((ObjectNode) document.path("connection")).putNull("config")); + } + + @Test + void writes_keepMaskingAndInvalidConnectionErrors(TestNamespace ns) { + final ServicePage fixture = createPage(DATABASE, ns); + final ObjectNode update = + serviceRequest( + DATABASE, fixture.domain(), fixture.services().getFirst().path("name").asText()); + ((ObjectNode) update.path("connection").path("config").path("authType")) + .put("password", PasswordEntityMasker.PASSWORD_MASK); + putService(update); + assertHealthyPassword(fixture.services().getFirst()); + final ObjectNode invalid = update.deepCopy(); + ((ObjectNode) invalid.path("connection").path("config")) + .put("unsupportedConnectionField31887", true); + assertEquals( + 400, assertThrows(OpenMetadataException.class, () -> putService(invalid)).getStatusCode()); + assertHealthyPassword(fixture.services().getFirst()); + final JsonNode created = + putService(serviceRequest(DATABASE, fixture.domain(), ns.prefix(ns.uniqueShortId()))); + ns.trackRoot(Entity.DATABASE_SERVICE, UUID.fromString(created.path("id").asText())); + assertHealthyPassword(created); + } + + private static JsonNode putService(ObjectNode request) { + return JsonUtils.readTree( + SdkClients.adminClient() + .getHttpClient() + .executeForString(HttpMethod.PUT, DATABASE.path(), request)); + } + + private static void assertHealthyPassword(JsonNode service) { + final JsonNode response = + JsonUtils.readTree( + SdkClients.ingestionBotClient() + .getHttpClient() + .executeForString( + HttpMethod.GET, DATABASE.path() + "/" + service.path("id").asText(), null)); + assertEquals( + TEST_PASSWORD, + response.path("connection").path("config").path("authType").path("password").asText()); + } + + private static Stream invalidTokens() { + return Stream.of( + "fernet:" + + Token.generate(new SecureRandom(), new Key(OTHER_KEY), TEST_PASSWORD).serialise(), + "fernet:not-a-valid-token"); + } + + private static Stream serviceCases() { + return Stream.of( + DATABASE, + new ServiceCase( + Entity.DASHBOARD_SERVICE, + "dashboardServices", + "Metabase", + "{\"type\":\"Metabase\",\"hostPort\":\"http://localhost:3000\",\"username\":\"test\"}"), + new ServiceCase( + Entity.MESSAGING_SERVICE, + "messagingServices", + "Kafka", + "{\"type\":\"Kafka\",\"bootstrapServers\":\"localhost:9092\"}"), + new ServiceCase( + Entity.PIPELINE_SERVICE, + "pipelineServices", + "Airflow", + "{\"type\":\"Airflow\",\"hostPort\":\"http://localhost:8080\"}"), + new ServiceCase( + Entity.MLMODEL_SERVICE, + "mlmodelServices", + "Mlflow", + "{\"type\":\"Mlflow\",\"trackingUri\":\"http://localhost:5000\",\"registryUri\":\"http://localhost:5000\"}"), + new ServiceCase( + Entity.METADATA_SERVICE, + "metadataServices", + "Amundsen", + "{\"type\":\"Amundsen\",\"hostPort\":\"bolt://localhost:7687\",\"username\":\"test\",\"password\":\"test\"}"), + new ServiceCase( + Entity.STORAGE_SERVICE, + "storageServices", + "S3", + "{\"type\":\"S3\",\"awsConfig\":{\"awsRegion\":\"us-east-1\"}}"), + new ServiceCase( + Entity.SEARCH_SERVICE, + "searchServices", + "ElasticSearch", + "{\"type\":\"ElasticSearch\",\"hostPort\":\"http://localhost:9200\"}"), + new ServiceCase( + Entity.API_SERVICE, + "apiServices", + "Rest", + "{\"type\":\"Rest\",\"openAPISchemaConnection\":{\"openAPISchemaURL\":\"http://localhost:8585/swagger.json\"}}"), + new ServiceCase( + Entity.LLM_SERVICE, + "llmServices", + "CustomLLM", + "{\"type\":\"CustomLLM\",\"baseURL\":\"http://localhost:8080\"}"), + new ServiceCase( + Entity.DRIVE_SERVICE, + "driveServices", + "GoogleDrive", + """ + {"type":"GoogleDrive","driveId":"test-drive","credentials":{"gcpConfig":{ + "type":"service_account","projectId":"test-project","privateKeyId":"test-key-id", + "privateKey":"test-private-key","clientEmail":"test@test-project.iam.gserviceaccount.com", + "clientId":"123456789"}}} + """), + new ServiceCase( + Entity.SECURITY_SERVICE, + "securityServices", + "Ranger", + "{\"type\":\"Ranger\",\"hostPort\":\"http://localhost:6080\",\"authType\":{\"username\":\"test\",\"password\":\"test\"}}"), + new ServiceCase(Entity.MCP_SERVICE, "mcpServices", "Mcp", "{\"type\":\"Mcp\"}")); + } + + private static ServicePage createPage(ServiceCase serviceCase, TestNamespace ns) { + final String fixtureId = ns.uniqueShortId(); + final Domain domain = + ns.trackRoot( + Entity.DOMAIN, + SdkClients.adminClient() + .domains() + .create( + new CreateDomain() + .withName(ns.prefix(serviceCase.entityType() + fixtureId)) + .withDescription("Service connection resilience isolation") + .withDomainType(CreateDomain.DomainType.AGGREGATE))); + final List services = + Stream.of("a_healthy", "b_broken", "c_healthy") + .map(name -> createService(serviceCase, domain, ns.prefix(fixtureId + "_" + name), ns)) + .toList(); + return new ServicePage(serviceCase, domain.getFullyQualifiedName(), services, fixtureId); + } + + private static JsonNode createService( + ServiceCase serviceCase, Domain domain, String name, TestNamespace ns) { + final ObjectNode request = serviceRequest(serviceCase, domain.getFullyQualifiedName(), name); + final JsonNode created = + JsonUtils.readTree( + SdkClients.adminClient() + .getHttpClient() + .executeForString(HttpMethod.POST, serviceCase.path(), request)); + ns.trackRoot(serviceCase.entityType(), UUID.fromString(created.path("id").asText())); + return created; + } + + private static ObjectNode serviceRequest(ServiceCase serviceCase, String domain, String name) { + final ObjectNode request = + JsonUtils.getObjectNode().put("name", name).put("serviceType", serviceCase.serviceType()); + request.putArray("domains").add(domain); + request.putObject("connection").set("config", JsonUtils.readTree(serviceCase.config())); + return request; + } + + private static JsonNode list(ServicePage fixture, OpenMetadataClient client) { + return list(fixture, client, options(fixture).queryParam("limit", "3").build()); + } + + private static JsonNode list( + ServicePage fixture, OpenMetadataClient client, RequestOptions options) { + return JsonUtils.readTree( + client + .getHttpClient() + .executeForString(HttpMethod.GET, fixture.serviceCase().path(), null, options)); + } + + private static RequestOptions.Builder options(ServicePage fixture) { + final RequestOptions.Builder options = + RequestOptions.builder() + .queryParam("domain", fixture.domain()) + .queryParam("fields", "owners,domains") + .queryParam("includePermissions", "true"); + if (Entity.METADATA_SERVICE.equals(fixture.serviceCase().entityType())) { + // Metadata service listing does not accept a domain filter, so isolate its ordered page. + final String cursor = + JsonUtils.getObjectNode() + .put("name", fixture.namePrefix()) + .put("id", new UUID(0, 0).toString()) + .toString(); + options.queryParam("after", RestUtil.encodeCursor(cursor)); + } + return options; + } + + private static void assertMixedPage(ServicePage fixture, JsonNode baseline, JsonNode response) { + assertEquals(3, response.path("data").size()); + assertTrue(baseline.path("entityPermissions").isObject()); + assertEquals(3, baseline.path("entityPermissions").size()); + assertEquals(baseline.path("paging"), response.path("paging")); + assertEquals(baseline.path("entityPermissions"), response.path("entityPermissions")); + for (int index = 0; index < fixture.services().size(); index++) { + final JsonNode before = baseline.path("data").get(index); + final JsonNode after = response.path("data").get(index); + assertTrue( + baseline + .path("entityPermissions") + .has(fixture.services().get(index).path("id").asText())); + assertEquals(fixture.services().get(index).path("id"), after.path("id")); + if (index == 1) { + assertTrue(after.path("connection").isMissingNode() || after.path("connection").isNull()); + final ObjectNode expected = before.deepCopy(); + final ObjectNode actual = after.deepCopy(); + expected.remove("connection"); + actual.remove("connection"); + assertEquals(expected, actual); + } else { + assertEquals(before, after); + } + } + } + + private static void assertHealthyBotSecrets(ServicePage fixture) { + final JsonNode data = list(fixture, SdkClients.ingestionBotClient()).path("data"); + assertEquals( + TEST_PASSWORD, + data.get(0).path("connection").path("config").path("authType").path("password").asText()); + assertEquals( + TEST_PASSWORD, + data.get(2).path("connection").path("config").path("authType").path("password").asText()); + assertTrue( + data.get(1).path("connection").isMissingNode() || data.get(1).path("connection").isNull()); + final JsonNode human = list(fixture, SdkClients.adminClient()).path("data").get(0); + assertEquals( + PasswordEntityMasker.PASSWORD_MASK, + human.path("connection").path("config").path("authType").path("password").asText()); + } + + private static void assertSingleEntityStillFails(ServicePage fixture) { + final JsonNode broken = fixture.services().get(1); + for (String suffix : + List.of(broken.path("id").asText(), "name/" + broken.path("fullyQualifiedName").asText())) { + final OpenMetadataException failure = + assertThrows( + OpenMetadataException.class, + () -> + SdkClients.adminClient() + .getHttpClient() + .executeForString( + HttpMethod.GET, fixture.serviceCase().path() + "/" + suffix, null)); + assertEquals(400, failure.getStatusCode()); + } + } + + private static void assertCursorTraversal(ServicePage fixture) { + final OpenMetadataClient client = SdkClients.adminClient(); + final JsonNode first = list(fixture, client, options(fixture).queryParam("limit", "1").build()); + final JsonNode middle = + list( + fixture, + client, + options(fixture) + .queryParam("limit", "1") + .queryParam("after", first.path("paging").path("after").asText()) + .build()); + final JsonNode last = + list( + fixture, + client, + options(fixture) + .queryParam("limit", "1") + .queryParam("after", middle.path("paging").path("after").asText()) + .build()); + assertEquals(fixture.services().get(0).path("id"), first.path("data").get(0).path("id")); + assertEquals(fixture.services().get(1).path("id"), middle.path("data").get(0).path("id")); + assertEquals(fixture.services().get(2).path("id"), last.path("data").get(0).path("id")); + assertEquals(3, middle.path("paging").path("total").asInt()); + final JsonNode previous = + list( + fixture, + client, + options(fixture) + .queryParam("limit", "1") + .queryParam("before", middle.path("paging").path("before").asText()) + .build()); + assertEquals(first.path("data"), previous.path("data")); + } + + private static StoredConnection snapshot(ServicePage fixture) { + final JsonNode broken = fixture.services().get(1); + final String entityType = fixture.serviceCase().entityType(); + final UUID id = UUID.fromString(broken.path("id").asText()); + final String json = storedJson(entityType, id); + assertNotNull(JsonUtils.readTree(json).path("connection").get("config")); + return new StoredConnection(entityType, id, broken.path("fullyQualifiedName").asText(), json); + } + + private static String storedJson(String entityType, UUID id) { + final EntityDAO dao = Entity.getEntityRepository(entityType).getDao(); + return dao.findById(dao.getTableName(), id, dao.getCondition(Include.NON_DELETED)); + } + + private record ServiceCase( + String entityType, String collection, String serviceType, String config) { + private String path() { + return "/v1/services/" + collection; + } + + @Override + public String toString() { + return entityType; + } + } + + private record ServicePage( + ServiceCase serviceCase, String domain, List services, String namePrefix) {} + + private static final class StoredConnection implements AutoCloseable { + private final String entityType; + private final UUID id; + private final String fqn; + private final String original; + private JsonNode corrupted; + + private StoredConnection(String entityType, UUID id, String fqn, String original) { + this.entityType = entityType; + this.id = id; + this.fqn = fqn; + this.original = original; + } + + private void changeConfig(Consumer change) { + changeDocument( + document -> change.accept((ObjectNode) document.path("connection").path("config"))); + } + + private void changeDocument(Consumer change) { + final ObjectNode document = (ObjectNode) JsonUtils.readTree(original); + change.accept(document); + write(document.toString()); + corrupted = document; + } + + private void assertUnchangedAfterReads() { + final String stored = storedJson(entityType, id); + assertEquals(corrupted, JsonUtils.readTree(stored)); + } + + private void write(String json) { + Entity.getEntityRepository(entityType).getDao().update(id, fqn, json); + EntityRepository.invalidateCacheForEntity(entityType, id, fqn); + } + + @Override + public void close() { + write(original); + } + } +} diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServicesOverviewResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServicesOverviewResourceIT.java index 2431078c676d..11b99d5a7976 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServicesOverviewResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServicesOverviewResourceIT.java @@ -29,6 +29,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.openmetadata.it.factories.UserTestFactory; import org.openmetadata.it.util.SdkClients; import org.openmetadata.it.util.TestNamespace; import org.openmetadata.it.util.TestNamespaceExtension; @@ -516,6 +517,7 @@ void anUnresolvableDomainIsRejectedRatherThanIgnored(TestNamespace ns) throws Ex void aNonAdminSeesTheServiceTypesTheyArePermitted(TestNamespace ns) throws Exception { postgres(ns, "authz-postgres"); + UserTestFactory.getDataConsumer(null); JsonNode asConsumer = overviewAs(SdkClients.dataConsumerClient(), "limit=500&excludeProvider=system"); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java index 9efbec6e4d9f..bbecb4dc33d5 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java @@ -13,12 +13,12 @@ package org.openmetadata.service.resources.services; -import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty; import jakarta.ws.rs.core.SecurityContext; import jakarta.ws.rs.core.UriInfo; import lombok.Getter; +import lombok.extern.slf4j.Slf4j; import org.openmetadata.schema.ServiceConnectionEntityInterface; import org.openmetadata.schema.ServiceEntityInterface; import org.openmetadata.schema.entity.services.ServiceType; @@ -37,6 +37,7 @@ import org.openmetadata.service.secrets.masker.EntityMaskerFactory; import org.openmetadata.service.security.Authorizer; +@Slf4j public abstract class ServiceEntityResource< T extends ServiceEntityInterface, R extends ServiceEntityRepository, @@ -81,10 +82,27 @@ private Object retrieveServiceConnectionConfig(T service, boolean maskPassword) protected ResultList decryptOrNullify( SecurityContext securityContext, ResultList services) { - listOrEmpty(services.getData()).forEach(service -> decryptOrNullify(securityContext, service)); + if (services.getData() != null) { + services.setData( + services.getData().stream() + .map(service -> decryptServiceForList(securityContext, service)) + .toList()); + } return services; } + private T decryptServiceForList(SecurityContext securityContext, T service) { + try { + return decryptOrNullify(securityContext, service); + } catch (InvalidServiceConnectionException failure) { + LOG.warn( + "Failed to decrypt connection for {} service {}; returning it without a connection", + serviceType, + service.getId()); + return nullifyConnection(service); + } + } + protected T unmask(T service) { // TODO move this functionality to repository repository.setFullyQualifiedName(service); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ServiceEntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ServiceEntityResourceTest.java new file mode 100644 index 000000000000..a764727794be --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/ServiceEntityResourceTest.java @@ -0,0 +1,195 @@ +package org.openmetadata.service.resources.services; + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import jakarta.ws.rs.core.SecurityContext; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.function.UnaryOperator; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.MockedStatic; +import org.openmetadata.schema.api.services.CreateDatabaseService.DatabaseServiceType; +import org.openmetadata.schema.api.services.DatabaseConnection; +import org.openmetadata.schema.entity.services.DatabaseService; +import org.openmetadata.schema.entity.services.ServiceType; +import org.openmetadata.schema.system.EntityError; +import org.openmetadata.schema.type.ResourcePermission; +import org.openmetadata.schema.utils.JsonUtils; +import org.openmetadata.schema.utils.ResultList; +import org.openmetadata.service.Entity; +import org.openmetadata.service.exception.EntityMaskException; +import org.openmetadata.service.exception.InvalidServiceConnectionException; +import org.openmetadata.service.jdbi3.DatabaseServiceRepository; +import org.openmetadata.service.security.AuthorizationException; +import org.openmetadata.service.security.NoopAuthorizer; +import org.slf4j.LoggerFactory; + +class ServiceEntityResourceTest { + private static final String SECRET_SENTINEL = "synthetic-secret-must-not-be-logged"; + + @Test + void listFailurePreservesEnvelopeAndReturnedEntities() { + final TestResource resource = createResource(); + final DatabaseService healthy = service("healthy"); + final DatabaseService broken = service("broken"); + final DatabaseService later = service("later"); + resource.processor = + service -> { + if (service.getId().equals(broken.getId())) { + throw new InvalidServiceConnectionException(SECRET_SENTINEL); + } + return JsonUtils.deepCopy(service, DatabaseService.class).withDisplayName("processed"); + }; + final ResultList page = + new ResultList<>(List.of(healthy, broken, later), "before", "after", 9); + final List errors = List.of(new EntityError()); + final List warnings = List.of(new EntityError()); + final Map permissions = + Map.of(broken.getId().toString(), new ResourcePermission()); + page.setErrors(errors); + page.setWarnings(warnings); + page.setWarningsCount(1); + page.setEntityPermissions(permissions); + final String paging = JsonUtils.pojoToJson(page.getPaging()); + + assertSame(page, resource.decryptOrNullify(null, page)); + assertEquals( + List.of(healthy.getId(), broken.getId(), later.getId()), + page.getData().stream().map(DatabaseService::getId).toList()); + assertEquals("processed", page.getData().getFirst().getDisplayName()); + assertNull(page.getData().get(1).getConnection()); + assertEquals("processed", page.getData().getLast().getDisplayName()); + assertNotNull(broken.getConnection()); + assertEquals(paging, JsonUtils.pojoToJson(page.getPaging())); + assertSame(errors, page.getErrors()); + assertSame(warnings, page.getWarnings()); + assertEquals(1, page.getWarningsCount()); + assertSame(permissions, page.getEntityPermissions()); + } + + @Test + void warningDoesNotExposeExceptionTextOrThrowable() { + final TestResource resource = createResource(); + resource.processor = + service -> { + throw new InvalidServiceConnectionException(SECRET_SENTINEL); + }; + final DatabaseService broken = service("broken"); + final Logger logger = (Logger) LoggerFactory.getLogger(ServiceEntityResource.class); + final ListAppender events = new ListAppender<>(); + events.start(); + logger.addAppender(events); + try { + resource.decryptOrNullify(null, new ResultList<>(List.of(broken))); + assertEquals(1, events.list.size()); + final ILoggingEvent warning = events.list.getFirst(); + assertTrue(warning.getFormattedMessage().contains(broken.getId().toString())); + assertFalse(warning.getFormattedMessage().contains(SECRET_SENTINEL)); + assertNull(warning.getThrowableProxy()); + } finally { + logger.detachAppender(events); + events.stop(); + } + } + + @ParameterizedTest + @MethodSource("unrelatedFailures") + void unrelatedFailuresStillPropagate(RuntimeException failure) { + final TestResource resource = createResource(); + resource.processor = + service -> { + throw failure; + }; + assertSame( + failure, + assertThrows( + failure.getClass(), + () -> resource.decryptOrNullify(null, new ResultList<>(List.of(service("service")))))); + } + + private static Stream unrelatedFailures() { + return Stream.of( + new AuthorizationException("denied"), + new EntityMaskException("mask failed"), + new IllegalStateException("unexpected failure")); + } + + @Test + void emptyAndNullDataRetainTheirContract() { + final TestResource resource = createResource(); + final ResultList empty = new ResultList<>(List.of()); + final ResultList absent = new ResultList<>(); + assertSame(empty, resource.decryptOrNullify(null, empty)); + assertTrue(empty.getData().isEmpty()); + assertSame(absent, resource.decryptOrNullify(null, absent)); + assertNull(absent.getData()); + } + + private static DatabaseService service(String name) { + return new DatabaseService() + .withId(UUID.randomUUID()) + .withName(name) + .withFullyQualifiedName(name) + .withServiceType(DatabaseServiceType.Postgres) + .withConnection(new DatabaseConnection()); + } + + private static TestResource createResource() { + final DatabaseServiceRepository repository = mock(DatabaseServiceRepository.class); + when(repository.getAllowedFields()).thenReturn(Set.of()); + try (MockedStatic entities = mockStatic(Entity.class)) { + entities + .when(() -> Entity.getEntityRepository(Entity.DATABASE_SERVICE)) + .thenReturn(repository); + entities + .when(() -> Entity.getEntityClassFromType(Entity.DATABASE_SERVICE)) + .thenReturn(DatabaseService.class); + entities + .when(() -> Entity.getServiceEntityRepository(ServiceType.DATABASE)) + .thenReturn(repository); + return new TestResource(); + } + } + + private static class TestResource + extends ServiceEntityResource< + DatabaseService, DatabaseServiceRepository, DatabaseConnection> { + private UnaryOperator processor = UnaryOperator.identity(); + + private TestResource() { + super(Entity.DATABASE_SERVICE, new NoopAuthorizer(), null, ServiceType.DATABASE); + } + + @Override + protected DatabaseService decryptOrNullify(SecurityContext context, DatabaseService service) { + return processor.apply(service); + } + + @Override + protected DatabaseService nullifyConnection(DatabaseService service) { + return JsonUtils.deepCopy(service, DatabaseService.class).withConnection(null); + } + + @Override + protected String extractServiceType(DatabaseService service) { + return service.getServiceType().value(); + } + } +} From a4f47a6117a209b343bb90bb9a189a1937c4cbd8 Mon Sep 17 00:00:00 2001 From: Vamsi-klu Date: Sat, 17 Oct 2026 21:26:44 +0000 Subject: [PATCH 2/3] docs: record service list connection fallback (#31887) --- ...-10-17-service-list-connection-fallback.md | 29 +++++++++++++++++++ .../services/ServiceEntityResource.java | 1 + 2 files changed, 30 insertions(+) create mode 100644 docs/decisions/2026-10-17-service-list-connection-fallback.md diff --git a/docs/decisions/2026-10-17-service-list-connection-fallback.md b/docs/decisions/2026-10-17-service-list-connection-fallback.md new file mode 100644 index 000000000000..ac89acf215fc --- /dev/null +++ b/docs/decisions/2026-10-17-service-list-connection-fallback.md @@ -0,0 +1,29 @@ +# Service lists isolate broken connections while single-service operations remain strict + +- **Status:** Accepted +- **Revisions:** v1 2026-10-17 (initial) +- **Deciders:** Vamsi-klu +- **Guard:** `ServiceEntityResourceTest` and `ServiceConnectionResilienceIT` +- **Related:** https://github.com/open-metadata/OpenMetadata/issues/31887, + https://github.com/open-metadata/OpenMetadata/pull/35681 + +## Context + +A stored connection that cannot be decrypted or validated must not hide healthy services in the +same list. The broken service must remain discoverable so its configuration can be repaired. + +## Decision + +Service list endpoints catch `InvalidServiceConnectionException` for each entry and return that +service with `connection` null. The response keeps its identities, order, pagination and +permissions. The fallback logs only the service category and UUID and does not persist the change. + +Single-service GET by ID or name and writes retain their existing strict error behavior, including +HTTP 400 for invalid connections. Other exceptions continue to fail the list. + +## Consequences + +One broken connection no longer prevents listing healthy services. A null connection in a list +does not distinguish an invalid stored connection from other reasons it may be absent; callers +must not treat it as proof that no connection was configured. Single-service reads retain the +error needed to diagnose the broken configuration, and repair requires an explicit write. diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java index bbecb4dc33d5..dcc29114b59f 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java @@ -91,6 +91,7 @@ protected ResultList decryptOrNullify( return services; } + // Keep broken services discoverable: ADR:2026-10-17-service-list-connection-fallback. private T decryptServiceForList(SecurityContext securityContext, T service) { try { return decryptOrNullify(securityContext, service); From 4ba5494d3e09743c1ebbcba8287b86a4a362c50c Mon Sep 17 00:00:00 2001 From: Vamsi-klu Date: Sat, 17 Oct 2026 22:57:04 +0000 Subject: [PATCH 3/3] Fixes 31887: Use nullOrEmpty for service list data --- .../service/resources/services/ServiceEntityResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java index dcc29114b59f..7afc85b4754e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java @@ -82,7 +82,7 @@ private Object retrieveServiceConnectionConfig(T service, boolean maskPassword) protected ResultList decryptOrNullify( SecurityContext securityContext, ResultList services) { - if (services.getData() != null) { + if (!nullOrEmpty(services.getData())) { services.setData( services.getData().stream() .map(service -> decryptServiceForList(securityContext, service))