feat(api): add deis tls

[bacongobbler]

closes #579

[deis-bot]

@mboersma, @helgi and @kmala are potential reviewers of this pull request based on my analysis of git blame information. Thanks @bacongobbler!

[helgi]

Why a new model?

[bacongobbler]

The reason behind the new model is that I'd be interested in eventually adding more TLS-specific settings in the future like cipher suites, TLS protocols, enabling/disabling the session cache, session timeout, using session tickets, and the buffer size. Instead of trying to wedge all this logic into one generic "app settings" model, I decided to break it out into a single class so all TLS-related logic can be logically separated inside the controller.

Would you prefer to not split out this logic and just use the existing AppSettings model? I'm just concerned about adding more boilerplate to the AppSettings model, then in the future we decide to break it out because we don't want to do some TLS-specific business logic that doesn't exactly fit into the AppSettings model.

[helgi]

I was mostly wondering the reasoning since the same can be said about health checks, registry and bunch more - they are no longer values but rather their own complex structure in Config. Reason for AppSettings vs Config was no-deploy vs deploy trigger, so this one would have to have a good reason to be independent. If we go for separate at the end then we should evaluate other Config options and consider breaking them out to keep consistency

Do you see this somehow tracking releases? rollbacks, etc involved?

[bacongobbler]

I was mostly wondering the reasoning since the same can be said about health checks, registry and bunch more - they are no longer values but rather their own complex structure in Config.

I strongly disagree with embedding a tls JSON object in the AppSettings model. While TLS is just a bunch of annotations on the service - similar to Domains and Certificates - TLS is a complex object with its own business logic. maintenance and routable are simple boolean fields that are just flipswitches; there's no hard logic/validation required. They are great candidates for AppSettings, but it is very difficult/unwieldly to manage, migrate and maintain a JSON object with jsonschema and some dictionary parsing logic, in my opinion. The only reason why health checks are a JSON object is due to legacy purposes. We supported a json object in v1, so it only made sense to do the same for v2. The rest of the objects I still don't enjoy the fact that they are not concrete classes.

For example, this function in the Config object is a "data migration" from the "v1" healthcheck to the "v2" healthcheck which we must now maintain. If we had gone with well-defined model implementations in the first place, we could've handled this migration much cleanly through a SQL data migration. Future changes to the Config code base will require limits migrations, tags migration, or more healthcheck data migrations which cannot be handled by south.

Another example where JSON objects become hard to manage/maintain can be seen in the scheduler.

Do you see this somehow tracking releases? rollbacks, etc involved?

If you're referring to a "release summary" for the model to inform users what changed in the TLS object, I could see that being part of the TLS's save() function similar to AppSettings, yes. :)

The problem with tying this object to releases means that it's now part of the release process for an application. Since this stands alone to just modify the router annotations, I don't believe we should attach it to the release process unless we intend to consider it part of the application configuration.

[helgi]

I see, earlier in your comments you didn't seem so opposed to put this data elsewhere so I wanted to double check.

I'm working on the autoscale stuff right now and put it into AppSettings, using jsonschema. Seemed far more tangible to me than to create a lot of new (and similar) boilerplate in SDK / API. The scheduler code you pointed at, It's 5 lines, doubt we'd be in a far more solid situation either way, be it well defined model or not. Potentially tho.

All in all, I'm fine with doing a new model but I want it to be very well understood what that means for us. And especially how that will connect up into releases / rollbacks. I don't really care about the summary as much as I would care if this objects is going to be playing a part in a release and thus can be rollbacked or not. As you pointed out it doesn't touch the application code but it touches the application and if I was working on an application and modified TLS ciphers then I would want to be able to rollback if things went wrong (in fact in the last year I had the exact same scenario above).

Maybe we are back in the territory of having multiple diff Models connected to the Release object and having release versions (due to the append nature of the ledger) that are in fact not going to trigger a release unless you are rehydrating a server. I know this is thinking a bit far ahead but again, trying to have this well reasoned out.

As far as the data migrations. We haven't used south since Django 1.6 and we are on 1.10 now ;)

[codecov-io]

Current coverage is 86.68% (diff: 96.05%)

Merging #1004 into master will increase coverage by 0.09%

@@             master      #1004   diff @@
==========================================
  Files            28         29     +1   
  Lines          3244       3282    +38   
  Methods           0          0          
  Messages          0          0          
  Branches        552        555     +3   
==========================================
+ Hits           2809       2845    +36   
- Misses          287        288     +1   
- Partials        148        149     +1   

Powered by Codecov. Last update 9b8ede9...7204711

[helgi]

Migration needs to be renamed and based off the master since #1010 got merged in

[bacongobbler]

done

[helgi]

Doesn't this also need to depend on 0014? Suppose the migration system could be smart enough to resolve things

[bacongobbler]

fixed immediately after I pushed