feat: resolve tiles-deletion storage locators from task params (MAPCO-11295) by almog8k · Pull Request #37 · MapColonies/cleaner

almog8k · 2026-08-06T13:24:05Z

Question	Answer
Bug fix	✖
New feature	✔
Breaking change	✔
Deprecations	✖
Documentation	✖
Tests added	✔
Chore	✔

Related issues: MAPCO-11295 (epic MAPCO-11261)

Further information:

Adapts the cleaner to the reshaped raster-shared deletion schemas and moves the tiles-deletion storage locator out of deployment config into the task parameters.

What changed

Locators come from the task. TilesDeletionStrategy takes the S3 bucket and the FS sub path from the task params instead of strategies.tilesDeletion.{s3Bucket,fsSubPath}, so a single deployment is no longer pinned to one bucket and one sub path. storageProvider replaces sourceProvider and tilesRelativePath replaces tilesPath, following the shared schemas.
FS path resolution belongs to the provider. An FS storage target is now a sub path of the configured base path. FsStorageProvider joins its own base path and rejects a sub path that falls outside the configured deletion sub paths, so the strategy holds no filesystem knowledge and reads only batchSize / concurrency from config.
Sub-path validation is shared and enforced. The rule moved to isPathWithinAllowedSubPaths in cleaner/utils/path.ts, used by both targetExists and deleteResources, and is now an assertion rather than a boolean whose result could be ignored. The thrown message names the offending paths, the base path and the allowed sub paths.
IStorageProvider.delete takes the storage target first, matching targetExists.
Dead config removed. strategies.tilesDeletion.s3Bucket and .fsSubPath, their configmap entries (TILES_DELETION_S3_BUCKET, TILES_DELETION_FS_SUB_PATH) and the s3.tilesBucket value that fed the former. storage.fs.subPaths stays — it is the allowlist FS deletion targets are checked against. cleanupStorageProviders now defaults to empty so a deployment states its providers explicitly.

The five commits are meant to be reviewed in order: dependency pointer, the feature, the validator extraction, the parameter reorder, then the config cleanup.

Breaking change

Producers of tiles-deletion and artifacts-deletion tasks must now send bucket (S3) or subPath (FS) in the task parameters, and the two strategies.tilesDeletion config keys above no longer exist. The feat commit is not marked !, so release-please will cut a minor — say the word if this should be a major instead.

Temporary pointer to a locally packed tarball while the reshaped deletion schemas are unreleased. Must be swapped for a published 8.3.0-alpha before merge: the tarball reports the same version as the published alpha, so a plain install on CI would resolve the older package instead.

…ms (MAPCO-11295) Tiles deletion read its storage locator from strategies.tilesDeletion.{s3Bucket,fsSubPath}, which pinned every task in a deployment to one bucket and one sub path. The reshaped raster-shared deletion schemas carry the locator in the task params, so each task now brings its own. - storageProvider replaces sourceProvider and tilesRelativePath replaces tilesPath, following the shared schemas - an FS storage target is now a sub path of the configured base path. FsStorageProvider joins its own base path and rejects a sub path that falls outside the configured deletion sub paths, so the strategy holds no filesystem knowledge at all and reads only its batching knobs from config - REDIS joined both shared unions but has no provider here yet, so tiles deletion rejects it as unrecoverable (MAPCO-11261) Producers of tiles-deletion and artifacts-deletion tasks must now send the bucket or subPath in the task parameters.

isPathWithinAllowedSubPaths holds the rule that a path must sit strictly under one of the configured sub paths and still resolve inside the base path, which makes it unit testable on its own rather than only through the provider. FsStorageProvider's private check becomes an assertion instead of a boolean, so a caller cannot forget to act on the result, and the thrown message now names the offending paths, the base path and the allowed sub paths.

delete(storageTarget, paths) now reads the same way as targetExists(storageTarget, relativePath).

strategies.tilesDeletion.s3Bucket and .fsSubPath are no longer read now that the locator travels with the task, so they go along with their configmap entries (TILES_DELETION_S3_BUCKET, TILES_DELETION_FS_SUB_PATH) and the s3.tilesBucket value that fed the former. storage.fs.subPaths stays: it is the allowlist FS deletion targets are checked against. cleanupStorageProviders now defaults to empty so a deployment states its providers explicitly.

github-actions · 2026-08-06T13:24:21Z

🎫 Related Jira Issue: MAPCO-11295

vitaligi · 2026-08-09T11:04:12Z

-   * @param paths
-   * @returns boolean whether `paths` are valid and pass all checks
+   * Gate for every path this provider is asked to touch.
+   * @param relativePaths - Paths relative to the configured base path, sub path included


in function def it is still called paths, use it or relativePaths

vitaligi · 2026-08-09T12:00:18Z

+      expect(strategy.validate(redisParams)).toEqual(redisParams);
+    });
+
+    it('should throw ValidationError when paths is an empty array', () => {


there was a test checking if paths failed on empty string
please add it as well

vitaligi · 2026-08-09T12:04:37Z

-const s3Params: DeleteStoredResourcesParams = { storageProvider: SourceType.S3, paths: ['layer1'], bucket: S3_BUCKET };
-const fsParams: DeleteStoredResourcesParams = { storageProvider: SourceType.FS, paths: ['layer2'], subPath: FS_SUB_PATH };
+const s3Params: S3DeleteStoredResourcesParams = { storageProvider: SourceType.S3, paths: ['layer1'], bucket: S3_BUCKET };
+const fsParams: FsDeleteStoredResourcesParams = { storageProvider: SourceType.FS, paths: ['layer2'], subPath: FS_SUB_PATH };


add redisParams here

vitaligi · 2026-08-09T12:06:01Z

-    it('should throw ValidationError when tilesPath is empty string', () => {
-      expect(() => strategy.validate({ storageProvider: SourceType.S3, catalogId: '' })).toThrow(ValidationError);
+    it('should validate and return REDIS params, whose prefix is the locator', () => {
+      const redisParams = { storageProvider: 'REDIS', prefix: 'layer-redis_WorldCRS84' };


reuse redisParams here
and move test closer to happy path of FS/S3 above

vitaligi · 2026-08-09T12:07:00Z

+    it('should throw ValidationError when the FS subPath is missing', () => {
+      expect(() => strategy.validate({ storageProvider: SourceType.FS, paths: ['layer1'] })).toThrow(ValidationError);
    });



add test if redis prefix is missing and ValidationError is thrown

vitaligi · 2026-08-09T12:20:59Z


-const { s3Bucket: S3_BUCKET, fsBasePath: FS_BASE_PATH, fsSubPath: FS_SUB_PATH } = TILES_DELETION_CONFIG_DEFAULTS;
+const S3_BUCKET = 'test-bucket';
+const FS_SUB_PATH = 'artifacts/tiles';


'artifacts/tiles' can be extracted from FS_STORAGE_CONFIG_DEFAULTS

vitaligi · 2026-08-09T12:27:39Z

-      it('should call S3 provider with s3Bucket as storage target', async () => {
-        await strategy.execute(s3Params);
+      it("should call S3 provider with the task's own bucket as storage target", async () => {
+        const params: S3TilesDeletionParams = { ...s3Params, bucket: 'per-task-bucket' };


why can't we keep using s3Params without modification

vitaligi · 2026-08-09T12:39:03Z

        expect(MockS3Provider.delete).not.toHaveBeenCalled();
      });

+      it('should pass the subPath through untouched rather than resolving it', async () => {


this case is not so different from the happy path

almog8k added 6 commits August 6, 2026 15:38

refactor(storage): order provider delete params target-first

76e036f

delete(storageTarget, paths) now reads the same way as targetExists(storageTarget, relativePath).

chore: merge master

dce4f34

mapcolonies-devops assigned almog8k Aug 6, 2026

fix: add resolveAbsolutePath utility to fsStorageProvider imports

a7ce00f

vitaligi requested changes Aug 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat: resolve tiles-deletion storage locators from task params (MAPCO-11295) - #37

feat: resolve tiles-deletion storage locators from task params (MAPCO-11295)#37
almog8k wants to merge 7 commits into
masterfrom
feat/tiles-deletion-params-MAPCO-11295

almog8k commented Aug 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 6, 2026 •

edited by atlassian Bot

Loading

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

vitaligi Aug 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

almog8k commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Breaking change

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

almog8k commented Aug 6, 2026 •

edited

Loading

github-actions Bot commented Aug 6, 2026 •

edited by atlassian Bot

Loading