[Connector APIs] Enforce index prefix for managed connectors #117778

jedrazb · 2024-11-29T17:34:31Z

Changes

For 9.x, Connectors will be deployed in an agentless environment. This means the Fleet service account will now be responsible for generating API keys, enabling the connector to read/write from connector state indices and interact with the content index (where data is synced by connector from the 3rd party data source).

To maintain granular permissions and avoid granting the Fleet service account access to all indices (e.g., *), it has been decided to restrict content index names to those prefixed with content-*.

This PR introduces restrictions and validations to ensure that Elastic-managed connectors (with isNative: true flag) can only attach to indices with the content- prefix. We validate index name on:

connector creation
index name update (e.g. native connector cannot have index name changed with illegal prefix)
isNative property update (we need to first change the attached index to have content- then update the connector to native connector)

Additionally, the ConnectorIndexServiceTests have been unmuted. These tests have been stable for about a year. A few flaky runs (IMO not related to our test logic) had caused them to be muted earlier, but they now run successfully. We need to keep them active.

BCC approval

Relates to [Search Connectors] introduce "content-*" prefix requirement for Elastic-managed connectors dev issue that got green light from bcc.

Verification

Verified API calls manually.
YAML end-to-end tests.
unit tests

jedrazb · 2024-11-29T17:48:11Z

...earch/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java

+                ConnectorStatus status = getConnectorStatusFromSearchResult(connector);
+
+                // If connector was connected already, change its status to CONFIGURED as we need to re-connect
+                if (status == ConnectorStatus.CONNECTED) {


I realised we would always set status CONFIGURED once changing the is_native flag - it's likely wrong, we got here because I copied logic from Kibana initially https://github.com/elastic/kibana/pull/175536/files

nice catch.

jedrazb · 2024-12-02T08:43:52Z

...rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml

-  - match: { results.0.id: "connector-a" }
-  - match: { results.0.index_name: "search-1-test" }
-  - match: { results.0.language: "pl" }
+  - match: { results.0.id: "connector-b" }


content- index is first alphabetically so order of list API results changes (we sort by index_name by default)

elasticsearchmachine · 2024-12-02T11:12:09Z

Pinging @elastic/search-eng (Team:SearchOrg)

elasticsearchmachine · 2024-12-02T11:12:10Z

Pinging @elastic/search-extract-and-transform (Team:Search - Extract & Transform)

jedrazb · 2024-12-02T13:05:11Z

Rest compatibility failing because it's not a backward compatible change

seanstory · 2024-12-10T16:46:15Z

the relevant backwards compatibility change proposal is here: https://github.com/elastic/dev/issues/2932

jedrazb · 2024-12-11T08:38:31Z

This is a workaround for failing bcc changes #118260 (should be merged and backported first)

jedrazb · 2024-12-13T12:52:32Z

@elasticmachine merge upstream

…me-checks

jedrazb · 2024-12-16T08:50:02Z

@elasticmachine merge upstream

…me-checks

elasticsearchmachine · 2024-12-16T08:53:28Z

Hi @jedrazb, I've created a changelog YAML for you.

…of github.com:jedrazb/elasticsearch into connector-apis-add-native-connector-index-name-checks

jedrazb · 2024-12-16T11:17:25Z

muted-tests.yml

@@ -91,8 +91,6 @@ tests:
 - class: org.elasticsearch.search.StressSearchServiceReaperIT
  method: testStressReaper
  issue: https://github.com/elastic/elasticsearch/issues/115816
- class: org.elasticsearch.xpack.application.connector.ConnectorIndexServiceTests


The ConnectorIndexServiceTests has been running fine for almost a year, I saw seemingly unrelated issue in the failed buildkite run that led to muting this

kderusso

Looks reasonable to me. Are there any BWC cases that we should be aware of/test? Also are there any docs updates required?

...c/main/java/org/elasticsearch/xpack/application/connector/action/ConnectorActionRequest.java

timgrein · 2024-12-16T16:01:56Z

...earch/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java

+                // Ensure attached content index is prefixed correctly
+                if (isNative && indexName != null && isValidManagedConnectorIndexName(indexName) == false) {


Suggested change

// Ensure attached content index is prefixed correctly

if (isNative && indexName != null && isValidManagedConnectorIndexName(indexName) == false) {

// Ensure attached content index is prefixed correctly

boolean doesNotHaveContentPrefix = indexName != null && isValidManagedConnectorIndexName(indexName) == false

if (isNative && doesNotHaveContentPrefix) {

Rationale: Cognitive load is what matters: complex conditionals

Good read! Thank you for linking

timgrein · 2024-12-16T16:02:51Z

...earch/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java

+
+                // If connector was connected already, change its status to CONFIGURED as we need to re-connect
+                if (status == ConnectorStatus.CONNECTED) {
+                    status = ConnectorStatus.CONFIGURED;


Should this transition be guarded/validated through ConnectorStateMachine?

[Connector APIs] Enforce manage connector index prefix

e640ce3

jedrazb added >non-issue :SearchOrg/Extract&Transform Label for the Search E&T team Team:Search - Extract & Transform v9.0.0 labels Nov 29, 2024

jedrazb commented Nov 29, 2024

View reviewed changes

Update ConnectorIndexServiceTests to check for prefix

57d083f

jedrazb commented Dec 2, 2024

View reviewed changes

jedrazb changed the title ~~[Connector APIs] Enforce manage connector index prefix~~ [Connector APIs] Enforce index prefix for managed connectors Dec 2, 2024

jedrazb requested review from seanstory, artem-shelkovnikov and navarone-feekery December 2, 2024 09:08

jedrazb marked this pull request as ready for review December 2, 2024 11:11

elasticsearchmachine added the Team:SearchOrg Meta label for the Search Org (Enterprise Search) label Dec 2, 2024

jedrazb mentioned this pull request Dec 9, 2024

[Connector APIs] Update yaml rest tests for Connector APIs #118260

Merged

Merge branch 'main' into connector-apis-add-native-connector-index-na…

c807547

…me-checks

Merge branch 'main' into connector-apis-add-native-connector-index-na…

dea0c05

…me-checks

jedrazb added >feature and removed >non-issue labels Dec 16, 2024

jedrazb added 3 commits December 16, 2024 09:53

Update docs/changelog/117778.yaml

0aa8791

Fix accidental changes to muted tests

8abb14a

Merge branch 'connector-apis-add-native-connector-index-name-checks' …

c2f1818

…of github.com:jedrazb/elasticsearch into connector-apis-add-native-connector-index-name-checks

jedrazb commented Dec 16, 2024

View reviewed changes

kderusso reviewed Dec 16, 2024

View reviewed changes

...c/main/java/org/elasticsearch/xpack/application/connector/action/ConnectorActionRequest.java Show resolved Hide resolved

...c/main/java/org/elasticsearch/xpack/application/connector/action/ConnectorActionRequest.java Show resolved Hide resolved

timgrein reviewed Dec 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Connector APIs] Enforce index prefix for managed connectors #117778

[Connector APIs] Enforce index prefix for managed connectors #117778

jedrazb commented Nov 29, 2024 •

edited

Loading

jedrazb Nov 29, 2024 •

edited

Loading

seanstory Dec 10, 2024

jedrazb Dec 2, 2024 •

edited

Loading

elasticsearchmachine commented Dec 2, 2024

elasticsearchmachine commented Dec 2, 2024

jedrazb commented Dec 2, 2024

seanstory commented Dec 10, 2024

jedrazb commented Dec 11, 2024

jedrazb commented Dec 13, 2024

jedrazb commented Dec 16, 2024

elasticsearchmachine commented Dec 16, 2024

jedrazb Dec 16, 2024

kderusso left a comment

timgrein Dec 16, 2024

jedrazb Dec 17, 2024

timgrein Dec 16, 2024

		// Ensure attached content index is prefixed correctly
		if (isNative && indexName != null && isValidManagedConnectorIndexName(indexName) == false) {

[Connector APIs] Enforce index prefix for managed connectors #117778

Are you sure you want to change the base?

[Connector APIs] Enforce index prefix for managed connectors #117778

Conversation

jedrazb commented Nov 29, 2024 • edited Loading

Changes

BCC approval

Verification

jedrazb Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

seanstory Dec 10, 2024

Choose a reason for hiding this comment

jedrazb Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

elasticsearchmachine commented Dec 2, 2024

elasticsearchmachine commented Dec 2, 2024

jedrazb commented Dec 2, 2024

seanstory commented Dec 10, 2024

jedrazb commented Dec 11, 2024

jedrazb commented Dec 13, 2024

jedrazb commented Dec 16, 2024

elasticsearchmachine commented Dec 16, 2024

jedrazb Dec 16, 2024

Choose a reason for hiding this comment

kderusso left a comment

Choose a reason for hiding this comment

timgrein Dec 16, 2024

Choose a reason for hiding this comment

jedrazb Dec 17, 2024

Choose a reason for hiding this comment

timgrein Dec 16, 2024

Choose a reason for hiding this comment

jedrazb commented Nov 29, 2024 •

edited

Loading

jedrazb Nov 29, 2024 •

edited

Loading

jedrazb Dec 2, 2024 •

edited

Loading