Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(maven): Unified result type for s3 fetch #32814

Merged
merged 6 commits into from
Dec 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions lib/modules/datasource/maven/s3.spec.ts
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
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
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
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',
zharinov marked this conversation as resolved.
Show resolved Hide resolved
);
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({});
viceice marked this conversation as resolved.
Show resolved Hide resolved
}

logger.debug(
Expand Down
Loading