Closed jvrplmlmn closed 7 years ago
This is awesome!
And while there is a room for improvements like making some settings configurable, I think this is a great skeleton to start with for st2web
:+1:
Really first feature coming into my mind:
openssl
if it's a valid cert and not a random stringOverall, we can make configurable more settings like autogenerated cert details, ssl_
stuff and much more, but I personally don't think It's very important for initial state. If @shortdudey123 think there are more settings he would like to adjust in real deployment, - I'm OK with that as well.
But good idea is to keep the balance between "default & safe" working state and super-config where users will start shooting in their knees (we see that more frequently while supporting #community Slack) :smiley:
So my ask is to think what's really important (nginx
config is big). Like give settings they can't live without and then expect users to contribute if they need more and more fancy things to configure.
Users should be allowed to specify their own valid certificate
This should be left to a wrapped cookbook IMO, otherwise loaded from an encrypted data bag
We'll need README
changes as well for this PR.
@jvrplmlmn Can you sync with master and maybe provide changes we agreed on?
This is a great feature and I'd like to include it in v0.5
Also keep in mind, since you have write
access in repo, you can create development branches in this repo, rather then using forked PRs. So we could commit directly into your branch, if needed.
@jvrplmlmn Can you sync with master and maybe provide changes we agreed on? This is a great feature and I'd like to include it in v0.5
I'll try to find some time before the end of the week to get all those changes in.
Also keep in mind, since you have write access in repo, you can create development branches in this repo, rather then using forked PRs. So we could commit directly into your branch, if needed.
You're right, I'll adjust my git remotes accordingly 👍
@armab don't want to prolong this PR too much, so I'm quoting you on this:
Overall, we can make configurable more settings like autogenerated cert details, ssl_ stuff and much more, but I personally don't think It's very important for initial state.
This PR adds:
st2web
and nginx
.README
.@shortdudey123 taking https://github.com/StackStorm/chef-stackstorm/pull/45#issuecomment-264234091 and https://github.com/StackStorm/chef-stackstorm/pull/45#discussion_r90507487 into account, are you missing any major stuff that should be part of this PR?
As a first pass this looks good so far There is additional work to be done to generalize the nginx side and not force options, but that can be done in a separate PR for discussion
See https://github.com/StackStorm/chef-stackstorm/issues/24.
This still requires some additional testing, but @armab @shortdudey123 I'd appreciate some initial feedback on how you guys think that is the best approach to tackle this one.