BBlack has submitted this change and it was merged.

Change subject: V4 XFF Fixup 2/3
......................................................................


V4 XFF Fixup 2/3

set_xff: Move the req.restarts==0 part out to the caller

Change-Id: Ief13871d7fee35de28da8b38f50567f9749041ef
---
M modules/varnish/templates/vcl/wikimedia-backend.vcl.erb
M modules/varnish/templates/vcl/wikimedia-common.inc.vcl.erb
M modules/varnish/templates/vcl/wikimedia-frontend.vcl.erb
3 files changed, 13 insertions(+), 12 deletions(-)

Approvals:
  BBlack: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/modules/varnish/templates/vcl/wikimedia-backend.vcl.erb 
b/modules/varnish/templates/vcl/wikimedia-backend.vcl.erb
index c3da042..8d1690c 100644
--- a/modules/varnish/templates/vcl/wikimedia-backend.vcl.erb
+++ b/modules/varnish/templates/vcl/wikimedia-backend.vcl.erb
@@ -19,7 +19,9 @@
        }
 
 <% if not @varnish_version4 -%>
-       call wm_common_recv_set_xff;
+       if (req.restarts == 0) {
+               call wm_common_recv_set_xff;
+       }
 <% end -%>
        call wm_common_recv_early;
 
diff --git a/modules/varnish/templates/vcl/wikimedia-common.inc.vcl.erb 
b/modules/varnish/templates/vcl/wikimedia-common.inc.vcl.erb
index 0ddf287..54c76fd 100644
--- a/modules/varnish/templates/vcl/wikimedia-common.inc.vcl.erb
+++ b/modules/varnish/templates/vcl/wikimedia-common.inc.vcl.erb
@@ -243,15 +243,13 @@
 
 <% if not @varnish_version4 -%>
 sub wm_common_recv_set_xff {
-       // XFF-appending is non-idempotent for restart purposes..
-       if (req.restarts == 0) {
-               // All layers need to update XFF with client.ip hop-by-hop so 
that it
-               // looks right to layers beneath, including the app layer
-               if (req.http.X-Forwarded-For) {
-                       set req.http.X-Forwarded-For = req.http.X-Forwarded-For 
+ ", " + client.ip;
-               } else {
-                       set req.http.X-Forwarded-For = client.ip;
-               }
+       // XFF-appending is non-idempotent for restart purposes, should be in 
req.restarts == 0 block
+       // All layers need to update XFF with client.ip hop-by-hop so that it
+       // looks right to layers beneath, including the app layer
+       if (req.http.X-Forwarded-For) {
+               set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " 
+ client.ip;
+       } else {
+               set req.http.X-Forwarded-For = client.ip;
        }
 }
 <% end -%>
diff --git a/modules/varnish/templates/vcl/wikimedia-frontend.vcl.erb 
b/modules/varnish/templates/vcl/wikimedia-frontend.vcl.erb
index 4910265..33562eb 100644
--- a/modules/varnish/templates/vcl/wikimedia-frontend.vcl.erb
+++ b/modules/varnish/templates/vcl/wikimedia-frontend.vcl.erb
@@ -239,11 +239,12 @@
 
                // IP processing is req->req mangling that shouldn't be re-done 
on restart
                call recv_fe_ip_processing;
-       }
 
 <% if not @varnish_version4 -%>
-       call wm_common_recv_set_xff;
+               call wm_common_recv_set_xff;
 <% end -%>
+       }
+
        call wm_common_recv_early;
 
 <% if @varnish_version4 -%>

-- 
To view, visit https://gerrit.wikimedia.org/r/289257
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ief13871d7fee35de28da8b38f50567f9749041ef
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: BBlack <bbl...@wikimedia.org>
Gerrit-Reviewer: BBlack <bbl...@wikimedia.org>
Gerrit-Reviewer: Ema <e...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to