Skip to content
This repository has been archived by the owner on Dec 31, 2022. It is now read-only.

Support arbitrary customization of webserver site configuration #101

Merged

Conversation

nickmarden
Copy link
Contributor

@nickmarden nickmarden commented Aug 25, 2017

Allow the user to deploy multiple applications on a single server,
each listening on a distinct port:

app['webserver']['port'] and app['webserver']['ssl_port'] (default
80 and 443, respectively), as well as the corresponding global values,
will allow the user to override the port that the application listens to
in Apache2 or nginx.

Allow user to override which template is used to generate Apache2 or
nginx per-app webserver configurations (aka "site configs"):

app['webserver']['site_config_template'] and
app['webserver']['site_config_template_cookbook'] (defaults are
appserver.#{webserver}.conf.erb and opswork_ruby, respectively), as
well as the corresponding global values, will allow the user to
specify which template (from which cookbook) should be used to render
the webserver site configuration.

Fixes #100

TODO:

  • Write integration tests

@coveralls
Copy link

coveralls commented Aug 25, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 53ad129 on RapidRiverSoftware:nick/site-config-customizations into 5568f6c on ajgon:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 53ad129 on RapidRiverSoftware:nick/site-config-customizations into 5568f6c on ajgon:master.

Copy link
Owner

@ajgon ajgon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really, really like the idea. Can you also add one or two integration tests for this config options?


def generate_appserver_config(opts, source_template, source_cookbook)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why passing separate variables here? Can we stick to using opts only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opts get passed to the cookbook template itself at L93, and I figured that the cookbook template didn't need to know the source_template and source_cookbook values to do its work.

@nickmarden
Copy link
Contributor Author

Can you also add one or two integration tests for this config options?

@ajgon What additional tests would you like? I'm unclear from the context of your question what "this config options" is.

@nickmarden
Copy link
Contributor Author

(I should add that I'm happy to add any tests you'd like - I'd really love to have my changes merged to master!)

@ajgon
Copy link
Owner

ajgon commented Aug 29, 2017

I was thinking about integration test, which actually sets custom ports, template and cookbook source. From my experience with chef, unit tests usually doesn't catch all the quirks. Integration test sets up fully functional chef environment, so I trust them way more.

@nickmarden
Copy link
Contributor Author

Ah. I did do template-level testing in the test that I wrote, showing that the VirtualHost and/or Listen/listen directives get rendered correctly. I also tested that when you override the cookbook source, that the template is pulled in from the specified cookbook.

For what it's worth I am actually testing this in a real chef environment and it works exactly as expected, but it's hard for me to see how to replicate that within the testing framework here. Do you have something in mind?

@ajgon
Copy link
Owner

ajgon commented Aug 29, 2017

Do you have any experience with chef integration testing? Because if not, that's okay - I'm going to merge all of your hard work anyway, and I can write those tests for you :)

The format is actually pretty simple, take a look here for example: https://github.com/ajgon/opsworks_ruby/blob/master/test/integration/default/serverspec/default_spec.rb - your changes would probably need a new scope for that. Maybe one test to rule them all, with passenger, templates and ports?

@nickmarden
Copy link
Contributor Author

Oh my, sorry. I had not noticed the test/ directory and was doing all of my spec work in, uh, the spec/ directory. That's embarrassing! :-/

Let me take a look and see how hard it would be to add some integration tests.

@ajgon
Copy link
Owner

ajgon commented Aug 29, 2017

Yeah, totally understand, chef cookbooks use this directory structure for tests for whatever reason... which is confusing.

@ajgon
Copy link
Owner

ajgon commented Aug 29, 2017

Oh, and if it's easier for you, you can create a new PR with integration tests, then I can close and merge all those PRs right now!

@nickmarden
Copy link
Contributor Author

No, no...make me do the work. I'll churn through it today - I really want to get these PRs closed so that I can move on with the client work that is driving me to make all these changes.

@ajgon
Copy link
Owner

ajgon commented Aug 29, 2017

Awesome, thanks!

I'm going to bed now, so no rush. I'm going to review and merge all of them tomorrow, and create a new release as well.

@nickmarden nickmarden force-pushed the nick/site-config-customizations branch from 53ad129 to da53e69 Compare August 29, 2017 20:53
@coveralls
Copy link

coveralls commented Aug 29, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling da53e69 on RapidRiverSoftware:nick/site-config-customizations into 89ce561 on ajgon:master.

Allow the user to deploy multiple applications on a single server,
each listening on a distinct port:

`app['webserver']['port']` and `app['webserver']['ssl_port']` (default
80 and 443, respectively), as well as the corresponding `global` values,
will allow the user to override the port that the application listens to
in Apache2 or nginx.

Allow user to override which template is used to generate Apache2 or
nginx per-app webserver configurations (aka "site configs"):

`app['webserver']['site_config_template']` and
`app['webserver']['site_config_template_cookbook']` (defaults are
`appserver.#{webserver}.conf.erb` and `opswork_ruby`, respectively), as
well as the corresponding `global` values, will allow the user to
specify which template (from which cookbook) should be used to render
the webserver site configuration.

Fixes ajgon#100
@nickmarden nickmarden force-pushed the nick/site-config-customizations branch from da53e69 to 4efd130 Compare August 30, 2017 00:57
@nickmarden
Copy link
Contributor Author

@ajgon per my commit on #102, this PR contains/is built upon the maximum_override suite introduced in #102. So please review/merge that PR before this one, and then it will magically become a one-commit PR.

SSL requests are enabled. Note that SSL itself is controlled by the
``app['enable_ssl']`` setting in Opsworks.

- ``app['webserver']['site_config_template']``
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a graceful to test the site_config_template/site_config_template_cookbook stuff in the integration tests without introducing another cookbook or at least some more recipes into the opsworks_ruby code base, so I am going to trust that the unit tests for these two attributes are sufficient.

Anyone mucking around with such hardcore customizations can feel free to write an integration test if they discover that I've somehow borked this part ;-)

@coveralls
Copy link

coveralls commented Aug 30, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 4efd130 on RapidRiverSoftware:nick/site-config-customizations into 89ce561 on ajgon:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4efd130 on RapidRiverSoftware:nick/site-config-customizations into 89ce561 on ajgon:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4efd130 on RapidRiverSoftware:nick/site-config-customizations into 89ce561 on ajgon:master.

@ajgon ajgon merged commit 8dda62d into ajgon:master Aug 30, 2017
@nickmarden nickmarden deleted the nick/site-config-customizations branch August 30, 2017 21:31
dotnofoolin pushed a commit to dotnofoolin/opsworks_ruby that referenced this pull request Nov 23, 2021
…-customizations

Support arbitrary customization of webserver site configuration
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants