[squid-dev] [RFC\PREVIEW] ICAP service nodown option addition.

[Eliezer Croitoru]

Thanks Alex,

Inline comments..

On 08/01/2016 17:27, Alex Rousskov wrote:
> On 01/07/2016 07:03 PM, Eliezer Croitoru wrote:
>
>> I added a configurable option to the ICAP services named "nodown" but
>> maybe another name would be better fit.
>
> Please do not use negative option names. If this feature is committed
> despite my objections, please consider using "always-up" or another
> positive name.
Both nodown and always-up do not describe the exact meaning of the flag.
Do you(..and others) think that a longer name would be better? something 
like "options-only-suspened" or "suspend-only-by-options-fail" ?


>> The idea of the patch is to allow the admin to count on the periodic
>> OPTIONS request only to identify the current state of the ICAP service.
>
> If an ICAP OPTIONS request fails, will the service be marked as down? If
> yes, the option name is misleading. If not, your description above is
> misleading. If this feature is committed, please adjust the
> documentation to clearly define whether Squid may mark a nodown=1
> service as down for any reason.

I suspected that my words might not be descriptive enough and I will try 
to clear it out in the next comments.

>
>> I was thinking about using the icap_service_failure_limit with a very
>> high limit but it is an ICAP global configuration and not a service
>> specific configuration.
>
> IMO, you should add a service-specific failure-limit option instead of
> adding a brand new option with an overlapping but very narrow scope.
> Squid already implements the failure-limit logic on a per-service basis.
> You just need to add a per-service option (that will default to the
> global one).

I was thinking about it but this feature eliminates every suspension 
between each OPTIONS fetch\check.

I know it doesn't fit all setups but I want to eliminate any other test 
then the OPTIONS fetch and the reason is that only using a very high 
failure limit will answer to this specific attack I have seen. A simple 
JS just sent about 1k requests pretty fast and with all of them failing 
pretty fast the ICAP service is just being suspended so just eliminating 
the issues solved my problem.

(I cannot do a thing about a webui programmer that doesn't like to do 
his job properly??)

>> +       [...] It gives the
>> +       service admin to implement his own heart-beat script which
>> +       will work as a replacment to the default internal requests
>> +       success\failure probes while the ICAP service state is UP.
>> +       An external heart-beat script can run under external_acl
>> +       and can be checked in http_access and couple other acls
>> +       vectoring points.
>
> IMO, this text should not be added to the new option because it talks
> about controlling access to an ICAP service rather than about [not]
> changing the service state which the new option controls. Referring to
> adaptation_access from the new option documentation and adding a similar
> hint to the adaptation_access documentation instead may be a good idea.

Not really but I do understand what you are writing about.
The intention is very specific and not related to the adaptation_access, 
the idea is that the service is essential and the proxy cannot allow any 
traffic without this service due to some health facilities 
confidentiality policy. So for the case that the service is indeed down 
the admin needs to deny access to the whole service since it's useless 
without the adaptation service. The external_acl is per squid port and 
will deny access to the whole service based on the real status of the 
ICAP service.
If I will use adaptation_access it will lead to a situation which the 
clients traffic will not pass inspection which is worst then having ICAP 
failures per specific wrongly formed requests.

However I did not test what is the situation with adaptation services 
set and if their "chained" use is based only on full suspension or it 
will also pass the request to the next host per request failure.

I will conduct couple tests this week to make sure how things works with 
sets.

Do you think a wiki article will be good enough to explain the use cases 
of this option instead of the documentation?
I was thinking about writing one.

> HTH,
>
> Alex.

Again Thanks!
Eliezer