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

webproxy: T3671: Fix paths from squid3 to squid #17

Merged
merged 1 commit into from
Dec 7, 2021

Conversation

sever-sever
Copy link

@sever-sever sever-sever commented Jul 12, 2021

In the original pkg squid a paths was changed from squid3 to squid.
This fixes changing all mentioned squid3 to squid.
https://phabricator.vyos.net/T3671

Webproxy in 1.2.8 can't start without this change

Tested configuration

set service webproxy listen-address 0.0.0.0 disable-transparent

Check processes

vyos@r12-lts:~$ 
vyos@r12-lts:~$ sudo netstat -tulpn | grep squid
tcp        0      0 0.0.0.0:3128            0.0.0.0:*               LISTEN      1702/(squid-1)  
udp        0      0 0.0.0.0:57481           0.0.0.0:*                           1702/(squid-1)  
udp6       0      0 :::42825                :::*                                1702/(squid-1)  
vyos@r12-lts:~$ 

@sever-sever sever-sever marked this pull request as draft July 12, 2021 17:43
Copy link
Contributor

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

My only immediate objection is that we should be using systemctl for restarting services, not legacy SysV init scripts.

There are also quite a few mentions of the long-gone "vyattaguard". But I think there are more, so we should probably make a separate task for it.

Also, the amount of duplicated code makes me wonder if we should consolidate clear and restart scripts into a single script.

scripts/vyatta-clear-squid Outdated Show resolved Hide resolved
scripts/vyatta-restart-squid Outdated Show resolved Hide resolved

if test -n "$PID"; then
/etc/init.d/squid3 stop
/etc/init.d/squid stop
if cli-shell-api existsActive service webproxy url-filtering \
Copy link
Contributor

Choose a reason for hiding this comment

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

VyattaGuard was a squidguard version with proprietary rules, as far as I remember. In any case it's no longer relevant.

Copy link
Author

Choose a reason for hiding this comment

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

I delete it.

scripts/vyatta-clear-squid Outdated Show resolved Hide resolved
@sever-sever
Copy link
Author

sever-sever commented Jul 13, 2021

The initial configuration works fine. Also after reboot, the service works.

set service webproxy listen-address 0.0.0.0 disable-transparent

I found some problem when we re-add configuration for webproxy, I can't commit it.

vyos@r12-lts# delete service webproxy 
[edit]
vyos@r12-lts# commit
[edit]
vyos@r12-lts#
vyos@r12-lts# set service webproxy listen-address 0.0.0.0 disable-transparent 
[edit]
vyos@r12-lts# commit

This commit hangs forever.

vyos@r12-lts:~$ ps ax | grep commit | egrep -v grep
 2453 pts/0    S+     0:00 /opt/vyatta/sbin/my_commit
vyos@r12-lts:~$ 

A similar bug was described in https://phabricator.vyos.net/T1770

Update. The reason for the commit hangs it trying to update squidguard db, but it is not configured.
https://github.com/vyos/vyatta-webproxy/blob/360343120ebcf50737f6914aac5c3efbad771ca9/scripts/vyatta-update-webproxy.pl#L1193
So it hangs on

vyos@r12-lts:~$ sudo -u proxy squidGuard -C all
We trust you have received the usual lecture from the local System
Administrator. It usually boils down to these three things:
    #1) Respect the privacy of others.
    #2) Think before you type.
    #3) With great power comes great responsibility.
[sudo] password for vyos:

In original pkg squid a paths was changed from squid3 to squid.
This fixes changing all mentioned squid3 to squid.
@c-po c-po requested a review from dmbaturin December 5, 2021 18:36
@dmbaturin dmbaturin merged commit 6fec71f into vyos-legacy:crux Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants