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

config/custom-environment-variables.json

@@ -114,8 +114,6 @@
   },
   "strategies": {
     "tilesDeletion": {
-      "s3Bucket": "TILES_DELETION_S3_BUCKET",
-      "fsSubPath": "TILES_DELETION_FS_SUB_PATH",
       "batchSize": {
         "__name": "TILES_DELETION_BATCH_SIZE",
         "__format": "number"

config/default.json

@@ -105,8 +105,6 @@
   },
   "strategies": {
     "tilesDeletion": {
-      "s3Bucket": "",
-      "fsSubPath": "tiles",
       "batchSize": 1000,
       "concurrency": 10
     }

helm/templates/configmap.yaml

@@ -5,7 +5,6 @@
 {{- $fs := ($storage.fs) | default dict -}}
 {{- $internalPvc := (($fs).internalPvc) | default dict -}}
 {{- $fsBasePath := clean (printf "/%s" $internalPvc.mountPath) -}}
-{{- $fsTilesDeletionSubPath := clean (printf "%s" ($internalPvc.tilesSubPath | default "tiles")) -}}
 {{- if .Values.enabled -}}
 apiVersion: v1
 kind: ConfigMap
@@ -74,9 +73,7 @@ data:
     {{- end -}}
   {{- end }}
   FS_SUB_PATHS: {{ $subPaths | toJson | quote }}
-  TILES_DELETION_FS_SUB_PATH: {{ $fsTilesDeletionSubPath | quote }}
   {{- end }}
-  TILES_DELETION_S3_BUCKET: {{ $s3.tilesBucket | default "" | quote }}
   {{- with .Values.env.strategies.tilesDeletion }}
   TILES_DELETION_BATCH_SIZE: {{ .batchSize | default 1000 | quote }}
   TILES_DELETION_CONCURRENCY: {{ .concurrency | default 10 | quote }}

helm/values.yaml

@@ -13,9 +13,7 @@ serviceUrls:
   jobTracker: ""
 
 storage:
-  cleanupStorageProviders:
-    - FS
-    - S3
+  cleanupStorageProviders: {}
   s3:
     delete:
       batchSize: 1000
@@ -24,7 +22,6 @@ storage:
     secretName: ""
     sslEnabled: false
     region: ""
-    tilesBucket: ""
   fs:
     delete:
       batchSize: 1000

package.json

@@ -37,7 +37,7 @@
     "@map-colonies/js-logger": "^5.0.0",
     "@map-colonies/mc-priority-queue": "^9.1.0",
     "@map-colonies/mc-utils": "^5.1.0",
-    "@map-colonies/raster-shared": "^8.3.0",
+    "@map-colonies/raster-shared": "^9.0.0-alpha.0",
     "@map-colonies/read-pkg": "^1.0.0",
     "@map-colonies/schemas": "^1.20.0",
     "@map-colonies/telemetry": "^10.0.1",

src/cleaner/storageProviders/fsStorageProvider.ts

@@ -4,7 +4,7 @@ import type { Logger } from '@map-colonies/js-logger';
 import type { DeleteStoredResourcesParams } from '@map-colonies/raster-shared';
 import { inject, injectable } from 'tsyringe';
 import { mergeFailures, type DeleteFailure, type DeleteResult, type IStorageProvider, type StorageProvider } from '@src/cleaner/storageProviders';
-import { getChunk, normalizeFolderPath, resolveAbsolutePath } from '@src/cleaner/utils';
+import { getChunk, isPathWithinAllowedSubPaths, resolveAbsolutePath } from '@src/cleaner/utils';
 import { SERVICES } from '@common/constants';
 import { describeError, UnrecoverableError } from '../errors';
 import type { FsStorageConfig } from './storageConfig';
@@ -20,24 +20,36 @@ export class FsStorageProvider implements IStorageProvider<'FS'> {
     this.logger.debug({ msg: 'Loaded FS storage provider', basePath: this.fsConfig.basePath });
   }
 
-  public async targetExists(basePath: string, relativePath: string): Promise<boolean> {
-    this.logger.debug({ msg: 'Checking if target resource exists', basePath, path: relativePath });
+  /**
+   * @param subPath - Sub path of the configured base path, as supplied by the task
+   * @param relativePath - Path below `subPath` to check for
+   */
+  public async targetExists(subPath: string, relativePath: string): Promise<boolean> {
+    this.assertPathsValid([join(subPath, relativePath)]);
+
+    const targetPath = join(this.fsConfig.basePath, subPath, relativePath);
+    this.logger.debug({ msg: 'Checking if target resource exists', subPath, path: relativePath, targetPath });
     try {
-      await stat(join(basePath, relativePath));
+      await stat(targetPath);
       return true;
     } catch (err) {
       if ((err as NodeJS.ErrnoException).code === 'ENOENT') return false;
       throw err;
     }
   }
 
-  public async delete(paths: string[], basePath: string): Promise<DeleteResult> {
-    this.logger.debug({ msg: 'Deleting files from filesystem', basePath, pathsCount: paths.length });
+  /**
+   * @param subPath - Sub path of the configured base path, as supplied by the task
+   * @param paths - Paths relative to `subPath`
+   */
+  public async delete(subPath: string, paths: string[]): Promise<DeleteResult> {
+    this.logger.debug({ msg: 'Deleting files from filesystem', subPath, pathsCount: paths.length });
+    const targetPath = join(this.fsConfig.basePath, subPath);
     let failures: DeleteFailure = new Map();
 
     const results = await Promise.allSettled(
       paths.map(async (relativePath) => {
-        await unlink(join(basePath, relativePath));
+        await unlink(join(targetPath, relativePath));
       })
     );
 
@@ -47,14 +59,14 @@ export class FsStorageProvider implements IStorageProvider<'FS'> {
         const relativePath = paths[idx]!;
         const error: unknown = result.reason;
         const reason = describeError(error);
-        this.logger.debug({ msg: 'Failed to delete file', path: join(basePath, relativePath), reason, error });
+        this.logger.debug({ msg: 'Failed to delete file', path: join(targetPath, relativePath), reason, error });
         const chunkFailure = chunkFailures.get(reason);
         chunkFailures.set(reason, { count: (chunkFailure?.count ?? 0) + 1, sample: chunkFailure?.sample ?? relativePath });
       }
     }
     failures = mergeFailures({ source: chunkFailures, target: failures });
 
-    await this.cleanupEmptyDirs(paths, basePath);
+    await this.cleanupEmptyDirs(paths, targetPath);
 
     return { failures };
   }
@@ -68,11 +80,7 @@ export class FsStorageProvider implements IStorageProvider<'FS'> {
       totalFailedPathsCount = 0;
 
     const relativePaths = paths.map((path) => join(subPath, path));
-
-    if (!this.arePathsValid(relativePaths))
-      throw new UnrecoverableError(
-        'Cannot delete files/folders outside base path or subpath as well as base path or subpath itself. paths must also match a valid configured path.'
-      );
+    this.assertPathsValid(relativePaths);
 
     let failures: DeleteFailure = new Map();
 
@@ -115,22 +123,26 @@ export class FsStorageProvider implements IStorageProvider<'FS'> {
   }
 
   /**
-   * Preforms several checks on input `paths`.
-   * Includes a check for path traversal (i.e. accessing folders above root folder)
-   * @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
+   * @throws {UnrecoverableError} if any path fails the check; invalid paths are a producer
+   * bug and will not become valid on retry
    */
-  private arePathsValid(paths: string[]): boolean {
+  private assertPathsValid(paths: string[]): void {
     this.logger.debug({ msg: 'Checking paths validity', paths });
-    const badPaths = paths.filter((path) => {
-      const startsWithAllowedSubPath = this.fsConfig.subPaths.some((subPath) => path.startsWith(normalizeFolderPath(subPath)));
-      const absolutePath = resolveAbsolutePath(join(this.fsConfig.basePath, path));
-      const startsWithBasePath = absolutePath.startsWith(normalizeFolderPath(this.fsConfig.basePath));
-      return !(startsWithAllowedSubPath && startsWithBasePath);
-    });
-    const areValid = badPaths.length === 0;
-    this.logger.debug({ msg: `Paths validity check ${areValid ? 'succeeded' : 'failed'}`, ...(!areValid && { badPaths }) });
-    return areValid;
+    const badPaths = paths.filter(
+      (path) => !isPathWithinAllowedSubPaths({ relativePath: path, basePath: this.fsConfig.basePath, allowedSubPaths: this.fsConfig.subPaths })
+    );
+
+    if (badPaths.length > 0) {
+      const { basePath, subPaths } = this.fsConfig;
+      this.logger.error({ msg: 'Paths validity check failed', badPaths, basePath, allowedSubPaths: subPaths });
+      throw new UnrecoverableError(
+        `Cannot delete paths outside the configured sub paths (${subPaths.join(', ')}) of base path '${basePath}', or the sub paths themselves: ${badPaths.join(', ')}`
+      );
+    }
+
+    this.logger.debug({ msg: 'Paths validity check succeeded' });
   }
 
   // Attempts to remove any directories that became empty after deletion. `relativePaths`

src/cleaner/storageProviders/iStorageProvider.ts

@@ -15,12 +15,13 @@ export interface IStorageProvider<T extends StorageProvider = StorageProvider> {
   /**
    * Deletes a batch of relative file paths within the given storage target.
    * - S3:  storageTarget = bucket name; paths are object keys
-   * - FS:  storageTarget = base directory; full path = join(storageTarget, path)
+   * - FS:  storageTarget = sub path of the configured base path; the provider joins its own
+   *        base path and rejects anything falling outside the configured deletion sub paths
    *
    * Returns an object including delete failures aggregation with one entry per failed reason.
    * "Not found" is reported as a failure with additional metadata on failure - count and sample
    */
-  delete: (paths: string[], storageTarget: string) => Promise<DeleteResult>;
+  delete: (storageTarget: string, paths: string[]) => Promise<DeleteResult>;
 
   /**
    * Deletes ALL objects/files under the given paths.
@@ -32,11 +33,15 @@ export interface IStorageProvider<T extends StorageProvider = StorageProvider> {
   /**
    * Returns true if relativePath exists within storageTarget and contains data.
    * - S3:  storageTarget = bucket, relativePath = key prefix — lists objects (KeyCount > 0)
-   * - FS:  storageTarget = base directory, relativePath = subdirectory — checks fs.access
+   * - FS:  storageTarget = sub path of the configured base path, relativePath = subdirectory
+   *        below it — checks fs.stat, subject to the same sub path validation as `delete`
    */
   targetExists: (storageTarget: string, relativePath: string) => Promise<boolean>;
 }
 
 export type StorageProviders = {
   [T in StorageProvider]?: IStorageProvider<T>;
 };
+
+/** A provider actually registered in the map. */
+export type ResolvedStorageProvider = NonNullable<StorageProviders[StorageProvider]>;

src/cleaner/storageProviders/s3StorageProvider.ts

@@ -45,7 +45,7 @@ export class S3StorageProvider implements IStorageProvider<S3StorageProviderType
     this.logger.debug({ msg: 'Loaded S3 storage provider', endpoint: s3Config.endpoint, batchSize: this.s3Config.batchSize });
   }
 
-  public async delete(paths: string[], bucket: string): Promise<DeleteResult> {
+  public async delete(bucket: string, paths: string[]): Promise<DeleteResult> {
     this.logger.debug({ msg: 'Deleting objects from S3', bucket, pathsCount: paths.length });
     let failures: DeleteFailure = new Map();
 

src/cleaner/strategies/deleteStoredResourcesStrategy.ts

@@ -1,9 +1,9 @@
 import type { Logger } from '@map-colonies/js-logger';
-import { deleteStoredResourcesParamsSchema, type DeleteStoredResourcesParams } from '@map-colonies/raster-shared';
+import { deleteStoredResourcesParamsSchema, DeleteStoredResourcesParams, StorageProvider } from '@map-colonies/raster-shared';
 import { inject, injectable } from 'tsyringe';
 import type { ConfigType } from '@common/config';
 import { SERVICES } from '@common/constants';
-import { summarizeDeleteFailures, type IStorageProvider, type StorageProvider, type StorageProviders } from '@src/cleaner/storageProviders';
+import { summarizeDeleteFailures, type IStorageProvider, type StorageProviders } from '@src/cleaner/storageProviders';
 import { RecoverableError, UnrecoverableError } from '../errors';
 import { validateSchema } from '../utils';
 import type { ITaskStrategy } from './taskStrategy';
@@ -22,16 +22,17 @@ export class DeleteStoredResourcesStrategy implements ITaskStrategy<DeleteStored
   }
 
   public async execute(params: DeleteStoredResourcesParams): Promise<void> {
-    const { paths } = params;
-    const provider = this.resolveStorageProvider(params);
+    const paths = params.storageProvider === StorageProvider.REDIS ? [] : params.paths;
+    const provider = this.resolveStorageProvider(params.storageProvider);
 
     this.logger.info({
       msg: 'Starting deletion',
       count: paths.length,
       paths,
       provider: params.storageProvider,
-      ...(params.storageProvider === 'S3' && { bucket: params.bucket }),
-      ...(params.storageProvider === 'FS' && { subPath: params.subPath }),
+      ...(params.storageProvider === StorageProvider.S3 && { bucket: params.bucket }),
+      ...(params.storageProvider === StorageProvider.FS && { subPath: params.subPath }),
+      ...(params.storageProvider === StorageProvider.REDIS && { prefix: params.prefix }),
     });
 
     const { failures } = await provider.deleteResources(params);
@@ -56,14 +57,11 @@ export class DeleteStoredResourcesStrategy implements ITaskStrategy<DeleteStored
     });
   }
 
-  private resolveStorageProvider<K extends StorageProvider>(
-    params: Extract<DeleteStoredResourcesParams, { storageProvider: K }>
-  ): IStorageProvider<K> {
-    if (!(params.storageProvider in this.storageProviders)) throw new UnrecoverableError(`Unsupported storage provider ${params.storageProvider}`);
-    // eslint-disable-next-line @typescript-eslint/naming-convention
-    const storageProvider = this.storageProviders[params.storageProvider];
-    if (storageProvider === undefined) throw new UnrecoverableError(`Unsupported storage provider ${params.storageProvider}`);
-    this.logger.debug({ msg: `Using ${params.storageProvider} provider` });
-    return storageProvider;
+  private resolveStorageProvider<K extends StorageProvider>(storageProvider: K): IStorageProvider<K> {
+    this.logger.debug({ msg: `Resolving storage provider`, provider: storageProvider, providers: Object.keys(this.storageProviders) });
+    const provider = this.storageProviders[storageProvider];
+    if (provider === undefined) throw new UnrecoverableError(`Unsupported storage provider ${storageProvider}`);
+    this.logger.debug({ msg: `Using ${storageProvider} provider` });
+    return provider;
   }
 }