Skip to content

Conversation

@natasha-moore-elastic
Copy link
Contributor

@natasha-moore-elastic natasha-moore-elastic commented Jun 11, 2025

This PR replaces the old Security API doc links with API reference links in the Kibana doc link service.

This security-docs PR, which removes outdated Security Detections asciidoc API docs, is failing with the following error:

INFO:build_docs:Bad cross-document links:
INFO:build_docs: Kibana [8.x]: src/platform/packages/shared/kbn-doc-links/src/get_doc_links.ts contains broken links to:
INFO:build_docs: - en/security/8.x/rule-api-overview.html
INFO:build_docs: - en/security/8.x/signals-migration-api.htm

Replacing these links in the Kibana doc link service should unblock the docs PR.

The links were replaced in main in this PR.

@natasha-moore-elastic natasha-moore-elastic self-assigned this Jun 11, 2025
@natasha-moore-elastic natasha-moore-elastic added Team:Docs release_note:skip Skip the PR/issue when compiling release notes v8.19.0 labels Jun 11, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-docs (Team:Docs)

@natasha-moore-elastic natasha-moore-elastic added the backport:skip This PR does not require backporting label Jun 11, 2025
@lcawl lcawl enabled auto-merge (squash) June 11, 2025 15:53
@lcawl
Copy link
Contributor

lcawl commented Jun 11, 2025

I've added a commit because it seems like there are some hard-coded occurrences of the old URLs which is something we hit in #217722 too. I tried to switch from "be" to "includes" so we could omit that hard-coded URL part of the string from the test like in #219005 or #219656 but it continued to fail until I copied in the full new URL. Ideally this would be fixed after discussing with appropriate dev team.

@lcawl lcawl disabled auto-merge June 11, 2025 22:50
@natasha-moore-elastic natasha-moore-elastic requested review from a team and jkelas June 12, 2025 16:20
@jkelas
Copy link
Contributor

jkelas commented Jun 13, 2025

I've added a commit because it seems like there are some hard-coded occurrences of the old URLs which is something we hit in #217722 too. I tried to switch from "be" to "includes" so we could omit that hard-coded URL part of the string from the test like in #219005 or #219656 but it continued to fail until I copied in the full new URL. Ideally this would be fixed after discussing with appropriate dev team.

Hi @lcawl
I tested that both below versions work the same, just fine.

expect(
          header.warning?.includes(
            '299 Kibana "Deprecated endpoint: /api/detection_engine/rules/_bulk_update API is deprecated since v8.2. Please use the /api/detection_engine/rules/_bulk_action API instead. See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail."'
          )
        ).to.be(true);


        expect(header.warning).to.be(
          '299 Kibana "Deprecated endpoint: /api/detection_engine/rules/_bulk_update API is deprecated since v8.2. Please use the /api/detection_engine/rules/_bulk_action API instead. See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail."'
        );

I think your problem was that in the first commit where you tried to change this, the 153d7a79f643131db0fbd9ecad43f2823816004b, the text verified was incomplete, that's why the test failed.
Then, in this commit a527f576ec80d1c6c15414402d51ef571138dfe8, you did two things: changed the expect method, and updated the text.
But I can see that this was not needed. Please restore the original expect, just update the text inside.

Basically use this version please:

expect(header.warning).to.be(
          '299 Kibana "Deprecated endpoint: /api/detection_engine/rules/_bulk_update API is deprecated since v8.2. Please use the /api/detection_engine/rules/_bulk_action API instead. See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail."'
        );

@lcawl
Copy link
Contributor

lcawl commented Jun 13, 2025

Basically use this version please:

expect(header.warning).to.be(
          '299 Kibana "Deprecated endpoint: /api/detection_engine/rules/_bulk_update API is deprecated since v8.2. Please use the /api/detection_engine/rules/_bulk_action API instead. See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail."'
        );

Is there a way to do it but omit the last sentence from that test (" See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail.")? That's what I was trying and failing to accomplish, but has been accomplished in other places where we test only a part of a string not the whole string (e.g. in degraded_field_flyout.ts). This minimizes the fragility of the test suite when/if the URL changes again in the doc link service.

@jkelas
Copy link
Contributor

jkelas commented Jun 16, 2025

Basically use this version please:

expect(header.warning).to.be(
          '299 Kibana "Deprecated endpoint: /api/detection_engine/rules/_bulk_update API is deprecated since v8.2. Please use the /api/detection_engine/rules/_bulk_action API instead. See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail."'
        );

Is there a way to do it but omit the last sentence from that test (" See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail.")? That's what I was trying and failing to accomplish, but has been accomplished in other places where we test only a part of a string not the whole string (e.g. in degraded_field_flyout.ts). This minimizes the fragility of the test suite when/if the URL changes again in the doc link service.

Basically use this version please:

expect(header.warning).to.be(
          '299 Kibana "Deprecated endpoint: /api/detection_engine/rules/_bulk_update API is deprecated since v8.2. Please use the /api/detection_engine/rules/_bulk_action API instead. See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail."'
        );

Is there a way to do it but omit the last sentence from that test (" See https://www.elastic.co/docs/api/doc/kibana/v8/group/endpoint-security-detections-api for more detail.")? That's what I was trying and failing to accomplish, but has been accomplished in other places where we test only a part of a string not the whole string (e.g. in degraded_field_flyout.ts). This minimizes the fragility of the test suite when/if the URL changes again in the doc link service.

As discussed offline, the header.warning.includes( should work.

Copy link
Contributor

@jkelas jkelas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified the code, it is OK.
I discussed the commented issue with the author and advised on the proper structure.

LGTM now. Approving.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
aiAssistantManagementSelection 93.3KB 93.5KB +122.0B
lists 182.3KB 182.4KB +122.0B
total +244.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 451.0KB 451.1KB +122.0B

History

cc @natasha-moore-elastic

@natasha-moore-elastic natasha-moore-elastic merged commit 3579e5c into elastic:8.19 Jun 17, 2025
8 checks passed
natasha-moore-elastic added a commit to natasha-moore-elastic/kibana that referenced this pull request Jun 17, 2025
…vice (elastic#223388)

This PR replaces the old Security API doc links with API refence links
in the Kibana doc link service.

[This security-docs
PR](elastic/security-docs#6872), which removes
outdated Security Detections asciidoc API docs, is failing with the
following error:

> INFO:build_docs:Bad cross-document links:
> INFO:build_docs: Kibana [8.x]:
src/platform/packages/shared/kbn-doc-links/src/get_doc_links.ts contains
broken links to:
> INFO:build_docs:   - en/security/8.x/rule-api-overview.html
> INFO:build_docs:   - en/security/8.x/signals-migration-api.htm

Replacing these links in the Kibana doc link service should unblock the
docs PR.

The links were replaced in `main` in [this
PR](elastic#219005).

---------

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Jacek Kolezynski <[email protected]>
(cherry picked from commit 3579e5c)
@natasha-moore-elastic
Copy link
Contributor Author

💔 Some backports could not be created

Status Branch Result
8.18
8.17 Conflict resolution was aborted by the user
8.16 Conflict resolution was aborted by the user

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 223388

Questions ?

Please refer to the Backport tool documentation

natasha-moore-elastic added a commit that referenced this pull request Jun 17, 2025
…ink service (#223388) (#224208)

This will backport the following commits from `8.19` to `8.18`:
* #223388

Co-authored-by: Lisa Cawley <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Jacek Kolezynski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Docs v8.19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants