Skip to content

Commit

Permalink
refactor(maven): Unified result type for s3 fetch (#32814)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zharinov
zharinov authored Dec 8, 2024
1 parent 283a7dc commit b908402
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 50 deletions.
33 changes: 23 additions & 10 deletions lib/modules/datasource/maven/s3.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -89,7 +92,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',
);
});

Expand All @@ -108,7 +111,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',
);
});

Expand All @@ -127,7 +130,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',
);
});

Expand All @@ -146,10 +149,23 @@ 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',
);
});

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, {
Expand All @@ -163,10 +179,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',
);
});

Expand All @@ -178,9 +194,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",
);
});
});
});
Expand Down
28 changes: 28 additions & 0 deletions lib/modules/datasource/maven/types.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { XmlDocument } from 'xmldoc';
import type { Result } from '../../../util/result';
import type { ReleaseResult } from '../types';

export interface MavenDependency {
Expand All @@ -19,3 +20,30 @@ export type DependencyInfo = Pick<
ReleaseResult,
'homepage' | 'sourceUrl' | 'packageScope'
>;

export interface MavenFetchSuccess<T = string> {
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<T = string> = Result<
MavenFetchSuccess<T>,
MavenFetchError
>;
8 changes: 6 additions & 2 deletions lib/modules/datasource/maven/util.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ 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,
Expand Down Expand Up @@ -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,
});
});
});

Expand Down
123 changes: 85 additions & 38 deletions lib/modules/datasource/maven/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import type {
DependencyInfo,
HttpResourceCheckResult,
MavenDependency,
MavenFetchResult,
MavenFetchSuccess,
MavenXml,
} from './types';

Expand Down Expand Up @@ -142,42 +144,85 @@ function isS3NotFound(err: Error): boolean {
return err.message === 'NotFound' || err.message === 'NoSuchKey';
}

export async function downloadS3Protocol(pkgUrl: URL): Promise<string | null> {
export async function downloadS3Protocol(
pkgUrl: URL,
): Promise<MavenFetchResult> {
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<MavenFetchResult> => {
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' });
}

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);
}

const failedUrl = pkgUrl.toString();

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 Result.err({ type: 'credentials-error' });
}

if (err.message === 'Region is missing') {
logger.debug(
{ failedUrl },
'Maven S3 lookup error: missing region, check "AWS_REGION" variable',
);
return Result.err({ type: 'missing-aws-region' });
}

if (isS3NotFound(err)) {
logger.trace({ failedUrl }, 'Maven S3 lookup error: object not found');
return Result.err({ type: 'not-found' });
}

logger.debug({ failedUrl, err }, 'Maven S3 lookup error: unknown error');
return Result.err({ type: 'unknown', err });
});
}

export async function downloadArtifactRegistryProtocol(
Expand Down Expand Up @@ -334,10 +379,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(
Expand Down

0 comments on commit b908402

Please sign in to comment.