Make Agentless Connectors task handle connector.deleted properly by artem-shelkovnikov · Pull Request #206606 · elastic/kibana

artem-shelkovnikov · 2025-01-14T15:32:01Z

Summary

This PR makes it so that the Agentless Kibana task implemented in #203973 properly handles soft-deleted connectors.

This helps with the situation when an integration policy has been created for an agentless connector but a connector record has not yet been created by an agentless host.

With current Kibana task implementation it could lead to the Policy being deleted.

With this change, only policies that refer to soft-deleted connectors will be cleaned up.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

…-fix'

jedrazb

Overall LGTM, I left a few comments that IMO are blocking (connector type def and handling nulls) and some nits

src/platform/packages/shared/kbn-search-connectors/types/connectors.ts

x-pack/solutions/search/plugins/search_connectors/server/services/index.ts

jedrazb · 2025-01-16T09:20:05Z

x-pack/solutions/search/plugins/search_connectors/server/services/index.ts

  return connectors.filter(
-    (x) => packagePolicies.filter((y) => y.connector_metadata.id === x.id).length === 0
+    (x) =>
+      packagePolicies.filter((y) => y.connector_settings.id === x.id).length === 0 &&


nit: I once got linked this on my PR Cognitive load is what matters: complex conditionals

I'm struggling to parse this boolean expression in my head, maybe we can split it up:

const statementIWithClearName1 = ... const statementIWithClearName2 = ... return statementIWithClearName1 && statementIWithClearName2

maybe more elegant with every lib function:

Suggested change

packagePolicies.filter((y) => y.connector_settings.id === x.id).length === 0 &&

packagePolicies.every((policy) => policy.connector_settings.id !== x.id)

Lemme see how I can make it simpler

jedrazb · 2025-01-16T09:20:49Z

x-pack/solutions/search/plugins/search_connectors/server/services/index.ts

  return packagePolicies.filter(
-    (x) => connectors.filter((y) => y.id === x.connector_metadata.id).length === 0
+    (x) =>
+      connectors.filter((y) => y.id === x.connector_settings.id && y.is_deleted === true).length > 0


nit: same, maybe let's split it up into separate boolean statements and join with && in return

instead of filter we can use some js lib function

jedrazb · 2025-01-16T09:24:15Z

x-pack/solutions/search/plugins/search_connectors/server/services/index.ts

          id: connector.id,
          name: connector.name,
          service_type: connector.service_type,
+          is_deleted: connector.deleted,


I would use !!connector.deleted or Boolean(connector.deleted)

This handles cases where deleted might be null (not present in connector doc) for older connectors by explicitly casting it to a boolean. Otherwise, you would need to account for this in your logic later on. JS is fun ...

Makes sense :)

…soft-deletion

jedrazb

LGTM! Nice one!

elasticmachine · 2025-01-16T11:37:46Z

💚 Build Succeeded

Buildkite Build
Commit: a27ce37

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/search-connectors`	3954	3956	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`enterpriseSearch`	1.6MB	1.6MB	+20.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/search-connectors`	3954	3956	+2

History

💔 Build #267305 failed 8b5444c
💔 Build #267257 failed 2d4cf9f
💔 Build #267247 failed f6b5c12
💔 Build #267151 failed 6d5695c
💔 Build #267039 failed 229883d
💔 Build #267022 failed 20ca974

…stic#206606) ## Summary This PR makes it so that the Agentless Kibana task implemented in elastic#203973 properly handles soft-deleted connectors. This helps with the situation when an integration policy has been created for an agentless connector but a connector record has not yet been created by an agentless host. With current Kibana task implementation it could lead to the Policy being deleted. With this change, only policies that refer to soft-deleted connectors will be cleaned up. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

Make Agentless Connectors task handle connector.deleted properly

20ca974

artem-shelkovnikov requested a review from a team as a code owner January 14, 2025 15:32

artem-shelkovnikov added backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes labels Jan 14, 2025

kibanamachine and others added 6 commits January 14, 2025 15:56

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

229883d

…-fix'

Default deleted to false

6d5695c

Fix create_connector_document

f6b5c12

Fix more places

2d4cf9f

Fix a couple tests

8b5444c

Fix more tests

4fd7c11

jedrazb reviewed Jan 16, 2025

View reviewed changes

artem-shelkovnikov added 2 commits January 16, 2025 13:49

Address comments

e754081

Merge branch 'main' into artem/make-agentless-connectors-task-handle-…

a27ce37

…soft-deletion

jedrazb approved these changes Jan 16, 2025

View reviewed changes

artem-shelkovnikov merged commit f2a7d90 into main Jan 16, 2025

artem-shelkovnikov deleted the artem/make-agentless-connectors-task-handle-soft-deletion branch January 16, 2025 11:54

kibanamachine added the v9.0.0 label Jan 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Agentless Connectors task handle connector.deleted properly#206606

Make Agentless Connectors task handle connector.deleted properly#206606
artem-shelkovnikov merged 9 commits intomainfrom
artem/make-agentless-connectors-task-handle-soft-deletion

artem-shelkovnikov commented Jan 14, 2025 •

edited

Loading

Uh oh!

jedrazb left a comment

Uh oh!

Uh oh!

Uh oh!

jedrazb Jan 16, 2025

Uh oh!

jedrazb Jan 16, 2025

Uh oh!

artem-shelkovnikov Jan 16, 2025

Uh oh!

jedrazb Jan 16, 2025

Uh oh!

jedrazb Jan 16, 2025

Uh oh!

jedrazb Jan 16, 2025 •

edited

Loading

Uh oh!

artem-shelkovnikov Jan 16, 2025

Uh oh!

jedrazb left a comment

Uh oh!

elasticmachine commented Jan 16, 2025

API count

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	packagePolicies.filter((y) => y.connector_settings.id === x.id).length === 0 &&
	packagePolicies.every((policy) => policy.connector_settings.id !== x.id)

Conversation

artem-shelkovnikov commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

jedrazb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jedrazb Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jedrazb Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

artem-shelkovnikov Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jedrazb Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jedrazb Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jedrazb Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artem-shelkovnikov Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jedrazb left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jan 16, 2025

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

API count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

artem-shelkovnikov commented Jan 14, 2025 •

edited

Loading

jedrazb Jan 16, 2025 •

edited

Loading