Feature (rhel7/httpd 2.4) : hardening apache and code refactoring

[k-hamza]

Context
The main goal of the changes is to be able to enforce values of directives for hardening purpose, based on CIS Benchmarks apache document
The structure of pillars is modified into lists / dictionnaries so the content can be "easily" parsed and modified

The result is a complete rewrite of config and vhosts states

All changes are backward compatible, they are all new files added (with -ng suffix)

Summary of Changes

• added apache_directives.py module to manage both content of server config and vhosts content
• vhosts contents are in managed in dictionnaries and there is no more need to maintain templates by type of vhost
• enforced values are in yaml file
• default server configurations (for rhel7, http 2.4) are in yaml file
• changed vhostdir variable to /etc/httpd/conf.d because of /etc/httpd/vhosts.d doesn't belong to the httpd-2.4 rhel7 rpm

Testing

• Tested on rhel7 httpd 2.4

[aboe76]

there are hard returns in modules-ng.sls I don't think you want those...

[k-hamza]

yes, you are right. Thanks !

[alxwr]

@k-hamza I greatly appreciate your effort! Your code even solves some problems I came across. Thank you!
(The following is not a criticism of your work, but of the "ng" pattern in general.)

@aboe76 @whiteinge @javierbertoli @landergate
Is there a way not to use the "ng" pattern? In my opinion this leads into an maintenance mess.
To be honest I regard the "ng" pattern as some sort of redundant addition to Git. It's really a formula within a formula. Hard to understand, hard to maintain. In this case I can't easily see the changes, because everything is in a new file.

We already discussed this in saltstack-formulas/postfix-formula#83. (@ixs)

I suggest we start versioning this formula. (https://semver.org) Let's add a tag (i.e. 0.1.0) to mark the state before this PR and after that (depending on possible breaking of Pillar data) set 0.2.0 or 1.0.0.
People could reference the tags. Patches to old versions (if anyone wants to keep them) would go into branches like v0.1.0-master. From there they could be ported to other branches way easier than from one sub directory to another.

The only real disadvantage I can think of is having to write a migration guide from one version to the other.

If we really have to use the "ng" pattern, we should put everything into its very own namespace (read: directory) as the php-formula does.

In the hopes of providing constructive criticism,
alxwr

---

@aboe76 @k-hamza
I'll have to test the "ng" version of the formula on one of my machines. (Sadly I've got to finish other stuff first.)

[aboe76]

@alxwr I know about the issue, with "ng" formula's and I would like to split those too.

I don't think that semver and git branches are the solution, those are to "developer"-minded solutions and are hard for simple sysadmins to grasp or even create PR against.

I would like to suggest to create separate formula's:

php-formula\
  php\
  LICENSE
  README.rst

php-ng-formula
  php\
  LICENSE
  README.rst

apache-formula
  apache\
  LICENSE
  README.rst

apache-ng-formula
  apache\
  LICENSE
  README.rst

Only downside to this is if somebody wants to use both formula's on one saltmaster...

[javierbertoli]

@alxwr, I completely dislike the ng solution mostly because, as you and @aboe76 mention, it follows a 'formula-in-formula' concept. But it has already crept into many formulas (from the top of my head, I rememeber it's present in apache, fail2ban, nginx, php...). It seems most of the times as the only solution to 'here's this big refactoring/update to the formula, it's a pain/impossible to make it backward compatible and there's no other proposed method in the formulas ecosystem to introduce these changes nicely' 😞

At this point, separating the formulas can be a solution, but I think that, sooner or later, somebody will again submit a 'formula-in-formula' 'ng-ng' PR, like 'apache-ng/ng' 😄

In the past, when writing Puppet & Chef, the Semver + Release Tags/Branches worked nice for me, but I again agree with @aboe76, it's not the easiest thing to learn how to use and interact for simple sysadmins (it took me a while to do it right 😋)

If we decide to do SemVer, we'll need to add proper documentation (perhaps in the template mentioning that:

• The master branch will always point to the latest code and might be backward incompatible
• Release tags should be used (ie, in gitfs), which might or might not be updated in time (that will depend on whoever pushes a PR)

etc, and try to enforce/maintain things that way whenever possible.

When I moved to Salt, I tried to read about the proposed method to maintain formulas, and found that Salt has a Salt Package Manager, but sincerely, never used it nor know if it is widely used.

My 2c.

[alxwr]

@aboe76 @javierbertoli I hear your concerns regarding sysadmins. Those are my concerns too. :-)
In my opinion it's not too much to ask/force somebody to learn how Git branches work. (But I may be wrong.) This is state-of-the-art technology after all. Furthermore, ng/ is confusing too¹.

I'm not in favor of creating a new repository for every refactoring, because it's not easier to maintain for anyone.

I also was thinking about doing version control in sub directories like so: v1/apache/..., v2/apache/...
Current development would happen in the latest version directory. It's still a maintenance mess, but it seems better than ng/ng/ng/.../ng/.

The Formula File docs suggest the version format YYYYMM.release. This contains no assurance of compatibility anyways.
"We strongly recommend forking a formula repository into your own GitHub account to avoid unexpected changes to your infrastructure."
Since formulas never made a promise to be backwards compatible, nobody should use a potentially unstable master anyways.

Maybe we should give up on „be backwards compatible at any cost“.¹
In my view there are at least two effective options to kill an Open Source project.

a) Make it too hard to use.
b) Make it too hard to maintain.

I get that keeping backwards compatibility tries to avoid a), but it drives the formulas to b). (And furthermore: ng/ng/ng/ goes against a) again.)

Therefore, another proposal:

• Use only the master branch.
• Set tags before breaking changes happen. (or before PRs are merged)
• Document breaking changes in a CHANGELOG.rst (including migration guidelines)
• Let the formula fail early if we detect legacy Pillar data.
  (i.e. detect whether there is Pillar apache:sites)

---

Don't take me wrong. I'm not here to enforce my point of view. I'm just committed to keep usability high and maintenance cost low, because I want the formulas to succeed. In the end, I'll go with the developers' consensus. :-)

---

¹ Why do I have a strong opinion on this?
I held a one-day Salt workshop for sysadmins of several departments of a German corporation a few months ago. They really liked Salt and especially the formulas. They had two concerns regarding the formulas:

• Sysadmin: How do I keep my system stable?
  Me: You fork/clone the formula's repository.
  Sysadmin: That's fine.
• Sysadmin: Whats this ng/-stuff?
  Me: Those are major refactorings.
  Sysadmin: You should really get rid of legacy code, because this is really confusing. How would I know which one is current? (In German: „Ihr solltet diese alten Zöpfe abschneiden, das führt nur zu Verwirrung. Wie soll ich wissen, was aktueller Stand ist?“)

[ixs]

My 2cents as I got pinged. ;-)

It's good to see everybody agree that ng sucks and should not be used.
I really dislike the idea of forking off an ng change into a separate repository. This is mostly for the following two reasons:

• Proliferation of formulas doing the same thing in different ways
• Becomes a nightmare as soon as someone is doing a ng fork of the ng version. Look at amavis, amavis-ng, amavisd-new etc. as a terrible example.

But even in-repo -ng is terrible. I stumbled over the ng version of fail2ban the other day and while that version resolves a few bugs of the non-ng version, it was also not functional. Needed one or two patches first.
Does this mean noone was using -ng at all other than me?

At the end of the day, I think semver and controlled breakage is the sensible option.
And for the poor busy and overwhelmed sysadmin we can add some easy to understand documentation that explains how to update your formula and again suggest to use your own fork in order to control the rate of change.

[aboe76]

Ok, let's ask saltstack Organisation to make it clear what they have in mind with the formula's,
because there is something like spm https://docs.saltstack.com/en/latest/topics/spm/

And I too want the saltstack-formulas be as successful as it can be, because, one, I don't want a mess of multiple roles like ansible-galaxy and I don't want it to become another forge where you need to learn some pdk to submit...

@saltstack-formulas/core and @saltstack-formulas/moderators can we have some input on this so we can align all our efforts?

[javierbertoli]

A valid question that relates to this discussion

[aboe76]

@javierbertoli yes it is, I was hoping to get the saltstack core team involved to maybe write something in saltstack docs on formulas and git tags...and how to use them, that way we can start teaching people on how to use them by letting them read the official saltstack docs.

[k-hamza]

I must say that I agree with you on "ng" pattern ... it is an "anti-pattern" of git usage

In the saltstack documentation I found this about formula versionning.

• use semver
• use tag for versions tracking
• The CHANGELOG.rst file should detail the individual versions, their release date and a set of bullet points for each version highlighting the overall changes in a given version of the formula

It seems that it is the "official answer" to the question, and it is mostly the same as the proposals above.
May be it would be usefull to report it on the template formula

What do you think about it ?

[noelmcloughlin]

I'm think if there is any way to keep one-on-one relation between formula and implementation then we should aim for that. When rewriting the mongdb-formula recently I had this exact dilemma - how to improve without losing backwards compatibility - I eventually found a way to support old and new in same implementation by having a twin-track map.jinja

See section two for backwards-compat data structures:

In this pattern the first section is the current-preferred-implementation and the second section is a reconstruction of the depreciated dicts to keep backwards compatibility. In this way there is no need for two separate formulas for one piece of software. I will admit writing the SECOND section in map.jinja is challenging but advantage is that its transparent to end users. I'd advocate for some kind of similar approach - let map.jinja do the heavy lifting. Cheers.

[aboe76]

@k-hamza nice finds, in the documentation, let's make it so, I think the majority is for semver and tagging,
can you create a PR to document and link to the docs in the template formula.

@alxwr, @noelmcloughlin and @javierbertoli let's educate people and refer to the template-formula.

[aboe76]

@k-hamza maybe we can try this after we set some tags:
https://github.com/vaab/gitchangelog

[k-hamza]

can you create a PR to document and link to the docs in the template formula.

done : saltstack-formulas/template-formula#16

[k-hamza]

Hello team, hope you are doing well
@alxwr, did you have time to test or to look into the code ?
Thanks !

[alxwr]

@k-hamza I'm trying hard to get some GitHub issues and PRs done. :-)
I'll give your code a try as soon as I can.

[alxwr]

@k-hamza I (rather hastily) tested this on Ubuntu 18.04 by including all ng-states and running them against pillar.example. Other than minor distribution-related bugs this worked fine. I would merge it if it wasn't for the ng-stuff. :-)

@k-hamza @aboe76 @noelmcloughlin @javierbertoli we should now discuss a transition to apache-formula 2.0.
I'm against merging the PR in the ng-style. I propose we rather make this a good example of versioning a salt formula by using the means already established in https://github.com/saltstack-formulas/template-formula.

@myii Is saltstack-formulas/template-formula#21 sufficient to serve as a checklist for this transition?

[alxwr]

I'd approve if it wasn't for the ng-pattern. :-)
We'll work that out.

[myii]

@alxwr saltstack-formulas/template-formula#21 is still a WIP and does need more discussion (no-one has commented there yet for adjustments!). But if it helps with this process, then please feel free to use it by all means. As a member, you are able to edit the comment, in order to copy the Markdown directly. That will ease pasting and modifying it for any other formula.

[myii]

Sorry, I was checking for the other labels and I ended up removing the label by mistake! Reinstated, so nothing to see here!

[alxwr]

minor note:
I'm looking forward to help with this PR, but I've already burnt through my GitHub time budget for this week. 😄

[myii]

@alxwr Your efforts are appreciated. See you next week!

[noelmcloughlin]

Nice PR @k-hamza I'm up to my neck in other formulae so cannot review.

@alxwr Regarding the ng pattern I have good idea how to smoothly transition away.

1. Identify all ng-formulae.
2. Assume ng is the primary content.
3. Obsolete non-ng content but empower the non-ng API

•    i.e.  update non-ng SLS files so they invoke `apache.ng.xxxx` states
  

•    i.e.  delete apache/map.jinja and yaml files in non-ng formulae.
  

•    i.e.  `apache.init`  points to `apache.ng.init` 
  

•    i.e. update README and stop documenting ng API
  

•    i.e. update pillar.example and stop documenting ng pillar data 
  

Note: we need to update the map.jinja so it can handle both ng and non-ng pillar data - this is possible I have experimented with this idea in mongoDB.

4. In say 6 months time get rid of ng structure; the ng API has not be documented for 6 months so assume this is long enough for people to have re-adopted the non-ng API.

WDYT?

I hope focus can be on template-formulae before apache 2.0. There are some new concepts & ideas being discussed which I need to get up to speed with anyway. thanks.

[k-hamza]

I agree not to merge it in master branch.
I suggest that you create a "develop-v2.0.0" branch (name for example) where to merge this PR.
In the meantime, i will start cleaning the "ng-" stuff, so the merge will contain "clean" content that will need some little work in order to be a good example of versioning a salt formula.

[myii]

@k-hamza Thanks for all of your hard work.

There have been quite a few developments over the last week or two and we're trying to revamp the way SaltStack-Formulas does things. One of the discussion points has been the ng concept. At the moment, it is still up for discussion how exactly this should be dealt with. For example:

1. Using release tags, just bump the major version and merge into master.
2. Use separate branches for different versions (as you have suggested).
3. Use a separate repo for ng formulas.

A further issue is that we're working on how to integrate these formulas with SPM. That will also have an effect on the final decision.

So the takeaway is to keep track of developments before investing a portion of time into a specific option. Best ways to do that:

• Get involved in the main discussion; the same location is accessible via. Slack/IRC/Matrix:
  • Slack (SaltStack Community): #formulas
  • IRC (Freenode): #saltstack-formulas
  • Matrix (riot.im): #freenode_#saltstack-formulas:matrix.org
• Keep an eye on Establish template-formula as the go-to resource for all formula development template-formula#21 for any changes.

[alxwr]

@k-hamza @aboe76 @myii I'm all for the "develop-v2.0.0" branch, as long as it will get merged into master at some point. :-) This gives us time to stabilize 2.0 while still having a stable master branch. Do you all agree?

[myii]

@alxwr @k-hamza I prefer method 2 as well but I don't know what the repercussions are going to be.

One further point: we're close to providing a solution for automatically updating the changelog, FORMULA file, tags and releases for each PR merge. During this process, we've compared across all existing SaltStack Formulas to work out comparitive "starting" version numbers:

• This is part of the spreadsheet, offering a rough outline of what we're proposing should be applied.

So if that is accepted, this apache-formula is currently v0.37.0. The new branch will then be develop-v1.0.0 accordingly. Please feel free to comment on this issue as well.

[k-hamza]

@alxwr @myii ok then, let's create develop-v1.0.0 branch so we can begin stabilize the code

[alxwr]

@k-hamza @myii I just created the develop-v1.0.0 branch.

[aboe76]

@alxwr nice thanks for that,

@k-hamza shall I change the base branch of the PR to target develop-v1.0.0

this is possible with: https://github.blog/2016-08-15-change-the-base-branch-of-a-pull-request/

[k-hamza]

@aboe76 Thanks for this trick ! just changed the base branch of the PR now
@alxwr let me do some cleaning : remove all the "-ng" stuff

[aboe76]

@k-hamza merged it,

[k-hamza]

Thanks for the merge