Begin implementation of Enterprise ManagementConsole API

[ryangribble]

This is the beginnings of adding support for #1024 the GitHub Enterprise Management Console API

To begin with Ive only added the Get/Set Maintenance Mode functions and the actual plumbing around how we can actually integrate with the management console API.

Im aware there are merge conflicts as I havent rebased to latest master yet. We are far from done with this PR so Ill do that in good time. For now I just want to get the [WIP] up to find out from @haacked and @shiftkey how far off track i've taken things :

Notes

This API is significantly different to all the "regular" github APIs. There are special requirements around connectivity and authentication and it also doesnt have fully fleshed out calls that take multiple parameters, but in more than one place simply have a single parameter that takes a json string specifying multiple parameters. To top it off it had some really strange handling around date formats and some other things.

Im definitely after guidance on some of the design decisions ive made...

Summary

Read below for large detail on the inner workings and design decisions I made, but in a nut-shell someone using octokit.net against a GitHub Enterprise server an now simply call the following methods on the same GitHubClient client they have already created for regular API access.

// Some examples...

var consolePassword = "MyPassword";

// Get maintenace status
var maintenance = await 
    _github.Enterprise.ManagementConsole.GetMaintenanceMode(consolePassword);

// Set maintenance mode ON now
var maintenance = await
    _github.Enterprise.ManagementConsole.EditMaintenanceMode(
        new UpdateMaintenanceRequest(true, MaintenanceDate.Now()),
        EnterpriseHelper.ManagementConsolePassword);

// Set maintenance mode OFF
var maintenance = await
    _github.Enterprise.ManagementConsole.EditMaintenanceMode(
        new UpdateMaintenanceRequest(),
        consolePassword);

// Schedule maintenance for next wednesday 5pm
var maintenance = await
    _github.Enterprise.ManagementConsole.EditMaintenanceMode(
        new UpdateMaintenanceRequest(true, MaintenanceDate.FromChronicValue("Next wednesday at 5pm")),
        consolePassword);

// Schedule maintenance mode in 10 minutes, with a DateTimeOffset
var start = DateTimeOffset.Now.AddMinutes(10);
var maintenance = await 
    _github.Enterprise.ManagementConsole.EditMaintenanceMode(
        new UpdateMaintenanceRequest(true, MaintenanceDate.FromDateTimeOffset(start)),
        consolePassword);

Network access

• TCP Port
  Since octokit handles following redirects, we don't have any issues around handling the special ports of 8080 and 8443 so we are good on this front and can simply target this API at the regular server address the user is passing into the GitHubClient as octokit will follow redirects as needed
• API Uri's
  Not as lucky on the Uri front though... The management console api calls must be targetted at http://my.enterprise.server/setup/api ie they can't have the /api/v3/ in the URL as all other calls do.
  Unfortunatley in the guts of octokit.net's GitHubClient and Connection classes, this /api/v3/ is added to any custom URL specified by the user and saved in the baseUri of the Connection object.
  Originally my implementation made it so that you had to declare a new/special GitHubClient using a new constructor essentially stating that you wanted to use it for management console access, and this essentially passed a bool through which prevented the /api/v3/ being put in the uri. But of course this then meant that all the other API clients on this GitHubClient wouldnt actually work... and similarly it meant that the client.Enterprise.ManagementConsole client wouldn't work on a "regular" GitHubClient you were using to call the normal APIs.
  My latest iteration was actually exploiting behaviour of the dotnet Uri appending behaviour, where if an endpoint relative Uri starts with a leading forward slash, it actually causes any Uri on the baseUri to be stripped off. We've encountered the odd bug over time where inadvertently including these leading slashes on Urls in the ApiUri helper class has caused endpoints to not work on github enterprise, but now it seemed like this behaviour might actually help us here. Rather than just magially include the leading slashes on the management console Uris, i implemented a function CorrectEndpointForManagementConsole(endpoint) that is called in each management console client method, to add the leading slash if not already there. This does seem a bit hacky but really it is a fairly reliable way to allow a user to specify one GitHubClient object and credentials that work against the regular APIs on their enterprise instance, then when they use the ManagementConsole API on the same client, we manipulate the Uri via the endpoint (ie not manipulating the baseUri) and provide the management console password each time. Plus there is zero behavioral change for anyone using octokit against github.com (ie not an enterprise user). As an octokit contributor and enterprise user I felt like this has achieved good usability and is an acceptable code base compromise. At least it's a starting point to see how @shiftkey and others feel :)

Authenticating

The management console has a special password to access it. For API use, this password is specified as the api key, however it appears it cant be specified in the "normal" way the user/password or OAUTH token can be (ie using our Credentials object which inserts it in a header in base64/UTF8 form). I was only able to get things successfully working, by having ?api_key=MyPassword on the end of the Uri (or potentially it would also work being embedded in the server name https://api_key:MyPassword@hostname but that causes similar problems as above if we want to try and have the one GitHubClient object able to work with all APIs
So with the way i've implemented it on the end of the Url, it actually doesnt work too badly as it means the existing credentials provided on creating the GitHubClient can stay, and are used for regular API access, and every management console method takes a managementConsolePassword parameter. I actually like this as it's more inline with the seriousness of some of these management console activities - eg along the lines of "provide your password everytime you do these management related things"
The irksome thing is that the password is essentiall in clear text on the end of the Url but im not sure if that's really any different to it being in the HTTP headers for regular calls anyway...

What the heck is up with parameters containing json strings?!!

The Set Maintenance Mode call takes a single maintenance parameter which is defined as follows:

maintenance string Required A JSON string with the attributes enabled and when.

I tried and tried and I just could NOT get things working in the normal way where we have a request object, which gets serialized to json and put in the body of the HTTP Post. I had many incarnations providing a class with a public Maintenance parameter that then contained the c# object class with the Enabled and When parameters on it. When that didnt work I tried simply having a public string Getter parameter that deserialized the class to json (didnt work), so then moved to constructing the json string in a more basic way - both normal format and fully escaped, I tried doing the parameter dictionary type approach like used in search requests, I tried the default content type and application/json content types but in all cases github would throw a HTTP 400 Bad Request.
Eventually I found that if I just specified the string in the url-form-encoded format of maintenace="{"enabled":true, "when":"now"}" it worked, so that's where I'm at at the moment. I plan to tidy up the json string to not be created manually and instead call a serializer but for now I have something working so didnt want to wait in pushing it up.
If anyone is able to show me how to get this working in a more "normal" way for octokit where the request object is serialized using the builtin SimpleJsonSerializer in the normal pipeline, I would love to see what I missed bercause I feel like I tried so many attempts and still couldnt crack it without going the form encoded route.

This is definitely important to sort out a strategy for, because the Settings endpoint on the management console is a real doozy containing literally 50+ items in a hierarchical json structure that all have to get packed into a single json string on a settings property in the API call. I am NOT looking forward to that endpoint!

What the heck is up with the time handling?!!

The response to a scheduled maintenance mode event includes this field
"scheduled_time": "Tuesday, January 22 at 15 => 34 -0800"
In any other github API call ive encountered, the dates are in a lovely and easily parseable ISO 8601 format
In this case we not only have the the full day and month names (this will pose issues if github can run in other cutures/languages), the silly formatting like comma and => the non standard timezone offset information (in dotnet land this should be -08:00 rather than -0800 but most importantly THE YEAR is missing! This plays havoc with attempting to actually parse this into a date. Now if we hardcode a culture like en-US we can get around the full day/month names, and we can "almost" use ParseExact() to match most of the formatting issues, except the lack of year means we cant use DateTimeOffset.ParseExact() but if we use DateTime.ParseExact() the time zone offset is not handled well.
Given that the day of week and month are mentioned, we "could" potentially take an educated guess at the year, since eg if it says Tuesday the 12th April, we can figure out what YEAR in the future the 12th April will fall on a Tuesday and presume it to be this... then we could shove that year into the string. This is clearly getting ridiculous though (espcially because this field is coming back from the API and we want it to be deserialised properly in the Json deserializer) so I took the easy way out and simply made it a string!!!
In runtime testing this behaviour it's even more odd because through the API I can scheduled maintenance for a date in 1/1/2017 and the API response Sunday, January 1 at 20:00 indicates it really is that 2017 date - the fact it says Sunday is the confirmation. Yet the management console UI will show it as 1/1/2016 but in a scheduled status. If it really was 20-16 we should either now BE in maintenance mode (not scheduled) or it should reject the update since it's in the past. So im thinking even the management console UI itself is using this API call with the missing date and just assuming it's the current year.
Now does it make sense to schedule maintenance for so far in the future? Not really! But given the API lets you do it, it's not really our place to limit this in any way. My hope is that this API call gets modified to include a properly parseable ISO 8601 formatted date like all other dates in the normal APIs, and the management console GUI fixed up to know the proper year as well 😀 In the meantime Im thinking we have to leave this field as a string because parsing it is too difficult and not 100% reliable.

Other design decisions

• Specifying "when" parameter in EditMaintenanceMode()
  The SetMaintenance method when parameter can handle "any" string that the mojombo/chronic ruby library can parse. This includes lots of humanised things like "next tuesday at 5pm" and "3pm on the monday before friday in 3 weeks time". Luckily for our api case we simply have to take whatever string the user says and pass it through. But ive also added specific suport to passing in a DateTimeOffset object since that is more aligned to our c# nature, and also the special now keyword as indicated by the API docs. To do this i created a MaintenanceDate object used as the When parameter on the UpdateMaintenanceRequest object, and helper functions MaintenanceDate.FromDateTimeOffset() MaintenanceDate.FromChronicString() and MaintenanceDate.Now()
• MaintenanceModeStatus as an enum
  There appear to be finite values of off on and scheduled so I made this an enum. The API docs dont definitively state the finite values though... This isnt too different to other enums we've implemented though
• Added environment var for management console password for integration tests
  added to the powershell helper script and the EnterpriseHelper class, so the integration tests can run

That's about it for now. Please have a read and provide feedback... I'm looking at you @shiftkey 😀

[ryangribble]

@shiftkey this is another one where im pretty much waiting for you or @haacked or someone "official" to provide some feedback/help, if you can add it to your list 😀

[ryangribble]

@shiftkey just a gentle reminder about this PR 😀 I know you've been travelling etc, I've been a bit preoccupied with some other things recently myself, but should be ready to get back into finishing off the enterprise api's now, pending advice/feedback on what I'd found with this PR

[shiftkey]

🔥 - the next assert will cover this

[ryangribble]

thanks for the comments. Ill revisit this soon with those comments in mind and see what a fresh perspective brings.

I think I've already got a branch somewhere where I at least used the serializer to build the JSON string but from memory I still couldnt get it to be accepted in the request body and had to go with the URLEncoded approach.

[ryangribble]

OK I think all comments have been addressed

I've also changed the request object side of things so the structure matches what the API expects, and I can use SimpleJsonSerializer to build the json string.

The usage is now new MaintenanceUpdate(new MaintenanceUpdateDetails(true, "next tuesday at 5pm")) - the nested class isn't ideal but that's how the API object needs to be (ie top level item name maintenance that then contains the enabled/when attributes).

I still need to UrlFormEncode the request rather than have it in the request body, so Ive made a new class similar to RequestParameters that helps with this and can be used on future ones too (eg the management console settings API). So at least it's now using a consistent/standard approach to generating the encoded parameters and using the SimpleJsonSerializer too

[khellang]

Why is this setting it to the exact same thing as the ctor?

[ryangribble]

this is a helper context class to be used via a using statement wrapping a test. So a test that sets maintenance ON, needs to ensure that maintenance was OFF to begin with and vice versa, hence the intial state and setting it in the ctor.

But also we dont want to leave GHE in maintenance mode after a test (since that is pretty useless for any subsequent tests) so the destructor should be ensuring maintenance mode is OFF again. Which make me realise Im currently setting the intial state in destructor but actually should always be setting OFF 😀

[khellang]

Which make me realise Im currently setting the intial state in destructor but actually should always be setting OFF 😄

👍

[ryangribble]

Rebased and assigned to @shiftkey for review

[shiftkey]

Moving this to #2010 to see if we can actually land this