Skip to content
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

[Defend Workflows] Set deprecated flag to true in router for deprecated endpoint API's #197029

Conversation

dasansol92
Copy link
Contributor

Summary

Set deprecated: true flag to endpoint api routes.
These routes are already marked as deprecated in api docs:

@dasansol92 dasansol92 added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0 backport:version Backport to applied version labels v8.17.0 labels Oct 21, 2024
@dasansol92 dasansol92 marked this pull request as ready for review October 21, 2024 13:35
@dasansol92 dasansol92 requested a review from a team as a code owner October 21, 2024 13:35
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

Copy link
Contributor

@szwarckonrad szwarckonrad left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this!

@TinaHeiligers
Copy link
Contributor

@dasansol92 If these APIs going to be removed in v9.0.0 and you want the Upgrade Assistant to show critical notices for them, please be aware of these pending changes to deprecation .

The tracking issue for APIs that need changes is API Deprecations replace deprecated: true with the deprecated object

@dasansol92
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 23, 2024

💔 Build Failed

Failed CI Steps

Metrics [docs]

✅ unchanged

History

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

deprecated needs to be an object within the version declaration.
You'll need to update the doc links and double check on the deprecation objects w.r.t correct replacement path and severity levels.

@@ -28,6 +28,7 @@ export function registerActionAuditLogRoutes(
access: 'public',
path: ENDPOINT_ACTION_LOG_ROUTE,
options: { authRequired: true, tags: ['access:securitySolution'] },
deprecated: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

deprecated needs to be declared on .addVersion because the endpoint is being removed completely and needs to be surfaced in the Upgrade Assistant.

**
 * Registers the endpoint activity_log route
 * @deprecated
 * @removeBy 9.0.0
 *
 */
export function registerActionAuditLogRoutes(
  router: SecuritySolutionPluginRouter,
  endpointContext: EndpointAppContext
) {
  router.versioned
    .get({
      access: 'public',
      path: ENDPOINT_ACTION_LOG_ROUTE,
      options: { authRequired: true, tags: ['access:securitySolution'] },
    })
    .addVersion(
      {
        version: '2023-10-31',
        options: {
          deprecated: {
            documentationUrl: 'https://elastic.co', // add a link to your docs about the change
            severity: 'critical',
            reason: {
              type: 'remove',
            },
          },
        },
        validate: {
          request: EndpointActionLogRequestSchema,
        },
      },
      withEndpointAuthz(
        { all: ['canIsolateHost'] },
        endpointContext.logFactory.get('hostIsolationLogs'),
        auditLogRequestHandler(endpointContext)
      )
    );
}

@@ -81,6 +81,7 @@ export function registerResponseActionRoutes(
access: 'public',
path: ISOLATE_HOST_ROUTE,
options: { authRequired: true, tags: ['access:securitySolution'] },
deprecated: true,
Copy link
Contributor

@TinaHeiligers TinaHeiligers Oct 23, 2024

Choose a reason for hiding this comment

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

Move deprecated into .addVersion:

  /**
   * @deprecated use ISOLATE_HOST_ROUTE_V2 instead
   */
  router.versioned
    .post({
      access: 'public',
      path: ISOLATE_HOST_ROUTE,
      options: { authRequired: true, tags: ['access:securitySolution'] },
    })
    .addVersion(
      {
        version: '2023-10-31',
        options: {
          deprecated: {
            documentationUrl: 'https://elastic.co',
            severity: 'critical',
            reason: {
              type: 'migrate',
              newApiPath: `${ISOLATE_HOST_ROUTE_V2}`,
              newApiMethod: 'post',
            },
          },
        },
        validate: {
          request: IsolateRouteRequestSchema,
        },
      },
      withEndpointAuthz({ all: ['canIsolateHost'] }, logger, redirectHandler(ISOLATE_HOST_ROUTE_V2))
    );

  /**
   * @deprecated use RELEASE_HOST_ROUTE instead
   */
  router.versioned
    .post({
      access: 'public',
      path: UNISOLATE_HOST_ROUTE,
      options: { authRequired: true, tags: ['access:securitySolution'] },
    })
    .addVersion(
      {
        version: '2023-10-31',
        options: {
          deprecated: {
            documentationUrl: 'https://elastic.co',
            severity: 'critical',
            reason: {
              type: 'migrate',
              newApiPath: `${ISOLATE_HOST_ROUTE_V2}`
              newApiMethod: 'post',
            },
          },
        },
        validate: {
          request: UnisolateRouteRequestSchema,
        },
      },
      withEndpointAuthz(
        { all: ['canUnIsolateHost'] },
        logger,
        redirectHandler(UNISOLATE_HOST_ROUTE_V2)
      )
    );

Note: RELEASE_HOST_ROUTE doesn't exist. The doc comment should probably be " use UNISOLATE_HOST_ROUTE_v2"

@@ -56,6 +56,7 @@ export function registerPolicyRoutes(
access: 'public',
path: AGENT_POLICY_SUMMARY_ROUTE,
options: { authRequired: true },
deprecated: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to .addVersion:

  /**
   * @deprecated
   * @removeBy 9.0.0
   *
   */
  router.versioned
    .get({
      access: 'public',
      path: AGENT_POLICY_SUMMARY_ROUTE,
      options: { authRequired: true },
    })
    .addVersion(
      {
        version: '2023-10-31',
        options: {
          deprecated: {
            documentationUrl: 'https://elastic.co', // link to docs explaining the removal
            severity: 'critical',
            reason: {
              type: 'remove',
            },
          },
        },
        validate: {
          request: GetAgentPolicySummaryRequestSchema,
        },
      },
      withEndpointAuthz(
        { all: ['canAccessEndpointManagement'] },
        logger,
        getAgentPolicySummaryHandler(endpointAppContext)
      )
    );

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

deprecated only needs to be changed to an object for merging to 8.x and main.

@TinaHeiligers TinaHeiligers self-requested a review October 25, 2024 15:11
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

route options deprecated needs to be an object or expect a linting error: // @ts-expect-error TODO(https://github.com/elastic/kibana/issues/196095): Replace {RouteDeprecationInfo}

@dasansol92
Copy link
Contributor Author

Closing this pr since we will handle those changes in a follow up

@dasansol92 dasansol92 closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants