cyverse-de / terrain

DE's main API entry-point service
Other
4 stars 9 forks source link

CORE-1744: added the Agave job status update callback endpoint #243

Closed slr71 closed 2 years ago

slr71 commented 2 years ago

The reason for making this change is to update the DE so that Agave job status update callbacks no longer bypass Terrain. When support for Agave jobs was initially added to the DE, Terrain had no way to support unauthenticated API calls, so all job status update callbacks had to bypass Terrain in order to work (or at least to work easily). Terrain can now support unauthenticated calls, so there is no longer a need to bypass it.

The mechanism we used to bypass terrain was to add a route to the nginx instance that ran along side the DE. In current deployments (which all use Kubernetes), this container is deployed as a sidecar container in the old DE UI pod. The old DE UI has been replaced, so it would be nice to simplify the configuration of the nginx instance as well.

Ideally, we'd like to avoid direct nginx configuration (because it's difficult to customize without using a template system) and use something that's easier to customize. The nginx ingress controller should work, but it's difficult to create URL rewrite rules for specific paths. Eliminating this custom path also eliminates one case where the URL has to be modified, which eliminates one barrier to using the nginx ingress controller.

We'll have to get this change in place in the QA environment to do any thorough testing, but some cursory testing was done to ensure that these API calls actually made it through to the apps service:

$ curl -sH "Content-Type: application/json" "http://localhost:9000/callbacks/agave-job/$ID?status=foo&external-id=bar&end-time=baz" -d '{"lastUpdated": "never"}'  | jq
{
  "error_code": "ERR_NOT_FOUND",
  "reason": "job step d1cba0a5-7adb-4d7f-82f4-e706b33119e1/bar not found"
}
slr71 commented 2 years ago

Thanks for the review!