From cd399f6a3a5a78d705a0144819685c019b19d560 Mon Sep 17 00:00:00 2001 From: shuchenliu Date: Fri, 21 Aug 2026 02:05:02 -0700 Subject: [PATCH 1/2] feat: add test-instance Elasticsearch connection presets The test cluster sits behind an ingress that routes on the Host header, so "test" carries the header explicitly alongside its resolved host rather than leaving it to the URL. Verified against the forwarded service: the request returns 200 with the header and 404 without it. "test_local_forward" pairs the same header with localhost:9200, mirroring ci_local_forward, since the direct host is not reachable outside the cluster. --- README.md | 7 +++++++ biothings_annotator/annotator/settings.py | 11 +++++++++++ tests/test_elasticsearch.py | 17 +++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/README.md b/README.md index 71c29d6..e71f7ab 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,10 @@ Set `ELASTICSEARCH_CONNECTION` to one of the named presets in `biothings_annotator/annotator/settings.py`. The `ci` preset points at `http://elasticsearch.es-core-components.svc.cluster.local:9200`. The `ci_local_forward` preset is for local port-forward use; `ci_forward` remains as a deprecated alias. +The `test` preset points at `http://core-components-es.test.transltr.io:9200` and sends an explicit +`Host: core-components-es.test.transltr.io` header, because the test cluster is reached through an +ingress that routes on that header. `test_local_forward` pairs the same header with +`http://localhost:9200` for port-forward use. The `/version` endpoint reports the active `query_backend` and, when Elasticsearch is active, the selected `elasticsearch_connection`. @@ -369,6 +373,9 @@ PUBMED_INTEGRATION_ELASTICSEARCH_CONNECTION=ci_local_forward \ python -m pytest -q tests/test_pubmed.py tests/test_document_metadata.py -m integration ``` +Point `PUBMED_INTEGRATION_ELASTICSEARCH_CONNECTION` at `test_local_forward` to run the same checks +against a forwarded test-instance service, or at `test` to reach it directly. + The document metadata live checks assert the index shape, resolution by every identifier type, case-insensitive matching, and an upper bound of three identifiers per record. That bound is a bad-export guard: pubmed2db PR #7 limits a record to its own identifiers, so a record carrying hundreds means the diff --git a/biothings_annotator/annotator/settings.py b/biothings_annotator/annotator/settings.py index 6abcb0a..4eea2b5 100644 --- a/biothings_annotator/annotator/settings.py +++ b/biothings_annotator/annotator/settings.py @@ -31,6 +31,17 @@ "ci_local_forward": CI_LOCAL_FORWARD_ELASTICSEARCH_CONNECTION, # Deprecated alias for compatibility with existing local-forward overrides. "ci_forward": CI_LOCAL_FORWARD_ELASTICSEARCH_CONNECTION, + # The test cluster is reached through an ingress that routes on the Host + # header, so the header is sent explicitly rather than left to the resolved + # host. "test_local_forward" pairs the same header with a port-forward. + "test": { + "host": "http://core-components-es.test.transltr.io:9200", + "headers": {"Host": "core-components-es.test.transltr.io"}, + }, + "test_local_forward": { + "host": "http://localhost:9200", + "headers": {"Host": "core-components-es.test.transltr.io"}, + }, } ELASTICSEARCH_REQUEST_TIMEOUT = 30 ELASTICSEARCH_QUERY_SIZE = 10 diff --git a/tests/test_elasticsearch.py b/tests/test_elasticsearch.py index b2f8342..b3cc40f 100644 --- a/tests/test_elasticsearch.py +++ b/tests/test_elasticsearch.py @@ -393,6 +393,18 @@ def test_elasticsearch_connection_config_supports_local_forwarded_ci_host(): assert get_elasticsearch_connection("ci_forward") == expected_connection +def test_elasticsearch_connection_config_supports_test_instance(): + assert get_elasticsearch_connection("test") == { + "host": "http://core-components-es.test.transltr.io:9200", + "headers": {"Host": "core-components-es.test.transltr.io"}, + } + assert get_elasticsearch_connection("test_local_forward") == { + "host": "http://localhost:9200", + "headers": {"Host": "core-components-es.test.transltr.io"}, + } + assert ELASTICSEARCH_CONNECTIONS["test"] is not ELASTICSEARCH_CONNECTIONS["test_local_forward"] + + def test_elasticsearch_client_uses_named_connection_config(): elasticsearch_settings = ANNOTATOR_CLIENTS["gene"]["elasticsearch"] original_instance = elasticsearch_settings.get("instance") @@ -408,6 +420,11 @@ def test_elasticsearch_client_uses_named_connection_config(): assert local_client is not ci_local_forward_client assert local_client.host == "http://localhost:9200" assert local_client.headers == {} + + test_client = get_elasticsearch_client("gene", "test") + assert test_client is not local_client + assert test_client.host == "http://core-components-es.test.transltr.io:9200" + assert test_client.headers == {"Host": "core-components-es.test.transltr.io"} finally: elasticsearch_settings["instance"] = original_instance From c55df2015cadef7015680a48102203930a1f0c34 Mon Sep 17 00:00:00 2001 From: shuchenliu Date: Tue, 25 Aug 2026 12:46:49 -0700 Subject: [PATCH 2/2] refactor: use in-cluster Elasticsearch preset --- README.md | 22 ++++++++++++---------- biothings_annotator/annotator/settings.py | 13 ++++++++----- deploy/values.yaml | 2 +- tests/test_application_endpoints.py | 8 +++++--- tests/test_document_metadata.py | 9 +++++++++ tests/test_elasticsearch.py | 18 ++++++++++++++---- 6 files changed, 49 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index e71f7ab..093edc6 100644 --- a/README.md +++ b/README.md @@ -171,17 +171,19 @@ tracing and targets `http://jaeger-otel-collector.sri:4318` by default. The annotator query backend is controlled with `ANNOTATOR_QUERY_BACKEND`. Supported values are `biothings` and `elasticsearch`; when unset, the service uses `biothings`. -The Helm/Jenkins deployment defaults set `ANNOTATOR_QUERY_BACKEND=elasticsearch` and -`ELASTICSEARCH_CONNECTION=ci`; set `ANNOTATOR_QUERY_BACKEND` to `biothings` during deployment +The checked-in Helm defaults set `ANNOTATOR_QUERY_BACKEND=elasticsearch` and +`ELASTICSEARCH_CONNECTION=in_cluster`; set `ANNOTATOR_QUERY_BACKEND` to `biothings` during deployment to switch back. Set `ELASTICSEARCH_CONNECTION` to one of the named presets in -`biothings_annotator/annotator/settings.py`. The `ci` preset points at -`http://elasticsearch.es-core-components.svc.cluster.local:9200`. The `ci_local_forward` preset is -for local port-forward use; `ci_forward` remains as a deprecated alias. -The `test` preset points at `http://core-components-es.test.transltr.io:9200` and sends an explicit -`Host: core-components-es.test.transltr.io` header, because the test cluster is reached through an -ingress that routes on that header. `test_local_forward` pairs the same header with -`http://localhost:9200` for port-forward use. +`biothings_annotator/annotator/settings.py`. The `in_cluster` preset points at +`http://elasticsearch.es-core-components.svc.cluster.local:9200`; `ci` remains as a compatibility +alias. The `ci_local_forward` preset is for CI port-forward use; `ci_forward` remains as its +deprecated alias. +The `test` preset is for external access through `http://core-components-es.test.transltr.io:9200` +and sends an explicit `Host: core-components-es.test.transltr.io` header because the ingress routes +on that header. `test_local_forward` pairs the same header with `http://localhost:9200` for +port-forward use. In-cluster CI and test deployments should use `in_cluster`, not either `test` +preset. The `/version` endpoint reports the active `query_backend` and, when Elasticsearch is active, the selected `elasticsearch_connection`. @@ -374,7 +376,7 @@ python -m pytest -q tests/test_pubmed.py tests/test_document_metadata.py -m inte ``` Point `PUBMED_INTEGRATION_ELASTICSEARCH_CONNECTION` at `test_local_forward` to run the same checks -against a forwarded test-instance service, or at `test` to reach it directly. +against a forwarded test-instance service, or at `test` to reach the test ingress directly. The document metadata live checks assert the index shape, resolution by every identifier type, case-insensitive matching, and an upper bound of three identifiers per record. That bound is a bad-export diff --git a/biothings_annotator/annotator/settings.py b/biothings_annotator/annotator/settings.py index 4eea2b5..5c01251 100644 --- a/biothings_annotator/annotator/settings.py +++ b/biothings_annotator/annotator/settings.py @@ -14,7 +14,11 @@ BIOTHINGS_SOURCE_DISCOVERY_TTL = 60 BIOTHINGS_SOURCE_DISCOVERY_ERROR_TTL = 5 -ELASTICSEARCH_CONNECTION = "ci" +ELASTICSEARCH_CONNECTION = "in_cluster" +IN_CLUSTER_ELASTICSEARCH_CONNECTION = { + "host": "http://elasticsearch.es-core-components.svc.cluster.local:9200", + "headers": {}, +} CI_LOCAL_FORWARD_ELASTICSEARCH_CONNECTION = { "host": "http://localhost:9200", "headers": {"Host": "core-components-es.ci.transltr.io"}, @@ -24,10 +28,9 @@ "host": "http://localhost:9200", "headers": {}, }, - "ci": { - "host": "http://elasticsearch.es-core-components.svc.cluster.local:9200", - "headers": {}, - }, + "in_cluster": IN_CLUSTER_ELASTICSEARCH_CONNECTION, + # Deprecated alias for deployments configured before the cluster-neutral rename. + "ci": IN_CLUSTER_ELASTICSEARCH_CONNECTION, "ci_local_forward": CI_LOCAL_FORWARD_ELASTICSEARCH_CONNECTION, # Deprecated alias for compatibility with existing local-forward overrides. "ci_forward": CI_LOCAL_FORWARD_ELASTICSEARCH_CONNECTION, diff --git a/deploy/values.yaml b/deploy/values.yaml index 8be07d1..1814a81 100644 --- a/deploy/values.yaml +++ b/deploy/values.yaml @@ -23,7 +23,7 @@ containers: name: biothingsannotator es_host: ES_HOST_VALUE query_backend: elasticsearch - elasticsearch_connection: ci + elasticsearch_connection: in_cluster port: 9000 OPENTELEMETRY_ENABLED_VALUE: True diff --git a/tests/test_application_endpoints.py b/tests/test_application_endpoints.py index 2b3af4c..435414a 100644 --- a/tests/test_application_endpoints.py +++ b/tests/test_application_endpoints.py @@ -173,12 +173,14 @@ async def test_version_get_success(test_annotator: sanic.Sanic, endpoint: str, m @pytest.mark.unit @pytest.mark.asyncio(loop_scope="module") @pytest.mark.parametrize("endpoint", ["/version/"]) -async def test_version_get_reports_elasticsearch_backend(test_annotator: sanic.Sanic, endpoint: str, monkeypatch): +async def test_version_get_reports_default_elasticsearch_connection( + test_annotator: sanic.Sanic, endpoint: str, monkeypatch +): """ Test the Version endpoint GET method includes runtime backend metadata. """ monkeypatch.setenv(QUERY_BACKEND_ENV, "elasticsearch") - monkeypatch.setenv("ELASTICSEARCH_CONNECTION", "ci") + monkeypatch.delenv("ELASTICSEARCH_CONNECTION", raising=False) with patch.object(VersionView, "open_version_file", return_value="GITHUB_HASH_VERSION_ABC123") as mock_file_read: request, response = await test_annotator.asgi_client.request(method="get", url=endpoint) @@ -194,7 +196,7 @@ async def test_version_get_reports_elasticsearch_backend(test_annotator: sanic.S expected_response_body = { "version": "GITHUB_HASH_VERSION_ABC123", "query_backend": "elasticsearch", - "elasticsearch_connection": "ci", + "elasticsearch_connection": "in_cluster", } assert response.http_version == "HTTP/1.1" assert response.content_type == "application/json" diff --git a/tests/test_document_metadata.py b/tests/test_document_metadata.py index 2aed1cd..a8702a5 100644 --- a/tests/test_document_metadata.py +++ b/tests/test_document_metadata.py @@ -45,6 +45,15 @@ } +@pytest.mark.unit +def test_document_metadata_service_uses_default_in_cluster_connection(monkeypatch): + monkeypatch.delenv("ELASTICSEARCH_CONNECTION", raising=False) + + service = DocumentMetadataService() + + assert service.elasticsearch_connection == "in_cluster" + + @pytest.mark.unit def test_format_publication_metadata_uses_empty_strings_and_date_precision(): assert format_publication_metadata(PUBMED_METADATA) == FORMATTED_METADATA diff --git a/tests/test_elasticsearch.py b/tests/test_elasticsearch.py index b3cc40f..e07ede1 100644 --- a/tests/test_elasticsearch.py +++ b/tests/test_elasticsearch.py @@ -27,11 +27,11 @@ def test_annotator_can_switch_query_backend_by_assignment(monkeypatch): annotator = Annotator() assert annotator.query_backend == "biothings" - assert annotator.elasticsearch_connection == "ci" + assert annotator.elasticsearch_connection == "in_cluster" annotator.query_backend = "elasticsearch" assert annotator.query_backend == "elasticsearch" - assert annotator.elasticsearch_connection == "ci" + assert annotator.elasticsearch_connection == "in_cluster" annotator.elasticsearch_connection = "local" assert annotator.elasticsearch_connection == "local" @@ -378,12 +378,17 @@ async def handler(request: httpx.Request) -> httpx.Response: assert result == [{"query": "1017", "notfound": True}] -def test_elasticsearch_connection_config_supports_local_forwarded_ci_host(): - assert get_elasticsearch_connection("ci") == { +def test_elasticsearch_connection_config_supports_in_cluster_with_ci_alias(): + expected_connection = { "host": "http://elasticsearch.es-core-components.svc.cluster.local:9200", "headers": {}, } + assert ELASTICSEARCH_CONNECTIONS["ci"] is ELASTICSEARCH_CONNECTIONS["in_cluster"] + assert get_elasticsearch_connection("in_cluster") == expected_connection + assert get_elasticsearch_connection("ci") == expected_connection + +def test_elasticsearch_connection_config_supports_local_forwarded_ci_host(): expected_connection = { "host": "http://localhost:9200", "headers": {"Host": "core-components-es.ci.transltr.io"}, @@ -411,6 +416,11 @@ def test_elasticsearch_client_uses_named_connection_config(): try: elasticsearch_settings["instance"] = None + in_cluster_client = get_elasticsearch_client("gene", "in_cluster") + assert in_cluster_client.host == "http://elasticsearch.es-core-components.svc.cluster.local:9200" + assert in_cluster_client.headers == {} + assert get_elasticsearch_client("gene", "ci") is in_cluster_client + ci_local_forward_client = get_elasticsearch_client("gene", "ci_local_forward") assert ci_local_forward_client.host == "http://localhost:9200" assert ci_local_forward_client.headers == {"Host": "core-components-es.ci.transltr.io"}