-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[UA][Core] Surface integrations with internal APIs in upgrade assistant #199026
Conversation
…ibana into core/internal_apis_deprecations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surfaces external calls to internal APIs in UA as expected.
Usage counters look off, we're counting calls to native kibana **/bundels/**
routes that I don't think we want to include.
@@ -20,5 +20,8 @@ export const DEPRECATED_ROUTES = { | |||
DEPRECATED_ROUTE: '/api/routing_example/d/deprecated_route', | |||
REMOVED_ROUTE: '/api/routing_example/d/removed_route', | |||
MIGRATED_ROUTE: '/api/routing_example/d/migrated_route', | |||
VERSIONED_ROUTE: '/api/routing_example/d/versioned', | |||
VERSIONED_ROUTE: '/api/routing_example/d/versioned_route', | |||
INTERNAL_DEPRECATED_ROUTE: '/api/routing_example/d/internal_deprecated_route', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are great for testing within Kibana, where the internal origin header is added to a request sent from console.
However, we need to surface calls to internal APIs mainly when requests originate externally and may not necessarily include the header. Surfacing these will warn end-users about the upcoming change.
We also want telemetry, to track adoption of internal APIs that are being consumed by integrations and use this info to consider public API alternatives instead,
In other words:
server.restrictInternalApis = true // commenting out also works
desired:
- surface external requests to internal APIs only (current behavior of this PR)
- increment counters for routes called externally from Kibana (ATM, usage counters include other routes such as
unversioned|get|/XXXXXXXXXXXX/bundles/plugin/transform/1.0.0/{path*}
,unversioned|get|/XXXXXXXXXXXX/bundles/plugin/discover/1.0.0/{path*}
andunversioned|get|/XXXXXXXXXXXX/bundles/plugin/aiops/1.0.0/{path*}
)) - no counters for routes called externally 😞
scenario 2:
server.restrictInternalApis = false //explicitly opt-in to use internal APIs.
desired:
- surface external requests to internal APIs only (only when requests don't include the origin header)
- log warning when restriction's disabledt
- usage counters: still logging
****/gundle/**
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the bundle tracking, it should be fixed now, the boolean i was using for non versioned routes excluded the bundles from the calculation. The bundles
are public
apis so they should not be tracked,
I dont fully understand the scenarios and desired behaviors though. I believe I am covering the desired behavior here but I am not sure i follow the rest of the comment.
These are great for testing within Kibana, where the internal origin header is added to a request sent from console.
I've updated the README and console commands to explicitly set elasticInternalOrigin=false
to enable tracking them from the console for quick tesitng. We do have integration tests to ensure proper testing for the behavior of this and the previous api deprecations.
In the dev console we need to explicitly set the query param elasticInternalOrigin
to false
to track the request as non-internal origin.
# Route deprecations for Versioned routes
GET kbn:/api/routing_example/d/versioned_route?apiVersion=2023-10-31&elasticInternalOrigin=false
# Route deprecations for Non-versioned routes
GET kbn:/api/routing_example/d/removed_route?elasticInternalOrigin=false
GET kbn:/api/routing_example/d/deprecated_route?elasticInternalOrigin=false
POST kbn:/api/routing_example/d/migrated_route?elasticInternalOrigin=false
{}
# Access deprecations
GET kbn:/api/routing_example/d/internal_deprecated_route?elasticInternalOrigin=false
GET kbn:/internal/routing_example/d/internal_only_route?elasticInternalOrigin=false
GET kbn:/internal/routing_example/d/internal_versioned_route?apiVersion=1&elasticInternalOrigin=false
However, we need to surface calls to internal APIs mainly when requests originate externally and may not necessarily include the header. Surfacing these will warn end-users about the upcoming change.
Can you clarify what you mean exactly here? External vs internal calls are specified via the header (or query param), and if the header/queryParam is missing then its an external call which in that case qualifies to be surfaced in the deprecations as access api deprecation (Internal access + the requesting caller is external).
no counters for routes called externally 😞
surface external requests to internal APIs only (only when requests don't include the origin header)
Are you sure? how are you testing this? I've tested locally and we do have unit and integration tests that ensure this behavior is working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retested and the changes look great! Both scenarios I was testing for work as expected.
With the restriction explicitly disabled in kibana.yml
, we get a deprecation entry in UA when the API is called without the internal origin header and the usage counters increment, consistent with logging added in #195696.
Steps to reproduce the scenario without using the example plugin:
- disable the restriction in
kibana.yml
:server.restrictInternalApis: false
- run es & kibana in dev
- make curl request to an internal API without the internal origin header. For example:
curl --location 'localhost:5601/abc/internal/kibana/global_settings' --header 'Content-Type: application/json' --header 'Accept-Encoding: gzip, deflate, br' --header 'kbn-xsrf: kibana' --header 'Kbn-Version: 9.0.0' --header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ=='
- Navigate to Upgrade Assistant, where you'll see a warning for a Kibana deprecation:
- Selecting the warning allows one to mark it as resolved:
- Get the usage counters from console:
GET .kibana_usage_counters/_search
{
"query": {
"bool": {
"should": [
{"match": { "usage-counter.counterType": "deprecated_api_call:total"}},
{"match": { "usage-counter.counterType": "deprecated_api_call:resolved"}},
{"match": { "usage-counter.counterType": "deprecated_api_call:marked_as_resolved"}}
]
}
}
}
Response should be similar to
Response should be similar to
{
"took": 2,
"timed_out": false,
"_shards": {
"total": 1,
"successful": 1,
"skipped": 0,
"failed": 0
},
"hits": {
"total": {
"value": 3,
"relation": "eq"
},
"max_score": 3.7216692,
"hits": [
{
"_index": ".kibana_usage_counters_9.0.0_001",
"_id": "usage-counter:core:unversioned|get|/internal/kibana/global_settings:deprecated_api_call:marked_as_resolved:server:20241111",
"_score": 3.7216692,
"_source": {
"usage-counter": {
"domainId": "core",
"counterName": "unversioned|get|/internal/kibana/global_settings",
"counterType": "deprecated_api_call:marked_as_resolved",
"source": "server",
"count": 1
},
"type": "usage-counter",
"managed": false,
"coreMigrationVersion": "8.8.0",
"updated_at": "2024-11-11T18:13:36.136Z"
}
},
{
"_index": ".kibana_usage_counters_9.0.0_001",
"_id": "usage-counter:core:unversioned|get|/internal/kibana/global_settings:deprecated_api_call:resolved:server:20241111",
"_score": 3.7216692,
"_source": {
"usage-counter": {
"domainId": "core",
"counterName": "unversioned|get|/internal/kibana/global_settings",
"counterType": "deprecated_api_call:resolved",
"source": "server",
"count": 1
},
"type": "usage-counter",
"managed": false,
"coreMigrationVersion": "8.8.0",
"updated_at": "2024-11-11T18:13:36.133Z"
}
},
{
"_index": ".kibana_usage_counters_9.0.0_001",
"_id": "usage-counter:core:unversioned|get|/internal/kibana/global_settings:deprecated_api_call:total:server:20241111",
"_score": 3.7216692,
"_source": {
"usage-counter": {
"domainId": "core",
"counterName": "unversioned|get|/internal/kibana/global_settings",
"counterType": "deprecated_api_call:total",
"source": "server",
"count": 2
},
"type": "usage-counter",
"managed": false,
"coreMigrationVersion": "8.8.0",
"updated_at": "2024-11-11T18:15:06.197Z"
}
}
]
}
}
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Public APIs missing exports
Unknown metric groupsAPI count
History
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just reviewed the ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM
logic and left a q. Overall approach LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New approach makes sense to me, test changes to UA make sense to me. Tested locally 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Bamieh !
Code only review, and left only minor nits with a question about setting internal routes as deprecated we should probably discuss!
const isNotPublicAccess = !isPublicAccess; | ||
const isNotInternalRequest = !isInternalApiRequest; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
const isNotPublicAccess = !isPublicAccess; | |
const isNotInternalRequest = !isInternalApiRequest; | |
const isInternalAccess = !isPublicAccess; | |
const isExternalApiRequest = !isInternalApiRequest; |
/** | ||
* Post Validation Route emitter metadata. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is this intended to be @public
?
deprecated: { | ||
documentationUrl: 'https://elastic.co/', | ||
severity: 'critical', | ||
message: 'Additonal message for internal deprecated api', | ||
reason: { type: 'deprecate' }, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually raises something of an oversight: I don't think ever setting an internal route as deprecated was intended for this API 🤔 , although being able to set critical
on it could be useful if we want to remove it and have known integrations.
My gut feel would be to remove it from the types for internal routes (versioned or non-versioned). FWIW I don't think we should block on it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets address this in a separate PR after we think about it for all API deprecations. Created a placeholder issue for now #199675
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut feel would be to remove it from the types for internal routes (versioned or non-versioned).
++ internal APIs aren't supposed to be consumed externally and we can add.jsdocs
to warn plugin developers that the route is deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works like a charm!
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/11796199057 |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ssistant (#199026) (#199764) # Backport This will backport the following commits from `main` to `8.x`: - [[UA][Core] Surface integrations with internal APIs in upgrade assistant (#199026)](#199026) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ahmad Bamieh","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-12T11:19:22Z","message":"[UA][Core] Surface integrations with internal APIs in upgrade assistant (#199026)\n\n## Summary\r\n\r\n> In #117241 we're surfacing\r\nusage of APIs marked as `deprecated: true` in the Upgrade Assistant to\r\nhelp users prepare for a major upgrade. While internal APIs aren't\r\nreally deprecated in the same sense we are making a breaking change by\r\nblocking external integrations with these APIs. Since this could be\r\nequally disruptive to users depending on these APIs it would help our\r\nusers to surface such usage in the UA too.\r\n\r\nThe `api` deprecations now have two sub types:\r\n1. routes deprecations `options.deprecated: { … }`\r\n2. access deprecations `options.access: 'internal'`\r\n\r\nThis PR adds the second `api` deprecation subtype. The reason i kept one\r\n`api` deprecation type and i didnt create a new type is that they have\r\nexactly the same registration process but are triggered by different\r\nattributes. The `api` deprecation is fully managed by the core team\r\ninternal services and are configured by the user through the route\r\ninterface so it makes sense to keep them as one type. I also can see us\r\nadding more subtypes to this and just piggybacking on the current flow\r\ninstead of duplicating it everytime.\r\n\r\n\r\n**Checklist**\r\n- [x] Create deprecation subtype\r\n- [x] Example plugin\r\n- [x] Surface the deprecation in UA\r\n- [x] Api access deprecation copy (@florent-leborgne )\r\n- [x] Update README and code annotations\r\n- [x] Unit tests\r\n- [x] Integration tests\r\n\r\n\r\nCloses https://github.com/elastic/kibana/issues/194675\r\n\r\n### Design decisions:\r\nIf the API has both route deprecation (`options.deprecated: { … }` ) AND\r\nis an internal api `options.access: 'internal'`\r\n\r\nThe current behavior i went for in my PR:\r\nI show this API once in the UA under the internal access deprecation.\r\nWhile showing the route deprecation details if defined. This seems to\r\nmake the most sense since users should stop using this API altogether.\r\n\r\n### Copy decisions:\r\n@florent-leborgne wrote the copy for this deprecation subtype.\r\n<img width=\"1319\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9a32f6d1-686a-4405-aec6-786ac5e10130\">\r\n\r\n<img width=\"713\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1304c98d-4c64-468e-a7d6-19c1193bf678\">\r\n\r\n\r\n## Testing\r\n\r\nRun kibana locally with the test example plugin that has deprecated\r\nroutes\r\n```\r\nyarn start --plugin-path=examples/routing_example --plugin-path=examples/developer_examples\r\n```\r\n\r\nThe following comprehensive deprecated routes examples are registered\r\ninside the folder:\r\n`examples/routing_example/server/routes/deprecated_routes`\r\n\r\nRun them in the dev console to trigger the deprecation condition so they\r\nshow up in the UA:\r\n\r\n```\r\nGET kbn:/api/routing_example/d/internal_deprecated_route?elasticInternalOrigin=false\r\nGET kbn:/internal/routing_example/d/internal_only_route?elasticInternalOrigin=false\r\nGET kbn:/internal/routing_example/d/internal_versioned_route?apiVersion=1&elasticInternalOrigin=false\r\n```\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"a10eb1fe4e55aa0cfbbb4b12a8d740a867463283","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","v8.17.0"],"number":199026,"url":"https://github.com/elastic/kibana/pull/199026","mergeCommit":{"message":"[UA][Core] Surface integrations with internal APIs in upgrade assistant (#199026)\n\n## Summary\r\n\r\n> In #117241 we're surfacing\r\nusage of APIs marked as `deprecated: true` in the Upgrade Assistant to\r\nhelp users prepare for a major upgrade. While internal APIs aren't\r\nreally deprecated in the same sense we are making a breaking change by\r\nblocking external integrations with these APIs. Since this could be\r\nequally disruptive to users depending on these APIs it would help our\r\nusers to surface such usage in the UA too.\r\n\r\nThe `api` deprecations now have two sub types:\r\n1. routes deprecations `options.deprecated: { … }`\r\n2. access deprecations `options.access: 'internal'`\r\n\r\nThis PR adds the second `api` deprecation subtype. The reason i kept one\r\n`api` deprecation type and i didnt create a new type is that they have\r\nexactly the same registration process but are triggered by different\r\nattributes. The `api` deprecation is fully managed by the core team\r\ninternal services and are configured by the user through the route\r\ninterface so it makes sense to keep them as one type. I also can see us\r\nadding more subtypes to this and just piggybacking on the current flow\r\ninstead of duplicating it everytime.\r\n\r\n\r\n**Checklist**\r\n- [x] Create deprecation subtype\r\n- [x] Example plugin\r\n- [x] Surface the deprecation in UA\r\n- [x] Api access deprecation copy (@florent-leborgne )\r\n- [x] Update README and code annotations\r\n- [x] Unit tests\r\n- [x] Integration tests\r\n\r\n\r\nCloses https://github.com/elastic/kibana/issues/194675\r\n\r\n### Design decisions:\r\nIf the API has both route deprecation (`options.deprecated: { … }` ) AND\r\nis an internal api `options.access: 'internal'`\r\n\r\nThe current behavior i went for in my PR:\r\nI show this API once in the UA under the internal access deprecation.\r\nWhile showing the route deprecation details if defined. This seems to\r\nmake the most sense since users should stop using this API altogether.\r\n\r\n### Copy decisions:\r\n@florent-leborgne wrote the copy for this deprecation subtype.\r\n<img width=\"1319\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9a32f6d1-686a-4405-aec6-786ac5e10130\">\r\n\r\n<img width=\"713\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1304c98d-4c64-468e-a7d6-19c1193bf678\">\r\n\r\n\r\n## Testing\r\n\r\nRun kibana locally with the test example plugin that has deprecated\r\nroutes\r\n```\r\nyarn start --plugin-path=examples/routing_example --plugin-path=examples/developer_examples\r\n```\r\n\r\nThe following comprehensive deprecated routes examples are registered\r\ninside the folder:\r\n`examples/routing_example/server/routes/deprecated_routes`\r\n\r\nRun them in the dev console to trigger the deprecation condition so they\r\nshow up in the UA:\r\n\r\n```\r\nGET kbn:/api/routing_example/d/internal_deprecated_route?elasticInternalOrigin=false\r\nGET kbn:/internal/routing_example/d/internal_only_route?elasticInternalOrigin=false\r\nGET kbn:/internal/routing_example/d/internal_versioned_route?apiVersion=1&elasticInternalOrigin=false\r\n```\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"a10eb1fe4e55aa0cfbbb4b12a8d740a867463283"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199026","number":199026,"mergeCommit":{"message":"[UA][Core] Surface integrations with internal APIs in upgrade assistant (#199026)\n\n## Summary\r\n\r\n> In #117241 we're surfacing\r\nusage of APIs marked as `deprecated: true` in the Upgrade Assistant to\r\nhelp users prepare for a major upgrade. While internal APIs aren't\r\nreally deprecated in the same sense we are making a breaking change by\r\nblocking external integrations with these APIs. Since this could be\r\nequally disruptive to users depending on these APIs it would help our\r\nusers to surface such usage in the UA too.\r\n\r\nThe `api` deprecations now have two sub types:\r\n1. routes deprecations `options.deprecated: { … }`\r\n2. access deprecations `options.access: 'internal'`\r\n\r\nThis PR adds the second `api` deprecation subtype. The reason i kept one\r\n`api` deprecation type and i didnt create a new type is that they have\r\nexactly the same registration process but are triggered by different\r\nattributes. The `api` deprecation is fully managed by the core team\r\ninternal services and are configured by the user through the route\r\ninterface so it makes sense to keep them as one type. I also can see us\r\nadding more subtypes to this and just piggybacking on the current flow\r\ninstead of duplicating it everytime.\r\n\r\n\r\n**Checklist**\r\n- [x] Create deprecation subtype\r\n- [x] Example plugin\r\n- [x] Surface the deprecation in UA\r\n- [x] Api access deprecation copy (@florent-leborgne )\r\n- [x] Update README and code annotations\r\n- [x] Unit tests\r\n- [x] Integration tests\r\n\r\n\r\nCloses https://github.com/elastic/kibana/issues/194675\r\n\r\n### Design decisions:\r\nIf the API has both route deprecation (`options.deprecated: { … }` ) AND\r\nis an internal api `options.access: 'internal'`\r\n\r\nThe current behavior i went for in my PR:\r\nI show this API once in the UA under the internal access deprecation.\r\nWhile showing the route deprecation details if defined. This seems to\r\nmake the most sense since users should stop using this API altogether.\r\n\r\n### Copy decisions:\r\n@florent-leborgne wrote the copy for this deprecation subtype.\r\n<img width=\"1319\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9a32f6d1-686a-4405-aec6-786ac5e10130\">\r\n\r\n<img width=\"713\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1304c98d-4c64-468e-a7d6-19c1193bf678\">\r\n\r\n\r\n## Testing\r\n\r\nRun kibana locally with the test example plugin that has deprecated\r\nroutes\r\n```\r\nyarn start --plugin-path=examples/routing_example --plugin-path=examples/developer_examples\r\n```\r\n\r\nThe following comprehensive deprecated routes examples are registered\r\ninside the folder:\r\n`examples/routing_example/server/routes/deprecated_routes`\r\n\r\nRun them in the dev console to trigger the deprecation condition so they\r\nshow up in the UA:\r\n\r\n```\r\nGET kbn:/api/routing_example/d/internal_deprecated_route?elasticInternalOrigin=false\r\nGET kbn:/internal/routing_example/d/internal_only_route?elasticInternalOrigin=false\r\nGET kbn:/internal/routing_example/d/internal_versioned_route?apiVersion=1&elasticInternalOrigin=false\r\n```\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"a10eb1fe4e55aa0cfbbb4b12a8d740a867463283"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Elastic Machine <[email protected]>
…nt (elastic#199026) ## Summary > In elastic#117241 we're surfacing usage of APIs marked as `deprecated: true` in the Upgrade Assistant to help users prepare for a major upgrade. While internal APIs aren't really deprecated in the same sense we are making a breaking change by blocking external integrations with these APIs. Since this could be equally disruptive to users depending on these APIs it would help our users to surface such usage in the UA too. The `api` deprecations now have two sub types: 1. routes deprecations `options.deprecated: { … }` 2. access deprecations `options.access: 'internal'` This PR adds the second `api` deprecation subtype. The reason i kept one `api` deprecation type and i didnt create a new type is that they have exactly the same registration process but are triggered by different attributes. The `api` deprecation is fully managed by the core team internal services and are configured by the user through the route interface so it makes sense to keep them as one type. I also can see us adding more subtypes to this and just piggybacking on the current flow instead of duplicating it everytime. **Checklist** - [x] Create deprecation subtype - [x] Example plugin - [x] Surface the deprecation in UA - [x] Api access deprecation copy (@florent-leborgne ) - [x] Update README and code annotations - [x] Unit tests - [x] Integration tests Closes elastic#194675 ### Design decisions: If the API has both route deprecation (`options.deprecated: { … }` ) AND is an internal api `options.access: 'internal'` The current behavior i went for in my PR: I show this API once in the UA under the internal access deprecation. While showing the route deprecation details if defined. This seems to make the most sense since users should stop using this API altogether. ### Copy decisions: @florent-leborgne wrote the copy for this deprecation subtype. <img width="1319" alt="image" src="https://github.com/user-attachments/assets/9a32f6d1-686a-4405-aec6-786ac5e10130"> <img width="713" alt="image" src="https://github.com/user-attachments/assets/1304c98d-4c64-468e-a7d6-19c1193bf678"> ## Testing Run kibana locally with the test example plugin that has deprecated routes ``` yarn start --plugin-path=examples/routing_example --plugin-path=examples/developer_examples ``` The following comprehensive deprecated routes examples are registered inside the folder: `examples/routing_example/server/routes/deprecated_routes` Run them in the dev console to trigger the deprecation condition so they show up in the UA: ``` GET kbn:/api/routing_example/d/internal_deprecated_route?elasticInternalOrigin=false GET kbn:/internal/routing_example/d/internal_only_route?elasticInternalOrigin=false GET kbn:/internal/routing_example/d/internal_versioned_route?apiVersion=1&elasticInternalOrigin=false ``` --------- Co-authored-by: kibanamachine <[email protected]>
…nt (elastic#199026) ## Summary > In elastic#117241 we're surfacing usage of APIs marked as `deprecated: true` in the Upgrade Assistant to help users prepare for a major upgrade. While internal APIs aren't really deprecated in the same sense we are making a breaking change by blocking external integrations with these APIs. Since this could be equally disruptive to users depending on these APIs it would help our users to surface such usage in the UA too. The `api` deprecations now have two sub types: 1. routes deprecations `options.deprecated: { … }` 2. access deprecations `options.access: 'internal'` This PR adds the second `api` deprecation subtype. The reason i kept one `api` deprecation type and i didnt create a new type is that they have exactly the same registration process but are triggered by different attributes. The `api` deprecation is fully managed by the core team internal services and are configured by the user through the route interface so it makes sense to keep them as one type. I also can see us adding more subtypes to this and just piggybacking on the current flow instead of duplicating it everytime. **Checklist** - [x] Create deprecation subtype - [x] Example plugin - [x] Surface the deprecation in UA - [x] Api access deprecation copy (@florent-leborgne ) - [x] Update README and code annotations - [x] Unit tests - [x] Integration tests Closes elastic#194675 ### Design decisions: If the API has both route deprecation (`options.deprecated: { … }` ) AND is an internal api `options.access: 'internal'` The current behavior i went for in my PR: I show this API once in the UA under the internal access deprecation. While showing the route deprecation details if defined. This seems to make the most sense since users should stop using this API altogether. ### Copy decisions: @florent-leborgne wrote the copy for this deprecation subtype. <img width="1319" alt="image" src="https://github.com/user-attachments/assets/9a32f6d1-686a-4405-aec6-786ac5e10130"> <img width="713" alt="image" src="https://github.com/user-attachments/assets/1304c98d-4c64-468e-a7d6-19c1193bf678"> ## Testing Run kibana locally with the test example plugin that has deprecated routes ``` yarn start --plugin-path=examples/routing_example --plugin-path=examples/developer_examples ``` The following comprehensive deprecated routes examples are registered inside the folder: `examples/routing_example/server/routes/deprecated_routes` Run them in the dev console to trigger the deprecation condition so they show up in the UA: ``` GET kbn:/api/routing_example/d/internal_deprecated_route?elasticInternalOrigin=false GET kbn:/internal/routing_example/d/internal_only_route?elasticInternalOrigin=false GET kbn:/internal/routing_example/d/internal_versioned_route?apiVersion=1&elasticInternalOrigin=false ``` --------- Co-authored-by: kibanamachine <[email protected]>
Summary
The
api
deprecations now have two sub types:options.deprecated: { … }
options.access: 'internal'
This PR adds the second
api
deprecation subtype. The reason i kept oneapi
deprecation type and i didnt create a new type is that they have exactly the same registration process but are triggered by different attributes. Theapi
deprecation is fully managed by the core team internal services and are configured by the user through the route interface so it makes sense to keep them as one type. I also can see us adding more subtypes to this and just piggybacking on the current flow instead of duplicating it everytime.Checklist
Closes #194675
Design decisions:
If the API has both route deprecation (
options.deprecated: { … }
) AND is an internal apioptions.access: 'internal'
The current behavior i went for in my PR:
I show this API once in the UA under the internal access deprecation. While showing the route deprecation details if defined. This seems to make the most sense since users should stop using this API altogether.
Copy decisions:
@florent-leborgne wrote the copy for this deprecation subtype.
Testing
Run kibana locally with the test example plugin that has deprecated routes
The following comprehensive deprecated routes examples are registered inside the folder:
examples/routing_example/server/routes/deprecated_routes
Run them in the dev console to trigger the deprecation condition so they show up in the UA: