Skip to content

Commit

Permalink
feat: rewrite raw endpoints to /contents api for GH
Browse files Browse the repository at this point in the history
Since GH enterprise 2.11 the `/raw` endpoint is not supported.
In order for us to make a gradual change without breaking our users broker clients
I added logic that rewrites /raw to /repos/owner/repo/contents endpoint.
Once all of our GHE users will use the we could change all our system code that touches GH files, or hopefully will do it on the GH agent, and delete this code.

I know that this code makes our filter logic GH aware, and this was not the intention of the broker, but this was the solution for non breaking change.
  • Loading branch information
Dar Malovani committed Nov 9, 2017
1 parent 6a12fe2 commit 9451aee
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 38 deletions.
3 changes: 0 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ To use the the broker client with a GitHub Enterprise deployment, run `docker pu
- `GITHUB_TOKEN` - a personal access token with full `repo` and `admin:repo_hook` scopes.
- `GITHUB` - the hostname of your GitHub Enterprise deployment, such as `your.ghe.domain.com`.
- `GITHUB_API` - the API endpoint of your GitHub Enterprise deployment. Should be `$GITHUB/api/v3`.
- `GITHUB_RAW` - the raw file access endpoint of your GitHub Enterprise deployment. Should be `$GITHUB/raw`.
- `PORT` - the local port at which the broker client accepts connections. Default is 7341.
- `BROKER_CLIENT_URL` - the full URL of the broker client as it will be accessible by your GitHub Enterprise deployment webhooks, such as `http://my.broker.client:7341`

Expand All @@ -75,7 +74,6 @@ docker run --restart=always \
-e GITHUB_TOKEN=secret-github-token \
-e GITHUB=your.ghe.domain.com \
-e GITHUB_API=your.ghe.domain.com/api/v3 \
-e GITHUB_RAW=your.ghe.domain.com/raw \
-e PORT=8000 \
-e BROKER_CLIENT_URL=http://my.broker.client:8000 \
snyk/broker:github-enterprise
Expand All @@ -92,7 +90,6 @@ ENV BROKER_TOKEN secret-broker-token
ENV GITHUB_TOKEN secret-github-token
ENV GITHUB your.ghe.domain.com
ENV GITHUB_API your.ghe.domain.com/api/v3
ENV GITHUB_RAW your.ghe.domain.com/raw
ENV PORT 8000
ENV BROKER_CLIENT_URL http://my.broker.client:8000
```
Expand Down
4 changes: 0 additions & 4 deletions client-templates/github/.env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ GITHUB=
# changed to "api.github.com"
GITHUB_API=$GITHUB/api/v3

# the url where raw file content is accessed, excluding scheme. For github.com
# this should be changed to "raw.githubusercontent.com"
GITHUB_RAW=$GITHUB/raw

# the url of your broker client (including scheme and port)
# BROKER_CLIENT_URL=

Expand Down
40 changes: 20 additions & 20 deletions client-templates/github/accept.json.sample
Original file line number Diff line number Diff line change
Expand Up @@ -124,62 +124,62 @@
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/package.json",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/package.json",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/package-lock.json",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/package-lock.json",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/Gemfile.lock",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/Gemfile.lock",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/Gemfile",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/Gemfile",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/pom.xml",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/pom.xml",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/requirements.txt",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/requirements.txt",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/yarn.lock",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/yarn.lock",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/build.gradle",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/build.gradle",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to determine the full dependency tree",
"method": "GET",
"path": "/:name/:repo/:branch*/build.sbt",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/build.sbt",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to check if there's any ignore rules or existing patches",
"method": "GET",
"path": "/:name/:repo/:branch*/.snyk",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_RAW}"
"path": "/repos/:name/:repo/contents/:path*/.snyk",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},

{
Expand Down
3 changes: 0 additions & 3 deletions dockerfiles/github-com/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ ENV GITHUB github.com
# The URL that the github API should be accessed, excluding scheme.
ENV GITHUB_API api.github.com

# The URL where raw file content is accessed, excluding scheme.
ENV GITHUB_RAW raw.githubusercontent.com

# The URL of the Snyk broker server
ENV BROKER_SERVER_URL https://broker.snyk.io

Expand Down
3 changes: 0 additions & 3 deletions dockerfiles/github-enterprise/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ ENV GITHUB=your.ghe.domain.com
# The URL that the github API should be accessed at.
ENV GITHUB_API $GITHUB/api/v3

# The URL where raw file content is accessed, excluding scheme.
ENV GITHUB_RAW $GITHUB/raw

# The port used by the broker client to accept webhooks
# Default value is 7341
ENV PORT 7341
Expand Down
29 changes: 28 additions & 1 deletion lib/relay.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,35 @@ function requestHandler(filterRules) {
function responseHandler(filterRules, config) {
const filters = Filters(filterRules);

const supportedFiles = (filterRules || [])
.filter(f => f.path && f.path.includes('/contents/'))
.map(f => f.path.split('/').pop());

return (brokerToken) => ({ url, headers, method, body = null } = {}, emit) => {
// run the request through the filter

// Changing old /raw endpoint to /contents api for GH clients.
// Since GH enterprise 2.11 version the /raw endpoint is deprecated
// This piece of code allow us to move to the new /content API without
// breaking old broker clients.
// Once all our GHE client will move to this new version of the broker
// client we could remove this code.
function rewriteRawApi() {
// find urls that ends with manifest files
const file = supportedFiles.find(f => url.endsWith(f));
if (file && url) {
headers.accept = 'application/vnd.github.2.11.raw';
// switching urls from: /:name/:repo/:branch*/:path
// to /repos/:name/:repo/contents/:path?ref=:branch
// $1 - repo/owner, $2 - branch, $3 - path
url = url
.replace(/(\/.+?\/.+?\/)(.+?)(\/.*)/, `/repos$1contents$3?ref=$2`);
}
}

if (config.GITHUB) {
rewriteRawApi();
}

logger.info({ method, url, headers }, 'request captured');
filters({ url, method, body, headers }, (error, result) => {
if (error) {
Expand Down
12 changes: 9 additions & 3 deletions test/fixtures/client/filters.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@
]
},


{
"path": "/repos/:repo/:owner/contents/folder*/package.json",
"method": "GET",
"origin": "http://localhost:${originPort}"
},

{
"path": "/nested/path-with/wild*/to/file.ext",
"method": "GET",
Expand Down Expand Up @@ -107,10 +114,9 @@
"valid": [
{
"queryParam": "proxyMe",
"values": ["please"],
"values": ["please"]
}
]
},

}
]
}
28 changes: 27 additions & 1 deletion test/functional/server-client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ test('proxy requests originating from behind the broker server', t => {

process.chdir(path.resolve(root, '../fixtures/client'));
process.env.BROKER_TYPE = 'client';
process.env.GITHUB = 'github.com';
process.env.BROKER_TOKEN = '12345';
process.env.BROKER_SERVER_URL = `http://localhost:${serverPort}`;
process.env.ORIGIN_PORT = echoServerPort;
Expand All @@ -35,7 +36,7 @@ test('proxy requests originating from behind the broker server', t => {
// wait for the client to successfully connect to the server and identify itself
server.io.on('connection', socket => {
socket.on('identify', token => {
t.plan(19);
t.plan(21);

t.test('successfully broker POST', t => {
const url = `http://localhost:${serverPort}/broker/${token}/echo-body`;
Expand Down Expand Up @@ -245,6 +246,31 @@ test('proxy requests originating from behind the broker server', t => {
});
});

t.test('/raw is rewritten to /contents for GH users', t => {
const url = `http://localhost:${serverPort}/broker/${token}/owner/repo/HEAD/folder/package.json`;
const headers = {};
request({ url, method: 'get', headers, json: true }, (err, res) => {
t.equal(res.statusCode, 200, '200 statusCode');
t.equal(res.body.headers.accept, 'application/vnd.github.2.11.raw',
'injected raw headers');
t.equal(res.body.query.ref, 'HEAD',
'extracted ref as query parameter');
t.equal(res.body.url,
'/repos/owner/repo/contents/folder/package.json?ref=HEAD',
'get correct full url');
t.end();
});
});

t.test('/raw is not rewritten to /contents for unsupported manifest file', t => {
const url = `http://localhost:${serverPort}/broker/${token}/owner/repo/HEAD/folder/unsupportedFile.ext`;
const headers = {};
request({ url, method: 'get', headers }, (err, res) => {
t.equal(res.statusCode, 401, '401 statusCode');
t.end();
});
});

t.test('clean up', t => {
client.close();
setTimeout(() => {
Expand Down
5 changes: 5 additions & 0 deletions test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ echoServer.get(
res.send(req.params.filename);
});

echoServer.get('/repos/owner/repo/contents/folder/package.json',
(req, res) => {
res.json({headers: req.headers, query: req.query, url: req.url});
});

echoServer.all('*', (req, res) => {
res.send(false);
});
Expand Down

0 comments on commit 9451aee

Please sign in to comment.