Re: Proposal: on insert client ip

[Thomas Jackson <ja...@gmail.com>]

+1 on config cleanup
On Sep 11, 2013 8:01 PM, "James Peach" <jp...@apache.org> wrote:

> On Sep 10, 2013, at 10:18 PM, Yongming Zhao <mi...@gmail.com> wrote:
>
> >
> > we have a way to insert and remove the Client-ip header, which is to
> record the UA ip address:
> >
> > proxy.config.http.anonymize_remove_client_ip
> >
> > INT
> >
> > 0
> >
> > When enabled (1), Traffic Server removes Client-IP headers for more
> privacy.
> >
> > proxy.config.http.anonymize_insert_client_ip
> >
> > INT
> >
> > 1
> >
> > When enabled (1), Traffic Server inserts Client-IP headers to retain the
> client IP address.
> >
> >
> > that is really strange in directive name, as
> >
> > we have anonymize for remove sensitive headers:
> >
> > zymMBPr:mgmt zym$ grep anonymize RecordsConfig.cc
> >  {RECT_CONFIG, "proxy.config.http.anonymize_remove_from", RECD_INT, "0",
> RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_remove_referer", RECD_INT,
> "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_remove_user_agent",
> RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_remove_cookie", RECD_INT,
> "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_remove_client_ip", RECD_INT,
> "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_insert_client_ip", RECD_INT,
> "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_other_header_list",
> RECD_STRING, NULL, RECU_DYNAMIC, RR_NULL, RECC_STR, ".*", RECA_NULL}
> >
> > we do have another way to insert some useful headers:
> >
> > zymMBPr:mgmt zym$ grep insert RecordsConfig.cc
> >  {RECT_CONFIG, "proxy.config.http.insert_request_via_str", RECD_INT,
> "1", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.insert_response_via_str", RECD_INT,
> "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_insert_client_ip", RECD_INT,
> "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.insert_squid_x_forwarded_for",
> RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.insert_age_in_response", RECD_INT,
> "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >
> >
> > but we have now both of the name:
> >
> >  {RECT_CONFIG, "proxy.config.http.anonymize_remove_client_ip", RECD_INT,
> "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >  {RECT_CONFIG, "proxy.config.http.anonymize_insert_client_ip", RECD_INT,
> "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
> >
> >
> > we should name proxy.config.http.anonymize_insert_client_ip as
> 'proxy.config.http.insert_request_client_ip' or just
> 'proxy.config.http.insert_client_ip'
> >
> > and the current implement of the insert client ip works only if the
> client request do not have a 'Client-ip' header, but sometimes we need to
> replace it even if someone send us a fake 'Client-ip':
> >
> > proxy.config.http.insert_client_ip
> >
> > INT
> >
> > 1
> >
> >
> > When disabled(0), do nothing.
> > When enabled (1), Traffic Server inserts Client-IP headers to retain the
> client IP address, if there is no such headers.
> > When forced (2), Traffic Server inserts Client-IP, or replace the origin
> Client-IP header if it is already there.
> >
> >
> > any comments?
>
> I like this. I think that it's worth being consistent and predictable. The
> ability to forcibly insert the ClientIP is also useful. I'd like to see
> this addition in the 4.x series, with deprecation warnings for the old
> names. Once 5.x comes around, we can remove the old, inconsistent names
>
> J