From 6224974e81e764d941ea63943ff2a23a3508ccd8 Mon Sep 17 00:00:00 2001 From: almog8k Date: Thu, 30 Jul 2026 17:27:22 +0300 Subject: [PATCH 1/2] feat: add forbidden layers configuration for deletion (MAPCO-11281) Add a configurable safety list of layers that are protected from deletion. Layers are identified by their map serving name (-) and configured via the FORBIDDEN_LAYERS_FOR_DELETION env var, settable at Helm values level under env.deleteLayer.forbiddenLayers. deleteLayer now validates the requested layer against this list before any other check and throws a ConflictError explaining that the layer cannot be deleted. --- config/custom-environment-variables.json | 6 +++ config/default.json | 3 ++ helm/templates/configmap.yaml | 1 + helm/values.yaml | 4 ++ src/ingestion/models/ingestionManager.ts | 19 ++++++++ tests/integration/ingestion/ingestion.spec.ts | 15 ++++++ tests/mocks/configMock.ts | 3 ++ tests/mocks/mockFactory.ts | 10 ++++ .../ingestion/models/ingestionManager.spec.ts | 48 ++++++++++++++++++- 9 files changed, 108 insertions(+), 1 deletion(-) diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 7c3b8ac..5a96fd8 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -107,6 +107,12 @@ "__format": "json" } }, + "deleteLayer": { + "forbiddenLayers": { + "__name": "FORBIDDEN_LAYERS_FOR_DELETION", + "__format": "json" + } + }, "httpRetry": { "attempts": { "__name": "HTTP_RETRY_ATTEMPTS", diff --git a/config/default.json b/config/default.json index 606d920..9a1bcb8 100644 --- a/config/default.json +++ b/config/default.json @@ -72,6 +72,9 @@ ], "forbiddenJobTypesForParallelIngestion": ["Ingestion_New", "Ingestion_Update", "Ingestion_Swap_Update", "Delete_Layer"] }, + "deleteLayer": { + "forbiddenLayers": [] + }, "httpRetry": { "attempts": 5, "delay": "exponential", diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 856e4d1..57123b7 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -29,6 +29,7 @@ data: STORAGE_EXPLORER_VALID_FILE_EXTENSIONS: {{ .Values.env.storageExplorer.validFileExtensions | toJson | quote }} FORBIDDEN_TYPES_FOR_PARALLEL_INGESTION: {{ .Values.env.forbiddenJobTypesForParallelIngestion | toJson | quote }} SUPPORTED_INGESTION_SWAP_TYPES: {{ .Values.env.supportedIngestionSwapTypes | toJson | quote }} + FORBIDDEN_LAYERS_FOR_DELETION: {{ .Values.env.deleteLayer.forbiddenLayers | toJson | quote }} HTTP_RETRY_ATTEMPTS: {{ .Values.env.httpRetry.attempts | quote }} HTTP_RETRY_DELAY: {{ .Values.env.httpRetry.delay | quote }} HTTP_RETRY_RESET_TIMEOUT: {{ .Values.env.httpRetry.resetTimeout | quote }} diff --git a/helm/values.yaml b/helm/values.yaml index d4bbaf2..1823f24 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -174,6 +174,10 @@ env: - Ingestion_New - Ingestion_Update - Ingestion_Swap_Update + deleteLayer: + # layers that are protected from deletion, in the format of - + forbiddenLayers: [] + # - 'VIVID_IHUD-Orthophoto' resources: enabled: false diff --git a/src/ingestion/models/ingestionManager.ts b/src/ingestion/models/ingestionManager.ts index aaf6a65..2b40d89 100644 --- a/src/ingestion/models/ingestionManager.ts +++ b/src/ingestion/models/ingestionManager.ts @@ -24,6 +24,7 @@ import { type IngestionUpdateJobParams, type IngestionValidationTaskParams, type InputFiles, + type LayerName, type RasterProductTypes, } from '@map-colonies/raster-shared'; import { withSpanAsyncV4, withSpanV4 } from '@map-colonies/telemetry'; @@ -80,6 +81,7 @@ export class IngestionManager { private readonly validationTaskType: string; private readonly finalizeTaskType: string; private readonly deleteTaskType: string; + private readonly forbiddenLayersForDeletion: LayerName[]; private readonly sourceMount: string; private readonly jobTrackerServiceUrl: string; @@ -113,6 +115,7 @@ export class IngestionManager { this.validationTaskType = config.get('jobManager.validationTaskType') as unknown as string; this.finalizeTaskType = config.get('jobManager.finalizeTaskType') as unknown as string; this.deleteTaskType = config.get('jobManager.deleteTaskType') as unknown as string; + this.forbiddenLayersForDeletion = config.get('deleteLayer.forbiddenLayers') as unknown as LayerName[]; this.sourceMount = config.get('storageExplorer.layerSourceDir') as unknown as string; this.jobTrackerServiceUrl = config.get('services.jobTrackerServiceURL') as unknown as string; } @@ -218,6 +221,7 @@ export class IngestionManager { activeSpan?.updateName('ingestionManager.deleteLayer'); const rasterLayerMetadata = await this.getLayerMetadata(catalogId); + this.validateLayerIsNotForbiddenForDeletion(rasterLayerMetadata); this.validateLayerIsUnpublished(rasterLayerMetadata); await this.validateNoParallelJobs(rasterLayerMetadata.productId, rasterLayerMetadata.productType); @@ -707,6 +711,21 @@ export class IngestionManager { return createJobRequest; } + @withSpanV4 + private validateLayerIsNotForbiddenForDeletion(rasterLayerMetadata: RasterLayerMetadata): void { + const logCtx: LogContext = { ...this.logContext, function: this.validateLayerIsNotForbiddenForDeletion.name }; + const { productId, productType } = rasterLayerMetadata; + const layerName = getMapServingLayerName(productId, productType); + + if (this.forbiddenLayersForDeletion.includes(layerName)) { + const message = `Layer: ${layerName}, is configured as a forbidden layer for deletion and therefore cannot be deleted`; + this.logger.error({ msg: message, logContext: logCtx, layerName }); + const error = new ConflictError(message); + trace.getActiveSpan()?.setAttribute('exception.type', error.status); + throw error; + } + } + @withSpanV4 private validateLayerIsUnpublished(rasterLayerMetadata: RasterLayerMetadata): void { if (rasterLayerMetadata.productStatus !== RecordStatus.UNPUBLISHED) { diff --git a/tests/integration/ingestion/ingestion.spec.ts b/tests/integration/ingestion/ingestion.spec.ts index 7e41de8..c31c7a3 100644 --- a/tests/integration/ingestion/ingestion.spec.ts +++ b/tests/integration/ingestion/ingestion.spec.ts @@ -27,6 +27,7 @@ import { createUpdateLayerRequest, generateCallbackUrl, generateMockJob, + getForbiddenLayerForDeletion, rasterLayerInputFilesGenerators, rasterLayerMetadataGenerators, } from '../../mocks/mockFactory'; @@ -2877,6 +2878,20 @@ describe('Ingestion', () => { } ); + it('should return 409 status code when the layer is configured as forbidden for deletion', async () => { + const approver = faker.person.fullName(); + const { productId, productType } = getForbiddenLayerForDeletion(); + const catalogLayerResponse = createCatalogLayerResponse({ metadata: { productId, productType, productStatus: RecordStatus.UNPUBLISHED } }); + const scope = nock(jobManagerURL).post('/jobs').reply(httpStatusCodes.OK, jobResponse); + nock(catalogServiceURL).post('/records/find', { id: catalogLayerResponse.metadata.id }).reply(httpStatusCodes.OK, [catalogLayerResponse]); + + const response = await requestSender.deleteLayer(catalogLayerResponse.metadata.id, { approver }); + + expect(response).toSatisfyApiSpec(); + expect(response.status).toBe(httpStatusCodes.CONFLICT); + expect(scope.isDone()).toBe(false); + }); + it('should return 409 status code when there are conflicting jobs', async () => { const approver = faker.person.fullName(); const catalogLayerResponse = createCatalogLayerResponse({ metadata: { productStatus: RecordStatus.UNPUBLISHED } }); diff --git a/tests/mocks/configMock.ts b/tests/mocks/configMock.ts index 5d1de24..7246fa6 100644 --- a/tests/mocks/configMock.ts +++ b/tests/mocks/configMock.ts @@ -105,6 +105,9 @@ const registerDefaultConfig = (): void => { ], forbiddenJobTypesForParallelIngestion: ['Ingestion_New', 'Ingestion_Update', 'Delete_Layer'], }, + deleteLayer: { + forbiddenLayers: ['VIVID_IHUD-Orthophoto'], + }, }; setConfigValues(config); diff --git a/tests/mocks/mockFactory.ts b/tests/mocks/mockFactory.ts index 4c2583c..1c44553 100644 --- a/tests/mocks/mockFactory.ts +++ b/tests/mocks/mockFactory.ts @@ -17,6 +17,7 @@ import { type IngestionSwapUpdateJobParams, type IngestionUpdateJobParams, type InputFiles, + type LayerName, type NewRasterLayerMetadata, type UpdateRasterLayerMetadata, JobTypes, @@ -482,6 +483,15 @@ export const createCatalogLayerResponse = (rasterLayerCatalog?: DeepPartial { + const forbiddenLayerName = configMock.get('deleteLayer.forbiddenLayers')[0]!; + const [productId, productType] = forbiddenLayerName.split('-') as [string, RasterProductTypes]; + return { forbiddenLayerName, productId, productType }; +}; + export const createFindJobsParams = (findJobsParams: IFindJobsByCriteriaBody): IFindJobsByCriteriaBody => { const defaultFindJobsParams = { isCleaned: false, diff --git a/tests/unit/ingestion/models/ingestionManager.spec.ts b/tests/unit/ingestion/models/ingestionManager.spec.ts index e9683ea..6a0b4f5 100644 --- a/tests/unit/ingestion/models/ingestionManager.spec.ts +++ b/tests/unit/ingestion/models/ingestionManager.spec.ts @@ -2,7 +2,7 @@ import { faker } from '@faker-js/faker'; import { BadRequestError, ConflictError, NotFoundError } from '@map-colonies/error-types'; import { jsLogger } from '@map-colonies/js-logger'; import { ICreateJobResponse, OperationStatus } from '@map-colonies/mc-priority-queue'; -import { getMapServingLayerName } from '@map-colonies/raster-shared'; +import { getMapServingLayerName, RasterProductTypes } from '@map-colonies/raster-shared'; import { RecordStatus } from '@map-colonies/types'; import { trace } from '@opentelemetry/api'; import { container } from 'tsyringe'; @@ -27,6 +27,7 @@ import { generateMockJob, generateNewLayerRequest, generateUpdateLayerRequest, + getForbiddenLayerForDeletion, } from '../../../mocks/mockFactory'; import { ChecksumProcessor } from '../../../../src/utils/hash/interfaces'; import { CHECKSUM_PROCESSOR } from '../../../../src/utils/hash/constants'; @@ -680,6 +681,51 @@ describe('IngestionManager', () => { expect(createIngestionJobSpy).not.toHaveBeenCalled(); }); + it('should throw conflict error when the layer is configured as forbidden for deletion', async () => { + const approver = faker.person.fullName(); + const { forbiddenLayerName, productId, productType } = getForbiddenLayerForDeletion(); + const catalogLayerResponse = createCatalogLayerResponse({ metadata: { productId, productType, productStatus: RecordStatus.UNPUBLISHED } }); + const expectedErrorMessage = `Layer: ${forbiddenLayerName}, is configured as a forbidden layer for deletion and therefore cannot be deleted`; + findByIdSpy.mockResolvedValue([catalogLayerResponse]); + + const promise = ingestionManager.deleteLayer(catalogLayerResponse.metadata.id, { approver }); + + await expect(promise).rejects.toThrow(new ConflictError(expectedErrorMessage)); + expect(createIngestionJobSpy).not.toHaveBeenCalled(); + }); + + it('should create delete layer job when only the productId matches a forbidden layer', async () => { + const approver = faker.person.fullName(); + const { productId } = getForbiddenLayerForDeletion(); + const catalogLayerResponse = createCatalogLayerResponse({ + metadata: { productId, productType: RasterProductTypes.RASTER_MAP, productStatus: RecordStatus.UNPUBLISHED }, + }); + const createJobResponse: ICreateJobResponse = { id: faker.string.uuid(), taskIds: [faker.string.uuid()] }; + findByIdSpy.mockResolvedValue([catalogLayerResponse]); + findJobsSpy.mockResolvedValue([]); + createIngestionJobSpy.mockResolvedValue(createJobResponse); + + const response = await ingestionManager.deleteLayer(catalogLayerResponse.metadata.id, { approver }); + + expect(response).toStrictEqual({ jobId: createJobResponse.id, taskId: createJobResponse.taskIds[0] }); + }); + + it('should create delete layer job when only the productType matches a forbidden layer', async () => { + const approver = faker.person.fullName(); + const { productType } = getForbiddenLayerForDeletion(); + const catalogLayerResponse = createCatalogLayerResponse({ + metadata: { productType, productStatus: RecordStatus.UNPUBLISHED }, + }); + const createJobResponse: ICreateJobResponse = { id: faker.string.uuid(), taskIds: [faker.string.uuid()] }; + findByIdSpy.mockResolvedValue([catalogLayerResponse]); + findJobsSpy.mockResolvedValue([]); + createIngestionJobSpy.mockResolvedValue(createJobResponse); + + const response = await ingestionManager.deleteLayer(catalogLayerResponse.metadata.id, { approver }); + + expect(response).toStrictEqual({ jobId: createJobResponse.id, taskId: createJobResponse.taskIds[0] }); + }); + it('should throw an error when job manager create delete layer job call throws an error', async () => { const approver = faker.person.fullName(); const catalogLayerResponse = createCatalogLayerResponse({ metadata: { productStatus: RecordStatus.UNPUBLISHED } }); From 8ab37b4a7d245c0ecc46d11fc5f0529076da6cc2 Mon Sep 17 00:00:00 2001 From: almog8k Date: Thu, 30 Jul 2026 17:41:46 +0300 Subject: [PATCH 2/2] feat: return 403 instead of 409 for forbidden layer deletion (MAPCO-11281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ForbiddenError better expresses that the layer is configured as protected than ConflictError does — the request does not conflict with any state, it is simply not permitted. Map ForbiddenError to 403 in the deleteLayer controller handler and document the 403 response on the delete endpoint in the OpenAPI spec. --- openapi3.yaml | 7 +++++++ src/ingestion/controllers/ingestionController.ts | 6 ++++-- src/ingestion/models/ingestionManager.ts | 4 ++-- tests/integration/ingestion/ingestion.spec.ts | 4 ++-- tests/unit/ingestion/models/ingestionManager.spec.ts | 6 +++--- 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/openapi3.yaml b/openapi3.yaml index 64dc7b4..4fa5ef9 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -153,6 +153,13 @@ paths: schema: $ref: >- ./Schema/ingestionTrigger/responses/ingestionTriggerResponses.yaml#/components/schemas/errorMessage + '403': + description: Forbidden + content: + application/json: + schema: + $ref: >- + ./Schema/ingestionTrigger/responses/ingestionTriggerResponses.yaml#/components/schemas/errorMessage '404': description: Not Found content: diff --git a/src/ingestion/controllers/ingestionController.ts b/src/ingestion/controllers/ingestionController.ts index 3838869..98f4b96 100644 --- a/src/ingestion/controllers/ingestionController.ts +++ b/src/ingestion/controllers/ingestionController.ts @@ -1,4 +1,4 @@ -import { BadRequestError, ConflictError, NotFoundError } from '@map-colonies/error-types'; +import { BadRequestError, ConflictError, ForbiddenError, NotFoundError } from '@map-colonies/error-types'; import type { RequestHandler } from 'express'; import { HttpError } from 'express-openapi-validator/dist/framework/types'; import { StatusCodes } from 'http-status-codes'; @@ -124,7 +124,9 @@ export class IngestionController { res.status(StatusCodes.OK).send(response); } catch (error) { - if (error instanceof NotFoundError) { + if (error instanceof ForbiddenError) { + (error as HttpError).status = StatusCodes.FORBIDDEN; //403 + } else if (error instanceof NotFoundError) { (error as HttpError).status = StatusCodes.NOT_FOUND; //404 } else if (error instanceof ConflictError) { (error as HttpError).status = StatusCodes.CONFLICT; //409 diff --git a/src/ingestion/models/ingestionManager.ts b/src/ingestion/models/ingestionManager.ts index 2b40d89..fc6b135 100644 --- a/src/ingestion/models/ingestionManager.ts +++ b/src/ingestion/models/ingestionManager.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'node:crypto'; import { relative } from 'node:path'; -import { ConflictError, NotFoundError } from '@map-colonies/error-types'; +import { ConflictError, ForbiddenError, NotFoundError } from '@map-colonies/error-types'; import type { Logger } from '@map-colonies/js-logger'; import { type IFindJobsByCriteriaBody, @@ -720,7 +720,7 @@ export class IngestionManager { if (this.forbiddenLayersForDeletion.includes(layerName)) { const message = `Layer: ${layerName}, is configured as a forbidden layer for deletion and therefore cannot be deleted`; this.logger.error({ msg: message, logContext: logCtx, layerName }); - const error = new ConflictError(message); + const error = new ForbiddenError(message); trace.getActiveSpan()?.setAttribute('exception.type', error.status); throw error; } diff --git a/tests/integration/ingestion/ingestion.spec.ts b/tests/integration/ingestion/ingestion.spec.ts index c31c7a3..245fead 100644 --- a/tests/integration/ingestion/ingestion.spec.ts +++ b/tests/integration/ingestion/ingestion.spec.ts @@ -2878,7 +2878,7 @@ describe('Ingestion', () => { } ); - it('should return 409 status code when the layer is configured as forbidden for deletion', async () => { + it('should return 403 status code when the layer is configured as forbidden for deletion', async () => { const approver = faker.person.fullName(); const { productId, productType } = getForbiddenLayerForDeletion(); const catalogLayerResponse = createCatalogLayerResponse({ metadata: { productId, productType, productStatus: RecordStatus.UNPUBLISHED } }); @@ -2888,7 +2888,7 @@ describe('Ingestion', () => { const response = await requestSender.deleteLayer(catalogLayerResponse.metadata.id, { approver }); expect(response).toSatisfyApiSpec(); - expect(response.status).toBe(httpStatusCodes.CONFLICT); + expect(response.status).toBe(httpStatusCodes.FORBIDDEN); expect(scope.isDone()).toBe(false); }); diff --git a/tests/unit/ingestion/models/ingestionManager.spec.ts b/tests/unit/ingestion/models/ingestionManager.spec.ts index 6a0b4f5..3aee6ae 100644 --- a/tests/unit/ingestion/models/ingestionManager.spec.ts +++ b/tests/unit/ingestion/models/ingestionManager.spec.ts @@ -1,5 +1,5 @@ import { faker } from '@faker-js/faker'; -import { BadRequestError, ConflictError, NotFoundError } from '@map-colonies/error-types'; +import { BadRequestError, ConflictError, ForbiddenError, NotFoundError } from '@map-colonies/error-types'; import { jsLogger } from '@map-colonies/js-logger'; import { ICreateJobResponse, OperationStatus } from '@map-colonies/mc-priority-queue'; import { getMapServingLayerName, RasterProductTypes } from '@map-colonies/raster-shared'; @@ -681,7 +681,7 @@ describe('IngestionManager', () => { expect(createIngestionJobSpy).not.toHaveBeenCalled(); }); - it('should throw conflict error when the layer is configured as forbidden for deletion', async () => { + it('should throw forbidden error when the layer is configured as forbidden for deletion', async () => { const approver = faker.person.fullName(); const { forbiddenLayerName, productId, productType } = getForbiddenLayerForDeletion(); const catalogLayerResponse = createCatalogLayerResponse({ metadata: { productId, productType, productStatus: RecordStatus.UNPUBLISHED } }); @@ -690,7 +690,7 @@ describe('IngestionManager', () => { const promise = ingestionManager.deleteLayer(catalogLayerResponse.metadata.id, { approver }); - await expect(promise).rejects.toThrow(new ConflictError(expectedErrorMessage)); + await expect(promise).rejects.toThrow(new ForbiddenError(expectedErrorMessage)); expect(createIngestionJobSpy).not.toHaveBeenCalled(); });