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

feat: github authenticated registry #506

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions .changeset/honest-pans-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperlane-xyz/registry': minor
---

feat: authenticated github registry
2 changes: 2 additions & 0 deletions .env.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Test Github token
GITHUB_TOKEN=""
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ yarn-error.log*
# local env files
.env*.local

# env file
.env

# typescript
*.tsbuildinfo

Expand Down
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
"@changesets/cli": "^2.26.2",
"@eslint/js": "^9.1.1",
"@hyperlane-xyz/sdk": "7.0.0",
"@types/chai-as-promised": "^8",
"@types/mocha": "^10.0.1",
"@types/node": "^16.9.1",
"@typescript-eslint/parser": "^7.7.0",
"chai": "^4.3.6",
"chai": "^5.1.2",
"chai-as-promised": "^8.0.1",
"dotenv": "^16.4.7",
"eslint": "^9.0.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-yml": "^1.14.0",
Expand Down Expand Up @@ -67,7 +70,7 @@
"prettier": "prettier --write ./chains ./deployments",
"prepare": "husky",
"release": "yarn build && yarn changeset publish",
"test:unit": "yarn build && mocha --config .mocharc.json './test/unit/*.test.ts' --exit",
"test:unit": "yarn build && mocha --require dotenv/config --config .mocharc.json './test/unit/*.test.ts' --exit",
"test:health": "yarn build && mocha --config .mocharc.json './test/health/*.test.ts' --exit --parallel",
"version:prepare": "yarn changeset version && yarn install --no-immutable",
"version:check": "yarn changeset status"
Expand Down
19 changes: 15 additions & 4 deletions src/registry/GithubRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class GithubRegistry extends BaseRegistry implements IRegistry {
public readonly repoOwner: string;
public readonly repoName: string;
public readonly proxyUrl: string | undefined;
private readonly authToken: string | undefined;

constructor(options: GithubRegistryOptions = {}) {
super({ uri: options.uri ?? DEFAULT_GITHUB_REGISTRY, logger: options.logger });
Expand All @@ -72,6 +73,7 @@ export class GithubRegistry extends BaseRegistry implements IRegistry {
this.repoOwner = pathSegments.at(-2)!;
this.repoName = pathSegments.at(-1)!;
this.proxyUrl = options.proxyUrl;
this.authToken = options.authToken;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we assert that proxyUrl and authToken can't be set at the same time?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually proxyUrl can still be set, when authentication limit is reached it will fallback to proxyUrl again as the test here testing it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh love it

}

getUri(itemPath?: string): string {
Expand Down Expand Up @@ -189,10 +191,14 @@ export class GithubRegistry extends BaseRegistry implements IRegistry {
}

public async getApiRateLimit(): Promise<GithubRateResponse['resources']['core']> {
const baseHeader = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a private class property like

private readonly baseApiHeaders: Record<string, string> = {
    'X-GitHub-Api-Version': GITHUB_API_VERSION,
  };

and reuse here and in the fetch method below

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

'X-GitHub-Api-Version': '2022-11-28',
};

const response = await fetch(`${GITHUB_API_URL}/rate_limit`, {
headers: {
'X-GitHub-Api-Version': '2022-11-28',
},
headers: this.authToken
? { ...baseHeader, Authorization: `Bearer ${this.authToken}` }
: baseHeader,
});
const { resources } = (await response.json()) as GithubRateResponse;
return resources.core;
Expand Down Expand Up @@ -225,7 +231,12 @@ export class GithubRegistry extends BaseRegistry implements IRegistry {

protected async fetch(url: string): Promise<Response> {
this.logger.debug(`Fetching from github: ${url}`);
const response = await fetch(url);
const useToken = !(this.proxyUrl && url.startsWith(this.proxyUrl)) && !!this.authToken;
const response = await fetch(url, {
headers: useToken
? { 'X-GitHub-Api-Version': '2022-11-28', Authorization: `token ${this.authToken}` }
: undefined,
});
if (!response.ok)
throw new Error(`Failed to fetch from github: ${response.status} ${response.statusText}`);
return response;
Expand Down
57 changes: 55 additions & 2 deletions test/unit/registry.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { expect } from 'chai';
import { use as chaiUse, expect } from 'chai';
import chaiAsPromised from 'chai-as-promised';
import sinon from 'sinon';

import type { ChainMetadata } from '@hyperlane-xyz/sdk';
Expand All @@ -24,6 +25,8 @@ const MOCK_ADDRESS = '0x0000000000000000000000000000000000000001';
// Must be kept in sync with value in canonical registry's main branch
const ETH_MAILBOX_ADDRESS = '0xc005dc82818d67AF737725bD4bf75435d065D239';

chaiUse(chaiAsPromised);

describe('Registry utilities', () => {
const githubRegistry = new GithubRegistry({ branch: GITHUB_REGISTRY_BRANCH });
expect(githubRegistry.repoOwner).to.eql('hyperlane-xyz');
Expand Down Expand Up @@ -186,7 +189,7 @@ describe('Registry utilities', () => {

describe('ProxiedGithubRegistry', () => {
const proxyUrl = 'http://proxy.hyperlane.xyz';
let proxiedGithubRegistry;
let proxiedGithubRegistry: GithubRegistry;
let getApiRateLimitStub;
beforeEach(() => {
proxiedGithubRegistry = new GithubRegistry({ branch: GITHUB_REGISTRY_BRANCH, proxyUrl });
Expand All @@ -209,6 +212,56 @@ describe('Registry utilities', () => {
);
});
});

describe('Authenticated GithubRegistry', () => {
const proxyUrl = 'http://proxy.hyperlane.xyz';
let authenticatedGithubRegistry;
let invalidTokenGithubRegistry;
let getApiRateLimitStub;
before(function () {
if (!process.env.GITHUB_TOKEN) {
console.log('Skipping tests because GITHUB_TOKEN is not defined');
this.skip();
}
});
beforeEach(() => {
authenticatedGithubRegistry = new GithubRegistry({
branch: GITHUB_REGISTRY_BRANCH,
proxyUrl,
authToken: process.env.GITHUB_TOKEN,
});
invalidTokenGithubRegistry = new GithubRegistry({
branch: GITHUB_REGISTRY_BRANCH,
proxyUrl,
authToken: 'invalid_token',
});
getApiRateLimitStub = sinon.stub(authenticatedGithubRegistry, 'getApiRateLimit');
});
afterEach(() => {
sinon.restore();
});
it('always uses the authenticated api if rate limit has been not been hit', async () => {
getApiRateLimitStub.returns({ remaining: 10 });
expect(await authenticatedGithubRegistry.getApiUrl()).to.equal(
`${GITHUB_API_URL}/repos/hyperlane-xyz/hyperlane-registry/git/trees/main?recursive=true`,
);
});

it('should fallback to proxy url if public rate limit has been hit', async () => {
getApiRateLimitStub.returns({ remaining: 0 });
expect(await authenticatedGithubRegistry.getApiUrl()).to.equal(
`${proxyUrl}/repos/hyperlane-xyz/hyperlane-registry/git/trees/main?recursive=true`,
);
});

it('should fetch chains with authenticated token', async () => {
expect(invalidTokenGithubRegistry.getChains()).to.eventually.be.fulfilled;
});

it('should fail fetching chains with invalid authentication token', async () => {
expect(invalidTokenGithubRegistry.getChains()).to.eventually.be.rejected;
});
});
});

describe('Registry regex', () => {
Expand Down
98 changes: 96 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1913,10 +1913,13 @@ __metadata:
"@changesets/cli": "npm:^2.26.2"
"@eslint/js": "npm:^9.1.1"
"@hyperlane-xyz/sdk": "npm:7.0.0"
"@types/chai-as-promised": "npm:^8"
"@types/mocha": "npm:^10.0.1"
"@types/node": "npm:^16.9.1"
"@typescript-eslint/parser": "npm:^7.7.0"
chai: "npm:^4.3.6"
chai: "npm:^5.1.2"
chai-as-promised: "npm:^8.0.1"
dotenv: "npm:^16.4.7"
eslint: "npm:^9.0.0"
eslint-config-prettier: "npm:^9.1.0"
eslint-plugin-yml: "npm:^1.14.0"
Expand Down Expand Up @@ -3319,6 +3322,24 @@ __metadata:
languageName: node
linkType: hard

"@types/chai-as-promised@npm:^8":
version: 8.0.1
resolution: "@types/chai-as-promised@npm:8.0.1"
dependencies:
"@types/chai": "npm:*"
checksum: 10/827f7d8d5af15761e4b4c0ef08134e12835b28b6da91dd75e4d3e76c856cee9f3dd8bef45d4c1219237951e3e588c3f70d7271b38e626415126c63b057bf2fba
languageName: node
linkType: hard

"@types/chai@npm:*":
version: 5.0.1
resolution: "@types/chai@npm:5.0.1"
dependencies:
"@types/deep-eql": "npm:*"
checksum: 10/0f829d4f4be06d6a32c9d89ac08c356df89bafc4b923d8b7fd56cf78d681f5fddfe7aa3391b747f076c57129428f4df694026f344ad3bf8bda65e2ca50c0fd37
languageName: node
linkType: hard

"@types/connect@npm:^3.4.33":
version: 3.4.38
resolution: "@types/connect@npm:3.4.38"
Expand All @@ -3328,6 +3349,13 @@ __metadata:
languageName: node
linkType: hard

"@types/deep-eql@npm:*":
version: 4.0.2
resolution: "@types/deep-eql@npm:4.0.2"
checksum: 10/249a27b0bb22f6aa28461db56afa21ec044fa0e303221a62dff81831b20c8530502175f1a49060f7099e7be06181078548ac47c668de79ff9880241968d43d0c
languageName: node
linkType: hard

"@types/http-cache-semantics@npm:*":
version: 4.0.4
resolution: "@types/http-cache-semantics@npm:4.0.4"
Expand Down Expand Up @@ -3884,6 +3912,13 @@ __metadata:
languageName: node
linkType: hard

"assertion-error@npm:^2.0.1":
version: 2.0.1
resolution: "assertion-error@npm:2.0.1"
checksum: 10/a0789dd882211b87116e81e2648ccb7f60340b34f19877dd020b39ebb4714e475eb943e14ba3e22201c221ef6645b7bfe10297e76b6ac95b48a9898c1211ce66
languageName: node
linkType: hard

"async-limiter@npm:~1.0.0":
version: 1.0.1
resolution: "async-limiter@npm:1.0.1"
Expand Down Expand Up @@ -4360,7 +4395,18 @@ __metadata:
languageName: node
linkType: hard

"chai@npm:^4.3.4, chai@npm:^4.3.6":
"chai-as-promised@npm:^8.0.1":
version: 8.0.1
resolution: "chai-as-promised@npm:8.0.1"
dependencies:
check-error: "npm:^2.0.0"
peerDependencies:
chai: ">= 2.1.2 < 6"
checksum: 10/1938d9f00b1ae197b47c01a9ff8455d3719bc4638103fa5ad254a32e27bc663ba8530fecfa840309e254c2a52ef171c194b7fb25615f8c5d33022d64456f07f9
languageName: node
linkType: hard

"chai@npm:^4.3.4":
version: 4.4.1
resolution: "chai@npm:4.4.1"
dependencies:
Expand All @@ -4375,6 +4421,19 @@ __metadata:
languageName: node
linkType: hard

"chai@npm:^5.1.2":
version: 5.1.2
resolution: "chai@npm:5.1.2"
dependencies:
assertion-error: "npm:^2.0.1"
check-error: "npm:^2.1.1"
deep-eql: "npm:^5.0.1"
loupe: "npm:^3.1.0"
pathval: "npm:^2.0.0"
checksum: 10/e8c2bbc83cb5a2f87130d93056d4cfbbe04106e12aa798b504816dbe3fa538a9f68541b472e56cbf0f54558b501d7e31867d74b8218abcd5a8cc8ba536fba46c
languageName: node
linkType: hard

"chalk@npm:5.3.0, chalk@npm:^5.3.0":
version: 5.3.0
resolution: "chalk@npm:5.3.0"
Expand Down Expand Up @@ -4419,6 +4478,13 @@ __metadata:
languageName: node
linkType: hard

"check-error@npm:^2.0.0, check-error@npm:^2.1.1":
version: 2.1.1
resolution: "check-error@npm:2.1.1"
checksum: 10/d785ed17b1d4a4796b6e75c765a9a290098cf52ff9728ce0756e8ffd4293d2e419dd30c67200aee34202463b474306913f2fcfaf1890641026d9fc6966fea27a
languageName: node
linkType: hard

"chokidar@npm:3.5.3":
version: 3.5.3
resolution: "chokidar@npm:3.5.3"
Expand Down Expand Up @@ -5021,6 +5087,13 @@ __metadata:
languageName: node
linkType: hard

"deep-eql@npm:^5.0.1":
version: 5.0.2
resolution: "deep-eql@npm:5.0.2"
checksum: 10/a529b81e2ef8821621d20a36959a0328873a3e49d393ad11f8efe8559f31239494c2eb889b80342808674c475802ba95b9d6c4c27641b9a029405104c1b59fcf
languageName: node
linkType: hard

"deep-is@npm:^0.1.3":
version: 0.1.4
resolution: "deep-is@npm:0.1.4"
Expand Down Expand Up @@ -5169,6 +5242,13 @@ __metadata:
languageName: node
linkType: hard

"dotenv@npm:^16.4.7":
version: 16.4.7
resolution: "dotenv@npm:16.4.7"
checksum: 10/f13bfe97db88f0df4ec505eeffb8925ec51f2d56a3d0b6d916964d8b4af494e6fb1633ba5d09089b552e77ab2a25de58d70259b2c5ed45ec148221835fc99a0c
languageName: node
linkType: hard

"eastasianwidth@npm:^0.2.0":
version: 0.2.0
resolution: "eastasianwidth@npm:0.2.0"
Expand Down Expand Up @@ -7876,6 +7956,13 @@ __metadata:
languageName: node
linkType: hard

"loupe@npm:^3.1.0":
version: 3.1.2
resolution: "loupe@npm:3.1.2"
checksum: 10/8f5734e53fb64cd914aa7d986e01b6d4c2e3c6c56dcbd5428d71c2703f0ab46b5ab9f9eeaaf2b485e8a1c43f865bdd16ec08ae1a661c8f55acdbd9f4d59c607a
languageName: node
linkType: hard

"lowercase-keys@npm:^2.0.0":
version: 2.0.0
resolution: "lowercase-keys@npm:2.0.0"
Expand Down Expand Up @@ -9004,6 +9091,13 @@ __metadata:
languageName: node
linkType: hard

"pathval@npm:^2.0.0":
version: 2.0.0
resolution: "pathval@npm:2.0.0"
checksum: 10/b91575bf9cdf01757afd7b5e521eb8a0b874a49bc972d08e0047cfea0cd3c019f5614521d4bc83d2855e3fcc331db6817dfd533dd8f3d90b16bc76fad2450fc1
languageName: node
linkType: hard

"pbkdf2@npm:^3.0.17":
version: 3.1.2
resolution: "pbkdf2@npm:3.1.2"
Expand Down