From 9abd5a193b07ef634029e2d709bca4249c9f0f1e Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 29 Nov 2024 16:54:46 -0300 Subject: [PATCH 1/5] refactor(maven): Unified result type for s3 protocol fetch --- lib/modules/datasource/maven/s3.spec.ts | 15 +-- lib/modules/datasource/maven/types.ts | 28 +++++ lib/modules/datasource/maven/util.spec.ts | 8 +- lib/modules/datasource/maven/util.ts | 135 ++++++++++++++++------ 4 files changed, 137 insertions(+), 49 deletions(-) diff --git a/lib/modules/datasource/maven/s3.spec.ts b/lib/modules/datasource/maven/s3.spec.ts index 8ec6e5a316463e..1e1ae4df70ae23 100644 --- a/lib/modules/datasource/maven/s3.spec.ts +++ b/lib/modules/datasource/maven/s3.spec.ts @@ -89,7 +89,7 @@ describe('modules/datasource/maven/s3', () => { { failedUrl: 's3://repobucket/org/example/package/maven-metadata.xml', }, - 'Dependency lookup authorization failed. Please correct AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY env vars', + 'Maven S3 lookup error: credentials provider error, check "AWS_ACCESS_KEY_ID" and "AWS_SECRET_ACCESS_KEY" variables', ); }); @@ -108,7 +108,7 @@ describe('modules/datasource/maven/s3', () => { { failedUrl: 's3://repobucket/org/example/package/maven-metadata.xml', }, - 'Dependency lookup failed. Please a correct AWS_REGION env var', + 'Maven S3 lookup error: missing region, check "AWS_REGION" variable', ); }); @@ -127,7 +127,7 @@ describe('modules/datasource/maven/s3', () => { { failedUrl: 's3://repobucket/org/example/package/maven-metadata.xml', }, - 'S3 url not found', + 'Maven S3 lookup error: object not found', ); }); @@ -146,7 +146,7 @@ describe('modules/datasource/maven/s3', () => { { failedUrl: 's3://repobucket/org/example/package/maven-metadata.xml', }, - 'S3 url not found', + 'Maven S3 lookup error: object not found', ); }); @@ -163,10 +163,10 @@ describe('modules/datasource/maven/s3', () => { expect(res).toBeNull(); expect(logger.debug).toHaveBeenCalledWith( { + err: expect.objectContaining({ message: 'Unknown error' }), failedUrl: 's3://repobucket/org/example/package/maven-metadata.xml', - message: 'Unknown error', }, - 'Unknown S3 download error', + 'Maven S3 lookup error: unknown error', ); }); @@ -178,9 +178,6 @@ describe('modules/datasource/maven/s3', () => { }) .resolvesOnce({}); expect(await get('org.example:package', baseUrlS3)).toBeNull(); - expect(logger.debug).toHaveBeenCalledWith( - "Expecting Readable response type got 'undefined' type instead", - ); }); }); }); diff --git a/lib/modules/datasource/maven/types.ts b/lib/modules/datasource/maven/types.ts index 7698e49fbc6663..853326887eaadc 100644 --- a/lib/modules/datasource/maven/types.ts +++ b/lib/modules/datasource/maven/types.ts @@ -1,4 +1,5 @@ import type { XmlDocument } from 'xmldoc'; +import type { Result } from '../../../util/result'; import type { ReleaseResult } from '../types'; export interface MavenDependency { @@ -19,3 +20,30 @@ export type DependencyInfo = Pick< ReleaseResult, 'homepage' | 'sourceUrl' | 'packageScope' >; + +export interface MavenFetchSuccess { + isCacheable?: boolean; + lastModified?: string; + data: T; +} + +export type MavenFetchError = + | { type: 'invalid-url' } + | { type: 'host-disabled' } + | { type: 'not-found' } + | { type: 'host-error' } + | { type: 'permission-issue' } + | { type: 'temporary-error' } + | { type: 'maven-central-temporary-error'; err: Error } + | { type: 'connection-error' } + | { type: 'unsupported-host' } + | { type: 'unsupported-format' } + | { type: 'unsupported-protocol' } + | { type: 'credentials-error' } + | { type: 'missing-aws-region' } + | { type: 'unknown'; err: Error }; + +export type MavenFetchResult = Result< + MavenFetchSuccess, + MavenFetchError +>; diff --git a/lib/modules/datasource/maven/util.spec.ts b/lib/modules/datasource/maven/util.spec.ts index fc42f24610b2b7..92ec1a52146214 100644 --- a/lib/modules/datasource/maven/util.spec.ts +++ b/lib/modules/datasource/maven/util.spec.ts @@ -8,6 +8,7 @@ import { downloadMavenXml, downloadS3Protocol, } from './util'; +import { MavenFetchError } from './types'; const http = new Http('test'); @@ -55,9 +56,12 @@ describe('modules/datasource/maven/util', () => { }); describe('downloadS3Protocol', () => { - it('returns null for non-S3 URLs', async () => { + it('fails for non-S3 URLs', async () => { const res = await downloadS3Protocol(new URL('http://not-s3.com/')); - expect(res).toBeNull(); + expect(res.unwrap()).toEqual({ + ok: false, + err: { type: 'invalid-url' } satisfies MavenFetchError, + }); }); }); diff --git a/lib/modules/datasource/maven/util.ts b/lib/modules/datasource/maven/util.ts index b8db9f298c0836..cc9765c06a5bad 100644 --- a/lib/modules/datasource/maven/util.ts +++ b/lib/modules/datasource/maven/util.ts @@ -20,6 +20,8 @@ import type { DependencyInfo, HttpResourceCheckResult, MavenDependency, + MavenFetchResult, + MavenFetchSuccess, MavenXml, } from './types'; @@ -142,42 +144,97 @@ function isS3NotFound(err: Error): boolean { return err.message === 'NotFound' || err.message === 'NoSuchKey'; } -export async function downloadS3Protocol(pkgUrl: URL): Promise { +export async function downloadS3Protocol( + pkgUrl: URL, +): Promise { logger.trace({ url: pkgUrl.toString() }, `Attempting to load S3 dependency`); - try { - const s3Url = parseS3Url(pkgUrl); - if (s3Url === null) { - return null; - } - const { Body: res } = await getS3Client().send(new GetObjectCommand(s3Url)); - if (res instanceof Readable) { - return streamToString(res); - } - logger.debug( - `Expecting Readable response type got '${typeof res}' type instead`, - ); - } catch (err) { - const failedUrl = pkgUrl.toString(); - if (err.name === 'CredentialsProviderError') { - logger.debug( - { failedUrl }, - 'Dependency lookup authorization failed. Please correct AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY env vars', - ); - } else if (err.message === 'Region is missing') { - logger.debug( - { failedUrl }, - 'Dependency lookup failed. Please a correct AWS_REGION env var', - ); - } else if (isS3NotFound(err)) { - logger.trace({ failedUrl }, `S3 url not found`); - } else { - logger.debug( - { failedUrl, message: err.message }, - 'Unknown S3 download error', - ); - } + + const s3Url = parseS3Url(pkgUrl); + if (!s3Url) { + return Result.err({ type: 'invalid-url' }); } - return null; + + return await Result.wrap(() => { + const command = new GetObjectCommand(s3Url); + const client = getS3Client(); + return client.send(command); + }) + .transform( + async ({ + Body, + LastModified, + DeleteMarker, + }): Promise => { + if (DeleteMarker) { + return Result.err({ type: 'not-found' }); + } + + if (!(Body instanceof Readable)) { + return Result.err({ type: 'unsupported-format' }); + } + + const data = await streamToString(Body); + const result: MavenFetchSuccess = { data }; + + const lastModified = normalizeDate(LastModified); + if (lastModified) { + result.lastModified = lastModified; + } + + return Result.ok(result); + }, + ) + .catch((err): MavenFetchResult => { + if (!(err instanceof Error)) { + return Result.err(err); + } + + if (err.name === 'CredentialsProviderError') { + return Result.err({ type: 'credentials-error' }); + } + + if (err.message === 'Region is missing') { + return Result.err({ type: 'missing-aws-region' }); + } + + if (isS3NotFound(err)) { + return Result.err({ type: 'not-found' }); + } + + return Result.err({ type: 'unknown', err }); + }) + .onError((err) => { + const failedUrl = pkgUrl.toString(); + + if (err.type === 'credentials-error') { + logger.debug( + { failedUrl }, + 'Maven S3 lookup error: credentials provider error, check "AWS_ACCESS_KEY_ID" and "AWS_SECRET_ACCESS_KEY" variables', + ); + return; + } + + if (err.type === 'missing-aws-region') { + logger.debug( + { failedUrl }, + 'Maven S3 lookup error: missing region, check "AWS_REGION" variable', + ); + return; + } + + if (err.type === 'not-found') { + logger.trace({ failedUrl }, 'Maven S3 lookup error: object not found'); + return; + } + + if (err.type === 'unknown') { + logger.debug( + { failedUrl, err: err.err }, + 'Maven S3 lookup error: unknown error', + ); + return; + } + }); } export async function downloadArtifactRegistryProtocol( @@ -334,10 +391,12 @@ export async function downloadMavenXml( } if (protocol === 's3:') { - const res = await downloadS3Protocol(pkgUrl); - if (res) { - return { xml: new XmlDocument(res) }; - } + const rawResult = await downloadS3Protocol(pkgUrl); + const xmlResult = rawResult.transform(({ isCacheable, data }): MavenXml => { + const xml = new XmlDocument(data); + return { xml }; + }); + return xmlResult.unwrapOr({}); } logger.debug( From 10e378061f8bd9b0e9e6213fd55d46ae558e7f00 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 29 Nov 2024 17:00:37 -0300 Subject: [PATCH 2/5] Fix eslint --- lib/modules/datasource/maven/util.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/datasource/maven/util.spec.ts b/lib/modules/datasource/maven/util.spec.ts index 92ec1a52146214..0921b7de58be6b 100644 --- a/lib/modules/datasource/maven/util.spec.ts +++ b/lib/modules/datasource/maven/util.spec.ts @@ -2,13 +2,13 @@ import type Request from 'got/dist/source/core'; import { partial } from '../../../../test/util'; import { HOST_DISABLED } from '../../../constants/error-messages'; import { Http, HttpError } from '../../../util/http'; +import type { MavenFetchError } from './types'; import { checkResource, downloadHttpProtocol, downloadMavenXml, downloadS3Protocol, } from './util'; -import { MavenFetchError } from './types'; const http = new Http('test'); From c9ad95c17b2a5031decd9f104e681f955d7a04f6 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 29 Nov 2024 17:08:44 -0300 Subject: [PATCH 3/5] Fix coverage --- lib/modules/datasource/maven/util.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/modules/datasource/maven/util.ts b/lib/modules/datasource/maven/util.ts index cc9765c06a5bad..7f3e45b9e48ddf 100644 --- a/lib/modules/datasource/maven/util.ts +++ b/lib/modules/datasource/maven/util.ts @@ -165,6 +165,7 @@ export async function downloadS3Protocol( LastModified, DeleteMarker, }): Promise => { + // istanbul ignore if: will be covered later if (DeleteMarker) { return Result.err({ type: 'not-found' }); } @@ -177,6 +178,7 @@ export async function downloadS3Protocol( const result: MavenFetchSuccess = { data }; const lastModified = normalizeDate(LastModified); + // istanbul ignore if: will be covered later if (lastModified) { result.lastModified = lastModified; } From bba8a0be132782b82ad4fccd346ec921b12df5aa Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 29 Nov 2024 17:13:13 -0300 Subject: [PATCH 4/5] Add tests --- lib/modules/datasource/maven/s3.spec.ts | 18 +++++++++++++++++- lib/modules/datasource/maven/util.ts | 2 -- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/modules/datasource/maven/s3.spec.ts b/lib/modules/datasource/maven/s3.spec.ts index 1e1ae4df70ae23..5030b0d21db780 100644 --- a/lib/modules/datasource/maven/s3.spec.ts +++ b/lib/modules/datasource/maven/s3.spec.ts @@ -46,7 +46,10 @@ describe('modules/datasource/maven/s3', () => { Bucket: 'repobucket', Key: 'org/example/package/maven-metadata.xml', }) - .resolvesOnce({ Body: meta as never }); + .resolvesOnce({ + Body: meta as never, + LastModified: new Date('2020-01-01T00:00Z'), + }); const res = await get('org.example:package', baseUrlS3); @@ -150,6 +153,19 @@ describe('modules/datasource/maven/s3', () => { ); }); + it('returns null for Deleted marker', async () => { + s3mock + .on(GetObjectCommand, { + Bucket: 'repobucket', + Key: 'org/example/package/maven-metadata.xml', + }) + .resolvesOnce({ DeleteMarker: true }); + + const res = await get('org.example:package', baseUrlS3); + + expect(res).toBeNull(); + }); + it('returns null for unknown error', async () => { s3mock .on(GetObjectCommand, { diff --git a/lib/modules/datasource/maven/util.ts b/lib/modules/datasource/maven/util.ts index 7f3e45b9e48ddf..cc9765c06a5bad 100644 --- a/lib/modules/datasource/maven/util.ts +++ b/lib/modules/datasource/maven/util.ts @@ -165,7 +165,6 @@ export async function downloadS3Protocol( LastModified, DeleteMarker, }): Promise => { - // istanbul ignore if: will be covered later if (DeleteMarker) { return Result.err({ type: 'not-found' }); } @@ -178,7 +177,6 @@ export async function downloadS3Protocol( const result: MavenFetchSuccess = { data }; const lastModified = normalizeDate(LastModified); - // istanbul ignore if: will be covered later if (lastModified) { result.lastModified = lastModified; } From 06bfc2a4312c568b13918e652d982a2199f98943 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 8 Dec 2024 15:30:11 -0300 Subject: [PATCH 5/5] Simplify error handling --- lib/modules/datasource/maven/util.ts | 44 ++++++++++------------------ 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/lib/modules/datasource/maven/util.ts b/lib/modules/datasource/maven/util.ts index cc9765c06a5bad..3a6d85956b4c2c 100644 --- a/lib/modules/datasource/maven/util.ts +++ b/lib/modules/datasource/maven/util.ts @@ -166,10 +166,18 @@ export async function downloadS3Protocol( DeleteMarker, }): Promise => { if (DeleteMarker) { + logger.trace( + { failedUrl: pkgUrl.toString() }, + 'Maven S3 lookup error: DeleteMarker encountered', + ); return Result.err({ type: 'not-found' }); } if (!(Body instanceof Readable)) { + logger.debug( + { failedUrl: pkgUrl.toString() }, + 'Maven S3 lookup error: unsupported Body type', + ); return Result.err({ type: 'unsupported-format' }); } @@ -189,51 +197,31 @@ export async function downloadS3Protocol( return Result.err(err); } - if (err.name === 'CredentialsProviderError') { - return Result.err({ type: 'credentials-error' }); - } - - if (err.message === 'Region is missing') { - return Result.err({ type: 'missing-aws-region' }); - } - - if (isS3NotFound(err)) { - return Result.err({ type: 'not-found' }); - } - - return Result.err({ type: 'unknown', err }); - }) - .onError((err) => { const failedUrl = pkgUrl.toString(); - if (err.type === 'credentials-error') { + if (err.name === 'CredentialsProviderError') { logger.debug( { failedUrl }, 'Maven S3 lookup error: credentials provider error, check "AWS_ACCESS_KEY_ID" and "AWS_SECRET_ACCESS_KEY" variables', ); - return; + return Result.err({ type: 'credentials-error' }); } - if (err.type === 'missing-aws-region') { + if (err.message === 'Region is missing') { logger.debug( { failedUrl }, 'Maven S3 lookup error: missing region, check "AWS_REGION" variable', ); - return; + return Result.err({ type: 'missing-aws-region' }); } - if (err.type === 'not-found') { + if (isS3NotFound(err)) { logger.trace({ failedUrl }, 'Maven S3 lookup error: object not found'); - return; + return Result.err({ type: 'not-found' }); } - if (err.type === 'unknown') { - logger.debug( - { failedUrl, err: err.err }, - 'Maven S3 lookup error: unknown error', - ); - return; - } + logger.debug({ failedUrl, err }, 'Maven S3 lookup error: unknown error'); + return Result.err({ type: 'unknown', err }); }); }