Page MenuHomePhabricator

Implement complex variable replacement DSLs in Harbormaster (application/json, application/xml)
Closed, WontfixPublic

Description

Currently, you can only specify the method of an HTTP request for a Harbormaster Build Step, however some external build systems (such as TeamCity, which I'm targeting), require a post body that specifies build specifics instead of URL params. It would be nice to have a text box in an HTTP build step that a user could provide text of type application/xml or application/json and when the build is triggered by Phabricator, this body is sent with the HTTP POST.

I would also like to include the already provided injectable build variables into this POST body if the user deems it necessary.

Event Timeline

halojedi20 renamed this task from Implement post request body text box to Implement POST request body text field.
halojedi20 updated the task description. (Show Details)
halojedi20 added a project: Harbormaster.
halojedi20 added a subscriber: halojedi20.

Does adding variables with ?var=value to the URL definitely not work?

Correct, I've looked into hacking it with url params but it's a no go.

Oh, you need application/json or other weird stuff.

See also T9456.

So, @epriestley, I'm actually building a Harbormaster build plugin for TeamCity as we speak and this is one of the few things left I need. I've completely spec'd and am about 80% finished with the plugin on the TeamCity side.

I think this is generally reasonable, but this is the tricky part:

I would also like to include the already provided injectable build variables into this POST body if the user deems it necessary.

I think we should support this, and we can't do this naively. I think we have to write "JSON + Variables" and "XML + Variables" parsers and build parse trees for stuff like this to work:

{
  "key": ${value}
}

We could do it like this instead, but I think this is generally fairly awkward and undesirable (that is, the behavior seems very surprising to me):

{
  "key": "${value}"
}

See some related discussion in T8975.

This is trickier in XML because the correct way to escape the value depends on where it appears:

<${value} />
<tag ${value} />
<tag attr="${value}" />
<tag>${value}</tag>

This seems like a very difficult problem to me.

A possible alternative is to use a dedicated "Run TeamCity Build" step, similar to the "Run CircleCI Build" step from T9456. (If that stuff lands, writing this build step would get a bit easier since it pulled some of the logging/HTTP request stuff up a level.)

You can definitely do this in a hard-coded way to get things working for your use case (just copy the existing "MakeHTTPRequest" build step into phabricator/src/extensions/, then rename it and adjust the content type / payload -- the CircleCI build step in T9456 / D14286 has an example). This might not be very general but could potentially get things working for now until we write a "JSON + Variables" parser.

epriestley renamed this task from Implement POST request body text field to Implement complex variable replacement DSLs in Harbormaster (application/json, application/xml).Nov 30 2015, 4:48 PM
epriestley triaged this task as Wishlist priority.
epriestley moved this task from Backlog to Far Future on the Harbormaster board.
eadler added a project: Restricted Project.Aug 3 2016, 5:31 PM

@eadler, does your use case need a fancy content type like application/json, or just plain old www-url-normal-web-form-encoded?

plain old www-url-normal-web-form-encoded

Ah, that one's easier.

This is also likely easy to build as an extension (e.g., "Run build with X"), although we don't have a great template for it right now -- HarbormasterCircleCIBuildStepImplementation and HarbormasterHTTPRequestBuildStepImplementation are both sort of halfway toward what you probably need, and I think getFieldSpecifications() may get replaced with something more EditEngine-flavored in the future.

Are you targeting a custom system, or some widely deployed system?

We're targeting jenkins. We just need to kick off a jenkins job.

I'm not super familiar with Jenkins -- what makes this job require parameters in the body? Other users seem to be using Jenkins successfully without requiring this capability on the Phabricator side, as far as I know.

I suspect that others don't need parameters in the kickoff process? Let me look into it. Maybe we have not explored Jenkins in detail.

I think this is a perfectly reasonable capability to want in the general case, I just want to make sure that this isn't a big fancy solution to a problem that could also be solved by checking a checkbox or making a one-line change to the build or whatever. Definitely possible that most Jenkins users are just doing very basic stuff with it, though.

after a deeper look it seems there might be an option to use a token for this. Let me play around with this a bit more

yeah, there is an option to enable building via a specific URL and a GET request. Cool

this relies on the "run with token as root" extension (https://wiki.jenkins-ci.org/display/JENKINS/Build+Token+Root+Plugin) which mostly bypasses other authentication mechanisms. This isn't that much of a problem in our environment, but is clearly non-optimal

In my jenkins setup, I wanted to keep auth, so I did this:

  • Disable all the Build With Tokens options
  • Enabled Jenkins SSH-CLI plugin (Which lets you SSH into it and run commands)
  • Wrote my own "start a build" command, which is also returns enough information to keep track of the build (Queue ID, which the http-invoke method does)
  • Wrote a JenkinsSSHBuildStepImplementation that uses that build via SSH, and has a funny JSON field for the arguments.

The ssh command uses -Pparam=value CLI arguments.

It's all at least a little rough and not very pretty, but I can clean up most of it.

(T12812 wants custom headers, but the only use case appears to be better covered by T12813.)

T12812 would also be beneficial for Jenkins users. The Token Root Plugin works but is an issue for our environment, as we would like to expose our Jenkins instance to the outside world. A better solution for us would be to use HTTP authentication, but Jenkins's CSRF Protection requires us to send an extra header.

Another use case :

In Jenkins using git or hg plugins you can have an anonimous call to http://jenkinsserver.com/mercurial/notifyCommit?url=${repository.uri} to launch new builds but this make the server to check all the jobs to find the correct one to run (as you are only telling it that there are changes in a repository), then check if there are commits (as it's anonimous, it doesn't trust the call), etc.

As an alternative, they also give the option to have a POST call and in the body put the uri, branch and changeset so it doesn't need to search so much. And is this second approach who would need this task to be completed.

epriestley claimed this task.

We have no open customer requests for this and it's very complex, so I don't currently plan to implement it.

So far, customer request have all been more easily solved in other ways (like T13424).