Merge lp://staging/~sdeziel/squid-deb-proxy/remove-unused-acls into lp://staging/squid-deb-proxy

Proposed by Simon Déziel
Status: Merged
Merged at revision: 70
Proposed branch: lp://staging/~sdeziel/squid-deb-proxy/remove-unused-acls
Merge into: lp://staging/squid-deb-proxy
Diff against target: 14 lines (+0/-4)
1 file modified
squid-deb-proxy.conf (+0/-4)
To merge this branch: bzr merge lp://staging/~sdeziel/squid-deb-proxy/remove-unused-acls
Reviewer Review Type Date Requested Status
Michael Vogt (community) Needs Information
Review via email: mp+102893@code.staging.launchpad.net

Description of the change

The provided squid-deb-proxy.conf includes an ACL for "all" but that ACL is now built-in as mentioned in http://<email address hidden>/msg55345.html

Since the config parser complains on startup and during log rotation, the useless ACL should be removed to avoid spamming the admin with mesages like these :

/etc/cron.daily/logrotate:
2012/04/20 06:25:14| WARNING: (B) '::/0' is a subnetwork of (A) '::/0'
2012/04/20 06:25:14| WARNING: because of this '::/0' is ignored to keep splay tree searching predictable
2012/04/20 06:25:14| WARNING: You should probably remove '::/0' from the ACL named 'all'

Thanks

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for your branch! From reading the linked mail it appears this is done in squid3, but currently squid-deb-proxy supports squid2 as well. Would removing those ACLs cause issues with squid2?

review: Needs Information
Revision history for this message
Simon Déziel (sdeziel) wrote :

> Thanks for your branch! From reading the linked mail it appears this is done
> in squid3, but currently squid-deb-proxy supports squid2 as well. Would
> removing those ACLs cause issues with squid2?

The ACL for 'all' appeared in squid3 only. AFAIK, Precise no longer ship squid2 though.

Revision history for this message
Michael Vogt (mvo) wrote :

On Mon, Apr 23, 2012 at 04:19:23PM -0000, Simon Déziel wrote:
> > Thanks for your branch! From reading the linked mail it appears this is done
> > in squid3, but currently squid-deb-proxy supports squid2 as well. Would
> > removing those ACLs cause issues with squid2?
>
> The ACL for 'all' appeared in squid3 only. AFAIK, Precise no longer ship squid2 though.

Aha, thanks! Yes, this makes sense then. Now that there is no squid2
anymore in ubuntu we can indeed remove it and more stuff as
well. Would you be interessted to work on the no longer squid/squid3
bits in the code? If not I will, but it will probably take ~2 weeks or
so as I'm traveling soon.

Thanks,
 Michael

Revision history for this message
Simon Déziel (sdeziel) wrote :

On 12-04-25 06:01 AM, Michael Vogt wrote:
> On Mon, Apr 23, 2012 at 04:19:23PM -0000, Simon Déziel wrote:
>> The ACL for 'all' appeared in squid3 only. AFAIK, Precise no longer ship squid2 though.
>
> Aha, thanks! Yes, this makes sense then. Now that there is no squid2
> anymore in ubuntu we can indeed remove it and more stuff as
> well. Would you be interessted to work on the no longer squid/squid3
> bits in the code?

I will try to have a look at this but that will probably not be before
next week.

Regards,
Simon

Revision history for this message
Nathan Bird (ecthellion) wrote :

Any chance of getting this merged?

The proprosed patch works for me and this would fix Bug #1005257 https://bugs.launchpad.net/ubuntu/+source/squid-deb-proxy/+bug/1005257

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: