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

Add an Azure Function for publishing incremental builds to Artifactory #8

Merged
merged 43 commits into from Apr 26, 2018

Conversation

@rtyler
Copy link
Member

rtyler commented Apr 20, 2018

Per my discussion with @jglick around JEP-305, rather than adding credentials for publishing into Artifactory, this approach allows credentials to stay off of ci.jenkins.io.

This also enforces commit verification and validation against jenkins-infra/repository-permissions-updater metadata before uploading into Artifactory.

It is also largely based on @jglick's orototype work here

rtyler added 13 commits Apr 19, 2018
This gives me something to do while I fight with test scaffolding
Azure Functions on supports node 8 right now :(
… upload permissions
@rtyler rtyler requested a review from slide Apr 20, 2018
This forbids arbitrary pummeling of other hosts
@rtyler
Copy link
Member Author

rtyler commented Apr 20, 2018

@slide
slide approved these changes Apr 20, 2018
Copy link

slide left a comment

This looks really cool.

| Variable | Description

| `GITHUB_TOKEN`
| A GitHub OAuth/Personal Access Token which can be used for GitHub API queries

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

preferably also repo:status (best to be explicit about what ought to be granted)

const buildUrl = data.body.build_url;
/* If we haven't received any valid data, just bail early
*/
if ((!buildUrl) || (!buildUrl.match(JENKINS_HOST))) {

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

Not sure what match means here. Is there a startsWith function? And the prefix URL should end with a / I think.

This comment has been minimized.

Copy link
@rtyler

rtyler Apr 20, 2018

Author Member

There is, that's a good point. Will change

defaults to `https://repo.jenkins-ci.org/incrementals/`

| `ARTIFACTORY_KEY`
| An Artifactory user's API key

This comment has been minimized.

Copy link
@jglick

This comment has been minimized.

Copy link
@rtyler

rtyler Apr 20, 2018

Author Member

Correct


| `JENKINS_HOST`
| A Jenkins instance (defaults to `https://ci.jenkins.io`) to which URLs are
expected to confirm.

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

conform

| A GitHub OAuth/Personal Access Token which can be used for GitHub API queries

| `JENKINS_HOST`
| A Jenkins instance (defaults to `https://ci.jenkins.io`) to which URLs are

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

Trailing slash? (see below)

let repoInfo = pipeline.getRepoFromUrl(metadata.remoteUrl);

if (!github.commitExists(repoInfo.owner, repoInfo.repo, metadata.hash)) {
context.log.error('This request was using a commit which does not exist on GitHub!', commit);

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

What is commit here? Seems to be an undefined variable?

return archivePath;
})
.catch(err => context.log.error(err));
context.log.info('Should be ready to upload', archive);

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

No, first you need to check whether the archive is empty.

const verified = await permissions.verify(repoPath, archivePath, perms);

/*
* Finally, we can upload to Artifactory

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

No, first you need to check whether some representative element of the archive has already been deployed, and if so, skip redeployment.

* Finally, we can upload to Artifactory
*/

const upload = await fetch(INCREMENTAL_URL,

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

Nope, you will get a 403 from Artifactory this way because it things you are trying to replace the entire repository.

You need to append archive.zip to the URL.

Been there, done that.

This comment has been minimized.

Copy link
@rtyler

rtyler Apr 20, 2018

Author Member

Hah! That's fun, good to know 😃

method: 'PUT',
body: fs.createReadStream(archivePath)
});
context.log.info('Uploaded', upload);

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

Misleading—it may in fact have failed.

Unit tests can simply be run by executing `make check` in this directory.

For acceptance testing, please set the appropriate <<env>> in a terminal and
then execute `make run` in the repository root directory.

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

This does not work:

Unable to find project root. Expecting to find one of host.json, local.settings.json in project root.

From Azure documentation, I gather that files of these names are supposed to be present here. But they are not.

Just adding host.json

{}

lets the container start, but then it throws all kinds of errors.

This comment has been minimized.

Copy link
@rtyler

rtyler Apr 20, 2018

Author Member

Ah bollocks, I have these files defined locally. Will check them in

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

Tried with your additions. Still get all kinds of errors at startup:

…
Starting Host (HostId=…, InstanceId=…, …)
Unable to configure java worker. Could not find JAVA_HOME app setting.

Could not configure language worker Java.
…
Host initialized (469ms)
Couldn't require bundle, falling back to Worker.js. Error: Cannot find module '/usr/local/lib/node_modules/azure-functions-core-tools/bin/workers/node/grpc/src/node/extension_binary/node-v59-linux-x64/grpc_node.node'
internal/modules/cjs/loader.js:550
    throw err;
    ^
Error: Cannot find module './Worker.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:548:15)
    at Function.Module._load (internal/modules/cjs/loader.js:475:25)
    at Module.require (internal/modules/cjs/loader.js:598:17)
    at require (internal/modules/cjs/helpers.js:11:18)
    at Object.<anonymous> (/usr/local/lib/node_modules/azure-functions-core-tools/bin/workers/node/dist/src/nodejsWorker.js:8:14)
    at Module._compile (internal/modules/cjs/loader.js:654:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:665:10)
    at Module.load (internal/modules/cjs/loader.js:566:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:506:12)
    at Function.Module._load (internal/modules/cjs/loader.js:498:3)
Host started (509ms)
Job host started
Worker encountered an error.
Worker process with pid 43 exited with code 1.
…
Couldn't require bundle, falling back to Worker.js. Error: Cannot find module '/usr/local/lib/node_modules/azure-functions-core-tools/bin/workers/node/grpc/src/node/extension_binary/node-v59-linux-x64/grpc_node.node'
internal/modules/cjs/loader.js:550
    throw err;
    ^
Error: Cannot find module './Worker.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:548:15)
    at Function.Module._load (internal/modules/cjs/loader.js:475:25)
    at Module.require (internal/modules/cjs/loader.js:598:17)
    at require (internal/modules/cjs/helpers.js:11:18)
    at Object.<anonymous> (/usr/local/lib/node_modules/azure-functions-core-tools/bin/workers/node/dist/src/nodejsWorker.js:8:14)
    at Module._compile (internal/modules/cjs/loader.js:654:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:665:10)
    at Module.load (internal/modules/cjs/loader.js:566:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:506:12)
    at Function.Module._load (internal/modules/cjs/loader.js:498:3)
Worker encountered an error.
Worker process with pid 52 exited with code 1.
…
Couldn't require bundle, falling back to Worker.js. Error: Cannot find module '/usr/local/lib/node_modules/azure-functions-core-tools/bin/workers/node/grpc/src/node/extension_binary/node-v59-linux-x64/grpc_node.node'
internal/modules/cjs/loader.js:550
    throw err;
    ^
Error: Cannot find module './Worker.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:548:15)
    at Function.Module._load (internal/modules/cjs/loader.js:475:25)
    at Module.require (internal/modules/cjs/loader.js:598:17)
    at require (internal/modules/cjs/helpers.js:11:18)
    at Object.<anonymous> (/usr/local/lib/node_modules/azure-functions-core-tools/bin/workers/node/dist/src/nodejsWorker.js:8:14)
    at Module._compile (internal/modules/cjs/loader.js:654:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:665:10)
    at Module.load (internal/modules/cjs/loader.js:566:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:506:12)
    at Function.Module._load (internal/modules/cjs/loader.js:498:3)
Worker encountered an error.
Worker process with pid 59 exited with code 1.
Listening on http://localhost:7071/
…

And every curl command gives a 500 error, with the container log saying:

Executing 'Functions.incrementals-publisher' (Reason='This function was programmatically called via the host APIs.', Id=…)
System.Private.CoreLib: Exception while executing function: Functions.incrementals-publisher. System.Private.CoreLib: One or more errors occurred. (Worker process with pid 43 exited with code 1) (Worker process with pid 52 exited with code 1) (Worker process with pid 59 exited with code 1). Worker process with pid 43 exited with code 1.
Executed 'Functions.incrementals-publisher' (Failed, Id=…)
System.Private.CoreLib: Exception while executing function: Functions.incrementals-publisher. System.Private.CoreLib: One or more errors occurred. (Worker process with pid 43 exited with code 1) (Worker process with pid 52 exited with code 1) (Worker process with pid 59 exited with code 1). Worker process with pid 43 exited with code 1.

This comment has been minimized.

Copy link
@rtyler

rtyler Apr 20, 2018

Author Member

Ah, I neglected to re-push the rtyler/azure-functionis container of which the latest tag is using Node 9. Turns out the Azure Functions runtime requires Node 8.

I'm tethered right now, so I'll have to push when I get back home. Or you could build this your own

This comment has been minimized.

Copy link
@jglick

jglick Apr 23, 2018

Contributor

Resolved with manual rebuild, FTR.

rtyler added 2 commits Apr 20, 2018
@@ -52,7 +52,7 @@ module.exports = async (context, data) => {
* The first step is to take the buildUrl and fetch some metadata about this
* specific Pipeline Run
*/
let metdata_url = pipeline.getApiUrl(buildUrl);
let metdataUrl = pipeline.getApiUrl(buildUrl);

This comment has been minimized.

Copy link
@jglick

jglick Apr 20, 2018

Contributor

Still a typo.

Gee this dynamic language thing is great! What was I so worried about?

rtyler and others added 2 commits Apr 20, 2018
jglick and others added 2 commits Apr 20, 2018
Fixes to incrementals-publisher
@jglick jglick mentioned this pull request Apr 23, 2018
9 of 9 tasks complete
jglick added 16 commits Apr 23, 2018
…chive" errors.
…nd-line curl always works.
… CI build).
@@ -9,6 +9,18 @@ check: depends
depends:
$(foreach project, $(PROJECTS), $(MAKE) -C $(project) $@ || exit 1;)

run:
docker run --net host --rm -ti \

This comment has been minimized.

Copy link
@olblak

olblak Apr 25, 2018

why do you specify the network host?

This comment has been minimized.

Copy link
@rtyler

rtyler Apr 25, 2018

Author Member

This is because the Azure Functions loader doesn't know (last I checked) won't bind to 0.0.0.0 and instead only binds to localhost

jglick and others added 6 commits Apr 25, 2018
…leading to cryptic warnings about promises.
Incrementals fixes
@rtyler
Copy link
Member Author

rtyler commented Apr 26, 2018

This all looks good to me, I'm going to merge this and then make sure the appropriate credentials are configured in the "App Service" resource in Azure

@rtyler rtyler merged commit 2650558 into master Apr 26, 2018
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@rtyler rtyler deleted the incrementals-publisher branch Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.