Generic Gateway cleanup

[radu-matei]

Cherry-pick and cleanup commits from #721, as the commit history there was becoming difficult to work with (it got mixed with the commit history from older PRs, and the commits contained changes to files moved to other repos). (This keeps @dgkanatsios as the author)

Let me know if I there is anything missing from that branch.

cc @dgkanatsios, @vdice

closes #602

edit:
when this is merged, Azure/brigade-charts#18 should also merged

[dgkanatsios]

LGTM!

[radu-matei]

Linking my comment from the original issue for this feature - #602 (comment)

[radu-matei]

Circling back on this issue, I really think we should not expose the Brigade API in order to have a generic gateway, and can have this just like a regular gateway, enabled / disabled through the installation.

Thoughts?

[radu-matei]

As discussed in the standup, let's update this PR with a separate gateway, without exposing the Brigade API.

cc @dgkanatsios.

[dgkanatsios]

moved generic gateway to a separate component
edit: I also moved the Generic Gateway secret generation (on brig project create) on the advanced options, WDYT?

cc @radu-matei @vdice

[vdice]

This looks great! Only two minor suggestions; neither blocking.

Tested in conjunction with Azure/brigade-charts#18 (with needed updates on that branch made locally) and looking great!

 $ curl -X POST -H "Content-Type: application/json" -d '{"commit":"foo"}' http://40.87.92.114:8081/webhook/incorrectproject/mygenericwebhooksecret && echo
{"status":"project not found"}

 $ curl -X POST -H "Content-Type: application/json" -d '{"commit":"foo"}' http://40.87.92.114:8081/webhook/brigade-9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c/mygenericwebhooksecret && echo
{"status":"Success"}

 $ b build list test -c 1
ID                        	TYPE   	PROVIDER      	PROJECT                                                       	STATUS   	AGE
01d1em32f8r4110b2hg2s5bh3v	webhook	GenericWebhook	brigade-9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c	Succeeded	2s

[dgkanatsios]

also renamed a bunch of stuff from 'GenericWebhook' to 'GenericGateway' (e.g. genericWebhookSecret -> genericGatewaySecret) for a more consistent naming with other gateways

[radu-matei]

Why bundle this change in this PR?

[dgkanatsios]

just bumped into it as I was copying/pasting stuff, can take it out if it makes sense for a different PR :)

[radu-matei]

Some questions:

• is there a scenario where I would want to send two different events to the same project using the generic gateway?
• if so, is there any way of differentiating between them, or are both of them going to trigger the same event.on("webhook") callback?
• you could probably check the payload and fire the correct function, but we should probably document this?

Not blocking this PR:

• how could we fire different kinds of events from this single gateway? One option would be for the project to define the list of custom events it could accept from the generic gateway, and then the event source could have the event type as query parameter / path. Are there security implications here, as the generic gateway wouldn't create the build if the event type it received from the source is not matched by the project?

[dgkanatsios]

thanks for the review! So, right now, you can call endpoint "webhook" on the Generic Gateway and this will trigger a "webhook" event. There is no other way to trigger any other type of event. What we can do is

• include the type of event on the JSON payload
• if it's missing, default to the "webhook" event
• add the "webhook" event to all Projects by default (add a type called GenericGatewayEvents, map[string]{} <- for easy lookup)
• allow user during brig create to set any additional events (during advanced options)
• during Generic Gateway calls, check the provided type of event on the JSON payload
• if it's included in GenericGatewayEvents, raise the provided event
• if it's not, return 400

WDYT? More importantly, should this be included in this PR or left for after 1.0?

[radu-matei]

This is no longer for the Brigade API server.

[radu-matei]

This is a bit confusing - when I don't go to advanced options, a secret for the generic gateway is not created for me?

Would it make sense to directly add a prompt for the generic gateway secret in the advanced options? If the user does not choose advanced options, auto-generate it. If so, let the user manually set it.

[radu-matei]

Also, I think there's a bug somewhere in setting the generic gateway secret - consider the following scenario: I create a new project and choose advanced options:

radu:brigade$ .brig project create --namespace brigade
? Project Name abcdef
? Full repository name github.com/abcdef
? Clone URL (https://github.com/your/repo.git) https://github.com/abcdef.git
? Add secrets? No
Auto-generated a Shared Secret: "m2XClW0LTMfwgoPWLARw5trT"
? Configure GitHub Access? No
? Configure advanced options Yes
? Custom VCS sidecar (enter 'NONE' for no sidecar) deis/git-sidecar:latest
? Build storage size 
? SecretKeyRef usage No
? Build storage class standard
? Job cache storage class standard
? Worker image registry or DockerHub org 
? Worker image name 
? Custom worker image tag 
? Worker image pull policy IfNotPresent
? Worker command yarn -s start
? Initialize Git submodules No
? Allow host mounts No
? Allow privileged jobs No
? Image pull secrets 
? Default script ConfigMap name 
? brigade.js file path relative to the repository root 
? Upload a default brigade.js script 
Auto-generated Generic Gateway Secret (for Brigade API Server): A1m9n
Project ID: brigade-bef57ec7f53a6d40beb640a780a639c83bc29ac8a9816f1fc6c5c6

radu:brigade$ brig project get brigade-bef57ec7f53a6d40beb640a780a639c83bc29ac8a9816f1fc6c5c6 --namespace brigade
{
  "metadata": {
    "name": "brigade-bef57ec7f53a6d40beb640a780a639c83bc29ac8a9816f1fc6c5c6",
    "creationTimestamp": null,
    "labels": {
      "app": "brigade",
      "component": "project",
      "heritage": "brigade"
    },
    "annotations": {
      "projectName": "abcdef"
    }
  },
  "stringData": {
    "allowHostMounts": "false",
    "allowPrivilegedJobs": "false",
    "brigadejsPath": "",
    "buildStorageSize": "50Mi",
    "cloneURL": "https://github.com/abcdef.git",
    "defaultScript": "",
    "defaultScriptName": "",
    "github.baseURL": "",
    "github.token": "",
    "github.uploadURL": "",
    "imagePullSecrets": "",
    "initGitSubmodules": "false",
    "kubernetes.allowSecretKeyRef": "false",
    "kubernetes.buildStorageClass": "standard",
    "kubernetes.cacheStorageClass": "standard",
    "namespace": "brigade",
    "repository": "github.com/abcdef",
    "secrets": "{}",
    "sharedSecret": "m2XClW0LTMfwgoPWLARw5trT",
    "sshKey": "",
    "vcsSidecar": "deis/git-sidecar:latest",
    "worker.name": "",
    "worker.pullPolicy": "IfNotPresent",
    "worker.registry": "",
    "worker.tag": "",
    "workerCommand": "yarn -s start"
  },
  "type": "brigade.sh/project"
}
radu:brigade$ brig version
v0.19.0-82-g3ae57cc

After creating the project, it tells me Auto-generated Generic Gateway Secret (for Brigade API Server): A1m9n. But that is nowhere to be found in the project.

[radu-matei]

It is also unclear from this doc how one would create the secret, and how it is used. I added a new secret named genericGatewaySecret (which I think is what the gateway is looking for?):

radu:brigade$ brig project create --namespace brigade
? Project Name manually-added-secreat
? Full repository name github.com/manually-added-secreat
? Clone URL (https://github.com/your/repo.git) https://github.com/manually-added-secreat.git
? Add secrets? Yes
?       Secret 1 genericGatewaySecret
?       Value testing
? ===> Add another? No
Auto-generated a Shared Secret: "3r2RaJ8AsEtcKz18nPKHbE8X"
? Configure GitHub Access? No
? Configure advanced options No
Project ID: brigade-6f06bff6a9d15af9ac255804c4904c0a0cb97bc5e73f1f67c2250f

radu:brigade$ brig project get --namespace brigade brigade-6f06bff6a9d15af9ac255804c4904c0a0cb97bc5e73f1f67c2250f
{
  "metadata": {
    "name": "brigade-6f06bff6a9d15af9ac255804c4904c0a0cb97bc5e73f1f67c2250f",
    "creationTimestamp": null,
    "labels": {
      "app": "brigade",
      "component": "project",
      "heritage": "brigade"
    },
    "annotations": {
      "projectName": "manually-added-secreat"
    }
  },
  "stringData": {
    "allowHostMounts": "false",
    "allowPrivilegedJobs": "false",
    "brigadejsPath": "",
    "buildStorageSize": "50Mi",
    "cloneURL": "https://github.com/manually-added-secreat.git",
    "defaultScript": "",
    "defaultScriptName": "",
    "github.baseURL": "",
    "github.token": "",
    "github.uploadURL": "",
    "imagePullSecrets": "",
    "initGitSubmodules": "false",
    "kubernetes.allowSecretKeyRef": "false",
    "kubernetes.buildStorageClass": "",
    "kubernetes.cacheStorageClass": "",
    "namespace": "brigade",
    "repository": "github.com/manually-added-secreat",
    "secrets": "{\"genericGatewaySecret\":\"testing\"}",
    "sharedSecret": "3r2RaJ8AsEtcKz18nPKHbE8X",
    "sshKey": "",
    "vcsSidecar": "deis/git-sidecar:latest",
    "worker.name": "",
    "worker.pullPolicy": "IfNotPresent",
    "worker.registry": "",
    "worker.tag": "",
    "workerCommand": "yarn -s start"
  },
  "type": "brigade.sh/project"
}

That is added in the project as "secrets": "{\"genericGatewaySecret\":\"testing\"}".

Now I'm trying to use the gateway:

$ curl http://localhost:8000/webhook/brigade-6f06bff6a9d15af9ac255804c4904c0a0cb97bc5e73f1f67c2250f/testing -X POST -H "Content-Type: application/json" -d '{"commit":"foo"}'

{"status":"secret is empty, please insert a secret in the URL and try again"}

And I suspect it's because in the project this secret is set as sv.String("genericGatewaySecret"), when it should probably be set as proj.Secrets["genericGatewaySecret"].

Am I missing something here?

[dgkanatsios]

Indeed, it was really confusing. Please take a look now, it should be good. I chose to not generate anything if user does not get into Advanced Options during brig project create, since they might not want to use the Generic Gateway, so it wouldn't be worth generating anything. Hope it's now clear in the docs as well.

[radu-matei]

Nit: you should add brigade-generic-gateway/rootfs/brigade-generic-gateway to .gitignore.

[radu-matei]

Ok, manually tested and LGTM.

There is still a small nit (I don't like that there are different behaviours between not choosing advanced options and choosing them, but going with all defaults), but I'll open a separate issue for it.

Thanks for your patience, I know there's been a lot of discussion going on for this feature, and I'm glad it's getting merged.

Thanks!

[dgkanatsios]

On the advanced options, yeah, I completely get you. In my mind, the "most wanted" Brigade options should go into the regular brig project create flow whereas the "optional" should go to the "Advanced Options". But, as you say, if you go "advanced options" a secret will be generated whereas if you don't, it will just be empty. Happy to brainstorm on the issue you'll open!
On the timing, etc: no worries, happy to wait/refactor/brainstorm, this is the way we'll get quality features built into the project!