Skip to content

Commit

Permalink
[UA][Core][API Deprecations] Add deprecate type and update copy (elas…
Browse files Browse the repository at this point in the history
…tic#198800)

## Summary

- [x] Add `deprecate` Type to the API Deprecations reasons.
- [x] Add a `message` optional field that is surfaced in the UA
- [x] Add IDE documentation in the autocomplete for all deprecation
fields.
- [x] Updated README and example plugin for the `deprecate` type
- [x] Update copy for `deprecate`.


Closes elastic#197721

## 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/deprecated_route
```

<img width="628" alt="image"
src="https://github.com/user-attachments/assets/3c0e1829-9a07-49bd-94a3-398514f448e2">

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: florent-leborgne <[email protected]>
  • Loading branch information
4 people authored Nov 6, 2024
1 parent 833658f commit 665cf98
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 30 deletions.
1 change: 1 addition & 0 deletions examples/routing_example/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const POST_MESSAGE_ROUTE_PATH = '/api/post_message';
export const INTERNAL_GET_MESSAGE_BY_ID_ROUTE = '/internal/get_message';

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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,28 @@ import { schema } from '@kbn/config-schema';
import { DEPRECATED_ROUTES } from '../../../common';

export const registerDeprecatedRoute = (router: IRouter) => {
router.get(
{
path: DEPRECATED_ROUTES.DEPRECATED_ROUTE,
validate: false,
options: {
access: 'public',
deprecated: {
documentationUrl: 'https://elastic.co/',
severity: 'warning',
message:
'This deprecation message will be surfaced in UA. use `i18n.translate` to internationalize this message.',
reason: { type: 'deprecate' },
},
},
},
async (ctx, req, res) => {
return res.ok({
body: { result: 'Called deprecated route. Check UA to see the deprecation.' },
});
}
);

router.get(
{
path: DEPRECATED_ROUTES.REMOVED_ROUTE,
Expand All @@ -27,7 +49,7 @@ export const registerDeprecatedRoute = (router: IRouter) => {
},
async (ctx, req, res) => {
return res.ok({
body: { result: 'Called deprecated route. Check UA to see the deprecation.' },
body: { result: 'Called to be removed route. Check UA to see the deprecation.' },
});
}
);
Expand Down Expand Up @@ -55,7 +77,7 @@ export const registerDeprecatedRoute = (router: IRouter) => {
},
async (ctx, req, res) => {
return res.ok({
body: { result: 'Called deprecated route. Check UA to see the deprecation.' },
body: { result: 'Called to be migrated route. Check UA to see the deprecation.' },
});
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,9 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import type { RequestHandler } from '@kbn/core-http-server';
import type { IRouter } from '@kbn/core/server';
import { DEPRECATED_ROUTES } from '../../../common';

const createDummyHandler =
(version: string): RequestHandler =>
(ctx, req, res) => {
return res.ok({ body: { result: `API version ${version}.` } });
};

export const registerVersionedDeprecatedRoute = (router: IRouter) => {
const versionedRoute = router.versioned.get({
path: DEPRECATED_ROUTES.VERSIONED_ROUTE,
Expand All @@ -40,14 +33,20 @@ export const registerVersionedDeprecatedRoute = (router: IRouter) => {
validate: false,
version: '1',
},
createDummyHandler('1')
(ctx, req, res) => {
return res.ok({
body: { result: 'Called deprecated version of the API. API version 1 -> 2' },
});
}
);

versionedRoute.addVersion(
{
version: '2',
validate: false,
},
createDummyHandler('2')
(ctx, req, res) => {
return res.ok({ body: { result: 'Called API version 2' } });
}
);
};
1 change: 0 additions & 1 deletion examples/routing_example/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,5 @@
"@kbn/core-http-browser",
"@kbn/config-schema",
"@kbn/react-kibana-context-render",
"@kbn/core-http-server",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,51 @@ describe('#registerApiDeprecationsInfo', () => {
`);
});

it('returns deprecated type deprecated route', async () => {
const getDeprecations = createGetApiDeprecations({ coreUsageData, http });
const deprecatedRoute = createDeprecatedRouteDetails({
routePath: '/api/test_deprecated/',
routeDeprecationOptions: { reason: { type: 'deprecate' }, message: 'additional message' },
});
http.getRegisteredDeprecatedApis.mockReturnValue([deprecatedRoute]);
usageClientMock.getDeprecatedApiUsageStats.mockResolvedValue([
createApiUsageStat(buildApiDeprecationId(deprecatedRoute)),
]);

const deprecations = await getDeprecations();
expect(deprecations).toMatchInlineSnapshot(`
Array [
Object {
"apiId": "123|get|/api/test_deprecated",
"correctiveActions": Object {
"manualSteps": Array [
"Identify the origin of these API calls.",
"For now, the API will still work, but will be moved or removed in a future version. Check the Learn more link for more information. If you are no longer using the API, you can mark this issue as resolved. It will no longer appear in the Upgrade Assistant unless another call using this API is detected.",
],
"mark_as_resolved_api": Object {
"apiTotalCalls": 13,
"routeMethod": "get",
"routePath": "/api/test_deprecated/",
"routeVersion": "123",
"timestamp": 2024-10-17T12:06:41.224Z,
"totalMarkedAsResolved": 1,
},
},
"deprecationType": "api",
"documentationUrl": "https://fake-url",
"domainId": "core.routes-deprecations",
"level": "critical",
"message": Array [
"The API \\"GET /api/test_deprecated/\\" has been called 13 times. The last call was on Sunday, September 1, 2024 6:06 AM -04:00.",
"This issue has been marked as resolved on Thursday, October 17, 2024 8:06 AM -04:00 but the API has been called 12 times since.",
"additional message",
],
"title": "The \\"GET /api/test_deprecated/\\" route is deprecated",
},
]
`);
});

it('does not return resolved deprecated route', async () => {
const getDeprecations = createGetApiDeprecations({ coreUsageData, http });
const deprecatedRoute = createDeprecatedRouteDetails({ routePath: '/api/test_resolved/' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const getApiDeprecationMessage = (
details: RouterDeprecatedRouteDetails,
apiUsageStats: CoreDeprecatedApiUsageStats
): string[] => {
const { routePath, routeMethod } = details;
const { routePath, routeMethod, routeDeprecationOptions } = details;
const { apiLastCalledAt, apiTotalCalls, markedAsResolvedLastCalledAt, totalMarkedAsResolved } =
apiUsageStats;

Expand Down Expand Up @@ -71,6 +71,11 @@ export const getApiDeprecationMessage = (
);
}

if (routeDeprecationOptions.message) {
// Surfaces additional deprecation messages passed into the route in UA
messages.push(routeDeprecationOptions.message);
}

return messages;
};

Expand Down Expand Up @@ -106,6 +111,15 @@ export const getApiDeprecationsManualSteps = (details: RouterDeprecatedRouteDeta
);
break;
}
case 'deprecate': {
manualSteps.push(
i18n.translate('core.deprecations.deprecations.manualSteps.removeTypeExplainationStep', {
defaultMessage:
'For now, the API will still work, but will be moved or removed in a future version. Check the Learn more link for more information. If you are no longer using the API, you can mark this issue as resolved. It will no longer appear in the Upgrade Assistant unless another call using this API is detected.',
})
);
break;
}
case 'migrate': {
const { newApiPath, newApiMethod } = routeDeprecationOptions.reason;
const newRouteWithMethod = `${newApiMethod.toUpperCase()} ${newApiPath}`;
Expand All @@ -121,12 +135,14 @@ export const getApiDeprecationsManualSteps = (details: RouterDeprecatedRouteDeta
}
}

manualSteps.push(
i18n.translate('core.deprecations.deprecations.manualSteps.markAsResolvedStep', {
defaultMessage:
'Check that you are no longer using the old API in any requests, and mark this issue as resolved. It will no longer appear in the Upgrade Assistant unless another call using this API is detected.',
})
);
if (deprecationType !== 'deprecate') {
manualSteps.push(
i18n.translate('core.deprecations.deprecations.manualSteps.markAsResolvedStep', {
defaultMessage:
'Check that you are no longer using the old API in any requests, and mark this issue as resolved. It will no longer appear in the Upgrade Assistant unless another call using this API is detected.',
})
);
}

return manualSteps;
};
68 changes: 57 additions & 11 deletions packages/core/http/core-http-server/src/router/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,36 +120,82 @@ export type Privilege = string;
* created from HTTP API introspection (like OAS).
*/
export interface RouteDeprecationInfo {
/**
* link to the documentation for more details on the deprecation.
*/
documentationUrl: string;
/**
* The description message to be displayed for the deprecation.
* Check the README for writing deprecations in `src/core/server/deprecations/README.mdx`
*/
message?: string;
/**
* levels:
* - warning: will not break deployment upon upgrade.
* - critical: needs to be addressed before upgrade.
*/
severity: 'warning' | 'critical';
reason: VersionBumpDeprecationType | RemovalApiDeprecationType | MigrationApiDeprecationType;
/**
* API deprecation reason:
* - bump: New version of the API is available.
* - remove: API was fully removed with no replacement.
* - migrate: API has been migrated to a different path.
* - deprecated: the deprecated API is deprecated, it might be removed or migrated, or got a version bump in the future.
* It is a catch-all deprecation for APIs but the API will work in the next upgrades.
*/
reason:
| VersionBumpDeprecationType
| RemovalApiDeprecationType
| MigrationApiDeprecationType
| DeprecateApiDeprecationType;
}

/**
* bump deprecation reason denotes a new version of the API is available
*/
interface VersionBumpDeprecationType {
/**
* bump deprecation reason denotes a new version of the API is available
*/
type: 'bump';
/**
* new version of the API to be used instead.
*/
newApiVersion: string;
}

/**
* remove deprecation reason denotes the API was fully removed with no replacement
*/
interface RemovalApiDeprecationType {
/**
* remove deprecation reason denotes the API was fully removed with no replacement
*/
type: 'remove';
}

/**
* migrate deprecation reason denotes the API has been migrated to a different API path
* Please make sure that if you are only incrementing the version of the API to use 'bump' instead
*/
interface MigrationApiDeprecationType {
/**
* migrate deprecation reason denotes the API has been migrated to a different API path
* Please make sure that if you are only incrementing the version of the API to use 'bump' instead
*/
type: 'migrate';
/**
* new API path to be used instead
*/
newApiPath: string;
/**
* new API method (GET POST PUT DELETE) to be used with the new API.
*/
newApiMethod: string;
}

interface DeprecateApiDeprecationType {
/**
* deprecate deprecation reason denotes the API is deprecated but it doesnt have a replacement
* or a clear version that it will be removed in. This is useful to alert users that the API is deprecated
* to allow them as much time as possible to work around this fact before the deprecation
* turns into a `remove` or `migrate` or `bump` type.
*
* Recommended to pair this with `severity: 'warning'` to avoid blocking the upgrades for this type.
*/
type: 'deprecate';
}

/**
* A set of privileges that can be used to define complex authorization requirements.
*
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/upgrade_assistant/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ GET kbn:/api/routing_example/d/versioned?apiVersion=2
# Non-versioned routes
GET kbn:/api/routing_example/d/removed_route
GET kbn:/api/routing_example/d/deprecated_route
POST kbn:/api/routing_example/d/migrated_route
{}
```
Expand Down

0 comments on commit 665cf98

Please sign in to comment.