Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Apache access control directives for version 2.4 #1607

Closed
psivesely opened this issue Mar 3, 2017 · 10 comments · Fixed by #5797
Closed

Update Apache access control directives for version 2.4 #1607

psivesely opened this issue Mar 3, 2017 · 10 comments · Fixed by #5797
Assignees

Comments

@psivesely
Copy link
Contributor

The old Allow, Deny and Order directives are deprecated: https://httpd.apache.org/docs/2.4/howto/access.html

-- #1242

This was a PR first, and now is an issue in order to better track it for the 0.4 milestone.

@conorsch
Copy link
Contributor

Sanity check: do we even need to change this? We currently mandate the use of Ubuntu Trusty LTS, which ships with Apache2 v2.4.7. The Apache project is not going to break configuration formats any time soon, nor will Trusty change the Apache version out from under us with little or no notice.

Given the recent expansion of our test suites, I'm comfortable continuing with the configs as currently declared and treating Apache as the stable project that it is. Down the road, we may decide to switch away from Apache, or to a more recent version, if we decide to upgrade Trusty to Xenial (as discussed in #1530). Either way, this issue does not strike me as a blocker for 0.4. The "pending PR" #1242 is badly out of date, likely even beyond the point that an interactive rebase would suffice to incorporate its changes.

Therefore I recommend closing both this issue (#1607) and the corresponding unusable PR #1242, and continuing with other issues on the 0.4 milestone.

@redshiftzero redshiftzero removed this from the 0.4 milestone Apr 24, 2017
@redshiftzero
Copy link
Contributor

I'm with you that this isn't a blocker for 0.4, and I've moved this issue off the 0.4 milestone. As far as closing the underlying PR goes, I think it's fine to close the PR given its age. I've added a note to #1530 about this for future reference so I'm closing this issue as well.

@psivesely
Copy link
Contributor Author

The Apache project is not going to break configuration formats any time soon, nor will Trusty change the Apache version out from under us with little or no notice.

This is not a guarantee. Apache documents:

The Allow, Deny, and Order directives, provided by mod_access_compat, are deprecated and will go away in a future version. You should avoid using them, and avoid outdated tutorials recommending their use.

Which is pretty vague in terms of a timeline. IMO we should play it safe.

Given the recent expansion of our test suites, I'm comfortable continuing with the configs as currently declared and treating Apache as the stable project that it is. Down the road, we may decide to switch away from Apache, or to a more recent version, if we decide to upgrade Trusty to Xenial (as discussed in #1530). Either way, this issue does not strike me as a blocker for 0.4. The "pending PR" #1242 is badly out of date, likely even beyond the point that an interactive rebase would suffice to incorporate its changes.

Therefore I recommend closing both this issue (#1607) and the corresponding unusable PR #1242, and continuing with other issues on the 0.4 milestone.

Our test suites aren't going to catch when a config-breaking package update hits production instances. That said, I think it's fine and perhaps even best we do remove this from 0.4. However, I think that instead of just removing it/ closing the issue and PR as happened, we should have just closed the PR, left this issue open, and moved the milestone to a later one.

@psivesely
Copy link
Contributor Author

To be clear the note is not sufficient IMO because transition to Xenial may never happen (e.g., if we move to Alpine as a container base as part of the proposed CoreOS migration), but the Apache version running in SD will eventually change (unless we switch to nginx, which is something to consider because of its smaller attack surface, though that's another story) and break our config.

@psivesely psivesely reopened this Apr 24, 2017
@psivesely psivesely added this to the 0.4.1 milestone Apr 24, 2017
@ageis
Copy link
Contributor

ageis commented Apr 25, 2017

As a best practice, you want to switch off of any parameter that is explicitly marked deprecated. Also not sure why the PR was described as "unusable" - is that due to changes to the tests?

@conorsch
Copy link
Contributor

As a best practice, you want to switch off of any parameter that is explicitly marked deprecated.

Updating the Ansible templates for the Apache configs doesn't help currently running instances, only fresh installs that occur after we ship the updated templates as part of a new release.

Also not sure why the PR was described as "unusable" - is that due to changes to the tests?

Due to a variety of reasons, summarized as massive churn on development. Yes, the tests have changed (#1616), but so have the Apache template targets themselves (#1614, #1650).

The real problem we need to solve is that we can't update Apache config files on running instances by pushing commits to this repository—we need manual intervention on the part of Admins to apply such changes, in the form rerunning the Ansible playbooks (after checking out the latest git tag and verifying it), and that's not a enjoyable workflow for anyone involved.

The upcoming 0.4 release is geared toward reconciling the develop and master branches such that we can start delivering changes much more quickly. Rather than require all instance Admins to run playbooks regularly, we need to figure out a longer-term solution that enables automatic updates that can make use of known-good vars provided at install time.

@ageis
Copy link
Contributor

ageis commented Apr 25, 2017

Ok, given that so long as the Apache version remains 2.4 (which it absolutely will) nothing will break, then makes sense to put off.

@psivesely
Copy link
Contributor Author

The real problem we need to solve is that we can't update Apache config files on running instances by pushing commits to this repository—we need manual intervention on the part of Admins to apply such changes, in the form rerunning the Ansible playbooks (after checking out the latest git tag and verifying it), and that's not a enjoyable workflow for anyone involved.

Unless we want to put it in a deb package 👹.

@redshiftzero redshiftzero modified the milestones: 0.4.4, 0.4.3 Aug 31, 2017
@redshiftzero redshiftzero modified the milestones: 0.4.4, 0.4.5 Oct 6, 2017
@emkll emkll modified the milestones: 0.6, Product Backlog Jan 22, 2018
@emkll emkll modified the milestones: Product Backlog, 0.6 Jan 22, 2018
@redshiftzero redshiftzero modified the milestones: 0.6, 0.7 Feb 27, 2018
@squeed
Copy link
Contributor

squeed commented Apr 2, 2018

Opened #1607 for moving configuration to the deb.

@redshiftzero redshiftzero removed this from the 0.7 milestone Apr 5, 2018
@conorsch
Copy link
Contributor

Handling as part of #1775. See details in https://httpd.apache.org/docs/trunk/upgrading.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants