Skip to content

Commit

Permalink
BC-5628 cyclic data deletion (#4557)
Browse files Browse the repository at this point in the history
* first commit

* add some tests

* add test cases and services

* add new (almost empty for now) batch deletion app

* refactor config vars

* add optional env var for specifying delay between the API calls

* add usecases and test cases

* fix importing

* add type in uc

* fix import

* add references service that'll load all the references to the data we want to delete

* fix most of issue form review

* add deletion API client with just a single method for now that allows for sending a deletion request

* refactor the env vars for configurting the Admin API

* add exporting DeletionClientConfig

* move references service to the deletion module

* delete unused code

* add batch deletion service that makes it possible ot queue deletion for many references at once

* move some parts of the interface to the interface subdir

* add an interface for the batch deletion summary

* move some interfaces to a separate subdir

* refactor the batch deletion summary interface

* add uc for the batch deletion

* remove unused annotation

* refactor deletion client implementation

* add batch deletion service implementation

* add UC for the batch deletion

* add a console app for the deletion module and a console command to manage deletion requests queue

* remove no longer used app, add param to make it possible to define delay between the client calls for the case one would like to queue many thousands of deletion requests at once

* remove no longer used separate batch-deletion module (it became a part of the main deletion module)

* fix invalid key

* remove no longer used config vars

* remove no longer used commands

* remove no longer used Nest cli config

* remove no longer used code

* change name of the method that prepares default headers

* add builders for most of the interfaces

* add builders for the remaining interfaces

* add type in catch clause

* do some adjustments, move PushDeletionRequestsOptions interface to a separate file

* remove unused import

* rollback

* remove unnecessary indent

* remove unnecessary indents

* remove empty line

* remove repeated imports

* refactor some imports to omit calling Configuration.get() on every subpackage import

* add builder for the DeletionRequestOutput class

* add unit tests for the batch deletion service

* add unit tests for the BatchDeletionUc

* modify env keys for the Admin API client configuration, refactor the way the deletion module's console is bootstrapped

* fix invalid import, remove unused undefined arg

* add comment to ignore console.ts file for coverage

* move deletion client config interface to a separate file, refactor function that prepares current config, add unit tests for it

* fix invalid import

* add more test cases to the deletion client unit tests

* change invalid import

Co-authored-by: WojciechGrancow <[email protected]>

* fix invalid import

* add builder for the PushDeletionRequestsOptions class, add unit tests for the DeletionQueueConsole

* rename the file containing the deletion module console to deletion.console.ts, add coverage exclusion for it for the Sonar coverage analysis

* remove deletion.console.ts from the sonar.coverage.exclusions param as it doesn't seem to work anyway

* add deletion.console.ts file to the coverage exclusions (another try with different path)

* change name of the file containing the deletion console app

* add deletion client method that allows for triggering a deletion requests executions

* fix some imports

* move default value for the ADMIN_API_CLIENT object to default.schema.json

* move default for the BASE_URL

* move Deletion module console app to the apps/ dir

* add separate functino to log error and set exit code

* add handling of the case that only CR chars are used as a line separators

* add use of the BatchDeletionSummaryBuilder in place of an anonymous object creation

* fix some imports/exports

* add an interface for the deletion execution trigger result, add builder for it as well

* add use case for the deletion executions

* add new interface for the deletion execution trigger options, add builder for it

* add console command for triggering the deletion execution

* add Admin API client secret provisioned by 1Password

* add data deletion trigger cronjob

* add metadata to the data deletion trigger CronJob

* add task to add data deletion trigger CronJob

* rewrite HTTP client execution to try/catch block, modify types of returned errors to meet current project's convention

* modify the solution to not catch the client's exception in the use case, but in the console app's command

* add more test cases

* delete unnecessary cronjob label

Co-authored-by: mamutmk5 <[email protected]>

* merge labels into metadata

Co-authored-by: mamutmk5 <[email protected]>

* BC-5628 - move logic of errors

* adjusted the second client method according to Sergej's proposal

* Update apps/server/src/modules/deletion/client/deletion.client.ts

Co-authored-by: Sergej Hoffmann <[email protected]>

* remove unnecessary label

---------

Co-authored-by: WojciechGrancow <[email protected]>
Co-authored-by: WojciechGrancow <[email protected]>
Co-authored-by: mamutmk5 <[email protected]>
Co-authored-by: SevenWaysDP <[email protected]>
Co-authored-by: Sergej Hoffmann <[email protected]>
  • Loading branch information
6 people authored Nov 28, 2023
1 parent fd8dd82 commit af700bb
Show file tree
Hide file tree
Showing 20 changed files with 561 additions and 34 deletions.
13 changes: 13 additions & 0 deletions ansible/roles/schulcloud-server-core/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
template: onepassword.yml.j2
when: ONEPASSWORD_OPERATOR is defined and ONEPASSWORD_OPERATOR|bool

- name: Admin API client secret (from 1Password)
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
namespace: "{{ NAMESPACE }}"
template: onepassword-admin-api-client.yml.j2
when: ONEPASSWORD_OPERATOR is defined and ONEPASSWORD_OPERATOR|bool

- name: remove old migration Job
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
Expand Down Expand Up @@ -108,6 +115,12 @@
namespace: "{{ NAMESPACE }}"
template: api-delete-s3-files-cronjob.yml.j2

- name: Data deletion trigger CronJob
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
namespace: "{{ NAMESPACE }}"
template: data-deletion-trigger-cronjob.yml.j2

- name: AMQPFileStorageDeployment
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
apiVersion: batch/v1
kind: CronJob
metadata:
namespace: {{ NAMESPACE }}
labels:
app: data-deletion-trigger
app.kubernetes.io/part-of: schulcloud-verbund
app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
app.kubernetes.io/name: data-deletion-trigger
app.kubernetes.io/component: data-deletion
app.kubernetes.io/managed-by: ansible
git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }}
name: data-deletion-trigger-cronjob
spec:
concurrencyPolicy: Forbid
schedule: "{{ SERVER_DATA_DELETION_TRIGGER_CRONJOB_SCHEDULE|default("@hourly", true) }}"
jobTemplate:
metadata:
labels:
app: data-deletion-trigger
app.kubernetes.io/part-of: schulcloud-verbund
app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
app.kubernetes.io/name: data-deletion-trigger
app.kubernetes.io/component: data-deletion
app.kubernetes.io/managed-by: ansible
git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }}
spec:
template:
spec:
containers:
- name: data-deletion-trigger-cronjob
image: {{ SCHULCLOUD_SERVER_IMAGE }}:{{ SCHULCLOUD_SERVER_IMAGE_TAG }}
envFrom:
- secretRef:
name: admin-api-client-secret
command: ['/bin/sh', '-c']
args: ['npm run nest:start:deletion-console -- execution trigger']
resources:
limits:
cpu: {{ API_CPU_LIMITS|default("2000m", true) }}
memory: {{ API_MEMORY_LIMITS|default("2Gi", true) }}
requests:
cpu: {{ API_CPU_REQUESTS|default("100m", true) }}
memory: {{ API_MEMORY_REQUESTS|default("150Mi", true) }}
restartPolicy: OnFailure
metadata:
labels:
app: data-deletion-trigger
app.kubernetes.io/part-of: schulcloud-verbund
app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
app.kubernetes.io/name: data-deletion-trigger
app.kubernetes.io/component: data-deletion
app.kubernetes.io/managed-by: ansible
git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: onepassword.com/v1
kind: OnePasswordItem
metadata:
name: admin-api-client-secret
namespace: {{ NAMESPACE }}
spec:
itemPath: "vaults/{{ ONEPASSWORD_OPERATOR_VAULT }}/items/admin-api-client"
70 changes: 69 additions & 1 deletion apps/server/src/modules/deletion/client/deletion.client.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { of } from 'rxjs';
import { of, throwError } from 'rxjs';
import { AxiosResponse } from 'axios';
import { HttpService } from '@nestjs/axios';
import { ConfigService } from '@nestjs/config';
Expand Down Expand Up @@ -51,6 +51,23 @@ describe(DeletionClient.name, () => {
});

describe('queueDeletionRequest', () => {
describe('when sending the HTTP request failed', () => {
const setup = () => {
const input = DeletionRequestInputBuilder.build('user', '652f1625e9bc1a13bdaae48b');

const error = new Error('unknown error');
httpService.post.mockReturnValueOnce(throwError(() => error));

return { input };
};

it('should catch and throw an error', async () => {
const { input } = setup();

await expect(client.queueDeletionRequest(input)).rejects.toThrow(Error);
});
});

describe('when received valid response with expected HTTP status code', () => {
const setup = () => {
const input = DeletionRequestInputBuilder.build('user', '652f1625e9bc1a13bdaae48b');
Expand Down Expand Up @@ -151,4 +168,55 @@ describe(DeletionClient.name, () => {
});
});
});

describe('executeDeletions', () => {
describe('when sending the HTTP request failed', () => {
const setup = () => {
const error = new Error('unknown error');
httpService.post.mockReturnValueOnce(throwError(() => error));
};

it('should catch and throw an error', async () => {
setup();

await expect(client.executeDeletions()).rejects.toThrow(Error);
});
});

describe('when received valid response with expected HTTP status code', () => {
const setup = () => {
const limit = 10;

const response: AxiosResponse<DeletionRequestOutput> = axiosResponseFactory.build({
status: 204,
});

httpService.post.mockReturnValueOnce(of(response));

return { limit };
};

it('should return proper output', async () => {
const { limit } = setup();

await expect(client.executeDeletions(limit)).resolves.not.toThrow();
});
});

describe('when received invalid HTTP status code in a response', () => {
const setup = () => {
const response: AxiosResponse<DeletionRequestOutput> = axiosResponseFactory.build({
status: 200,
});

httpService.post.mockReturnValueOnce(of(response));
};

it('should throw an exception', async () => {
setup();

await expect(client.executeDeletions()).rejects.toThrow(Error);
});
});
});
});
93 changes: 62 additions & 31 deletions apps/server/src/modules/deletion/client/deletion.client.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { firstValueFrom } from 'rxjs';
import { AxiosResponse } from 'axios';
import { Injectable } from '@nestjs/common';
import { HttpService } from '@nestjs/axios';
import { BadGatewayException, Injectable } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { DeletionRequestInput, DeletionRequestOutput, DeletionClientConfig } from './interface';
import { ErrorUtils } from '@src/core/error/utils';
import { firstValueFrom } from 'rxjs';
import { DeletionClientConfig, DeletionRequestInput, DeletionRequestOutput } from './interface';

@Injectable()
export class DeletionClient {
Expand All @@ -13,6 +13,8 @@ export class DeletionClient {

private readonly postDeletionRequestsEndpoint: string;

private readonly postDeletionExecutionsEndpoint: string;

constructor(
private readonly httpService: HttpService,
private readonly configService: ConfigService<DeletionClientConfig, true>
Expand All @@ -22,36 +24,65 @@ export class DeletionClient {

// Prepare the POST /deletionRequests endpoint beforehand to not do it on every client call.
this.postDeletionRequestsEndpoint = new URL('/admin/api/v1/deletionRequests', this.baseUrl).toString();
this.postDeletionExecutionsEndpoint = new URL('/admin/api/v1/deletionExecutions', this.baseUrl).toString();
}

async queueDeletionRequest(input: DeletionRequestInput): Promise<DeletionRequestOutput> {
const request = this.httpService.post(this.postDeletionRequestsEndpoint, input, this.defaultHeaders());

return firstValueFrom(request)
.then((resp: AxiosResponse<DeletionRequestOutput>) => {
// Throw an error if any other status code (other than expected "202 Accepted" is returned).
if (resp.status !== 202) {
throw new Error(`invalid HTTP status code in a response from the server - ${resp.status} instead of 202`);
}

// Throw an error if server didn't return a requestId in a response (and it is
// required as it gives client the reference to the created deletion request).
if (!resp.data.requestId) {
throw new Error('no valid requestId returned from the server');
}

// Throw an error if server didn't return a deletionPlannedAt timestamp so the user
// will not be aware after which date the deletion request's execution will begin.
if (!resp.data.deletionPlannedAt) {
throw new Error('no valid deletionPlannedAt returned from the server');
}

return resp.data;
})
.catch((err: Error) => {
// Throw an error if sending/processing deletion request by the client failed in any way.
throw new Error(`failed to send/process a deletion request: ${err.toString()}`);
});
try {
const request = this.httpService.post<DeletionRequestOutput>(
this.postDeletionRequestsEndpoint,
input,
this.defaultHeaders()
);

const resp = await firstValueFrom(request);

// Throw an error if any other status code (other than expected "202 Accepted" is returned).
if (resp.status !== 202) {
throw new Error(`invalid HTTP status code in a response from the server - ${resp.status} instead of 202`);
}

// Throw an error if server didn't return a requestId in a response (and it is
// required as it gives client the reference to the created deletion request).
if (!resp.data.requestId) {
throw new Error('no valid requestId returned from the server');
}

// Throw an error if server didn't return a deletionPlannedAt timestamp so the user
// will not be aware after which date the deletion request's execution will begin.
if (!resp.data.deletionPlannedAt) {
throw Error('no valid deletionPlannedAt returned from the server');
}

return resp.data;
} catch (err) {
// Throw an error if sending deletion request has failed.
throw new BadGatewayException('DeletionClient:queueDeletionRequest', ErrorUtils.createHttpExceptionOptions(err));
}
}

async executeDeletions(limit?: number): Promise<void> {
let requestConfig = {};

if (limit && limit > 0) {
requestConfig = { ...this.defaultHeaders(), params: { limit } };
} else {
requestConfig = { ...this.defaultHeaders() };
}

try {
const request = this.httpService.post(this.postDeletionExecutionsEndpoint, null, requestConfig);

const resp = await firstValueFrom(request);

if (resp.status !== 204) {
// Throw an error if any other status code (other than expected "204 No Content" is returned).
throw new Error(`invalid HTTP status code in a response from the server - ${resp.status} instead of 204`);
}
} catch (err) {
// Throw an error if sending deletion request(s) execution trigger has failed.
throw new BadGatewayException('DeletionClient:executeDeletions', ErrorUtils.createHttpExceptionOptions(err));
}
}

private apiKeyHeader() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { DeletionExecutionTriggerResult, DeletionExecutionTriggerStatus } from '../interface';
import { DeletionExecutionTriggerResultBuilder } from './deletion-execution-trigger-result.builder';

describe(DeletionExecutionTriggerResultBuilder.name, () => {
describe(DeletionExecutionTriggerResultBuilder.buildSuccess.name, () => {
describe('when called', () => {
const setup = () => {
const expectedOutput: DeletionExecutionTriggerResult = { status: DeletionExecutionTriggerStatus.SUCCESS };

return { expectedOutput };
};

it('should return valid object indicating success', () => {
const { expectedOutput } = setup();

const output = DeletionExecutionTriggerResultBuilder.buildSuccess();

expect(output).toStrictEqual(expectedOutput);
});
});
});

describe(DeletionExecutionTriggerResultBuilder.buildFailure.name, () => {
describe('when called with proper arguments', () => {
const setup = () => {
const error = new Error('test error message');

const expectedOutput: DeletionExecutionTriggerResult = {
status: DeletionExecutionTriggerStatus.FAILURE,
error: error.toString(),
};

return { error, expectedOutput };
};

it('should return valid object with expected values', () => {
const { error, expectedOutput } = setup();

const output = DeletionExecutionTriggerResultBuilder.buildFailure(error);

expect(output).toStrictEqual(expectedOutput);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { DeletionExecutionTriggerResult, DeletionExecutionTriggerStatus } from '../interface';

export class DeletionExecutionTriggerResultBuilder {
private static build(status: DeletionExecutionTriggerStatus, error?: string): DeletionExecutionTriggerResult {
const output: DeletionExecutionTriggerResult = { status };

if (error) {
output.error = error;
}

return output;
}

static buildSuccess(): DeletionExecutionTriggerResult {
return this.build(DeletionExecutionTriggerStatus.SUCCESS);
}

static buildFailure(err: Error): DeletionExecutionTriggerResult {
return this.build(DeletionExecutionTriggerStatus.FAILURE, err.toString());
}
}
2 changes: 2 additions & 0 deletions apps/server/src/modules/deletion/console/builder/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export * from './push-delete-requests-options.builder';
export * from './trigger-deletion-execution-options.builder';
export * from './deletion-execution-trigger-result.builder';
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { TriggerDeletionExecutionOptions } from '../interface';
import { TriggerDeletionExecutionOptionsBuilder } from './trigger-deletion-execution-options.builder';

describe(TriggerDeletionExecutionOptionsBuilder.name, () => {
describe(TriggerDeletionExecutionOptionsBuilder.build.name, () => {
describe('when called with proper arguments', () => {
const setup = () => {
const limit = 1000;

const expectedOutput: TriggerDeletionExecutionOptions = { limit };

return { limit, expectedOutput };
};

it('should return valid object with expected values', () => {
const { limit, expectedOutput } = setup();

const output = TriggerDeletionExecutionOptionsBuilder.build(limit);

expect(output).toStrictEqual(expectedOutput);
});
});
});
});
Loading

0 comments on commit af700bb

Please sign in to comment.