Add more attributes for most of the Scaleft Configuration Options #8
Conversation
|
This LGTM. Ping @martinb3 because you touched it last 😄 |
| **Note:** The `calculate_*` functions are a placeholder for how you would | ||
| determine the values of the various parameters at chef-client runtime. | ||
|
|
||
| Remeber to add `depends scaleft` to your `metatdata.rb` and this repo to your |
There was a problem hiding this comment.
Looks like 2 typos here -- remember and metadata.rb.
| # Cookbook Name:: scaleft | ||
| # Recipe:: sftd_config | ||
| # | ||
| # Copyright 2017, Omnyway and Rackspace |
There was a problem hiding this comment.
Is this a typo or is it really Omnyway and not Onmyway?
|
Can we squash this just to keep the commit history nice and clean? I'm happy to merge once the few comments here are addressed. I'm assuming we should bump major for this, since it could break someone's sftd config if they have one already. |
There was a problem hiding this comment.
I like the idea here, thank you for these changes. I would agree with @martinb3 that if you can also squash these that would be ideal.
| # AltNames is unset by default | ||
| <% end %> | ||
|
|
||
| <% if sft['auto_enroll'] %> |
There was a problem hiding this comment.
Since you set the default attribute to true you shouldn't need this logic. Looking at the documentation this matches the default of true too so looks good.
| # behavior with the original Rackspace version | ||
| # Override it to false to not require 'initial_url' in your | ||
| # environment, role or wrapper cookbook | ||
| default['scaleft']['initial_url_required'] = true |
There was a problem hiding this comment.
I would like to see this removed instead in favor of logic that checks if ['scaleft']['auto_enroll'] is true, then either ['scaleft']['initial_url'] or ['scaleft']['token_file'] need to be set.
Honestly I think we only had this check in place because Rackspace utilizes the SaaS version of ScaleFT so we wanted to catch and enforce using the initial_url. I like making this less opinionated and match the logic of the config file if you are doing this.
There was a problem hiding this comment.
I'm open, I put that in just to make it backwards compatible with what you folks were doing. I always set 'initial_url_required' to false.
Its not stated in the docs, but if you are using the SaaS you don't need to specify the initial_url, it seems to default to the right url. Seems to be only needed if you are not using the SaaS.
We don't set the initial_url on ours, we don't explicitly set auto-enroll (defaults to it) We just set the CannonicalName and the Bastion (and maybe the AltNames). We also associated an AWS Account with each project but that is thru the Web UI. (Just talked to Robert Chiniquy at ScaleFT and he's going to update the docs for Initial_Url to reflect that the default is the SaaS url)
My git-fu isn't up to knowing how to squash the commits/comments at this point.
Let me know if you want me to update the Pull Request by taking out all the logic around initial_url_required
It seems to me to be difficult and maybe unesseccary to have logic as @dude051 proposed. This is because:
auto_enroll is true by default if not specified in the sftd.yml file, and if the initial_url attribute is nil or false in the cookbook. Also as I mentioned if you are using the SaaS you do not need specific the initial_url
I had assumed that the original cookbook had the requirement for initial_url being set because folks at Rackspace was using a Private version of scaleft and not the SaaS. In which case requiring it to be set made sense, That is why I made the original behavior the default and had an explicit mechanism to override that.
Don't know if there is any way to programmatically sense if you had a private ScaleFt service and then require the initial_url to be set.
Added attributes and broke out the creation of the sftd.yaml based on a template into its own recipe.
Created an attribute:
That by default keeps the old behavior of requiring
['scaleft']['initial_url']to be set.If
['scaleft']['initial_url_required']is not set or is false, then it is not required to set['scaleft']['initial_url']. This later behavior is appropriate for the ScaleFT SaaS.The main feature of this pull request is the addition of a template and attributes to set all the other Common and Additional Configuration Options as described in https://www.scaleft.com/docs/sftd/#common-configuration-options