Bug 23068 - Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy
Summary: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR feature...
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords: dependency
: 21477 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-06-06 19:12 UTC by Kyle M Hall
Modified: 2019-11-20 14:01 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 23068: [DO NOT PUSH] Add debugging to C4::Context (728 bytes, patch)
2019-06-06 19:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068 - Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (2.19 KB, patch)
2019-06-06 19:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file (2.02 KB, patch)
2019-06-13 17:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (2.24 KB, patch)
2019-07-02 06:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file (2.07 KB, patch)
2019-07-02 06:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file (2.14 KB, patch)
2019-07-10 15:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (11.32 KB, patch)
2019-08-23 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: [DO NOT PUSH] Debugging code for test plan (646 bytes, patch)
2019-08-23 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (11.16 KB, patch)
2019-08-23 18:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: [DO NOT PUSH] Debugging code for test plan (646 bytes, patch)
2019-08-23 18:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (8.15 KB, patch)
2019-10-28 13:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: [DO NOT PUSH] Debugging code for test plan (695 bytes, patch)
2019-10-28 13:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (8.20 KB, patch)
2019-10-29 14:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (8.76 KB, patch)
2019-10-29 16:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy (8.81 KB, patch)
2019-10-31 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2019-06-06 19:12:17 UTC
Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. We can use Plack::Middleware::RealIP to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable so no Koha code need be modified. 

A future enhancement could be to take advantage of the module's trusted_proxy setting to allow Koha to use these feature even after going through *multiple* proxies. This would require being able to read configuration from the plack.psgi file and is beyond the scope of this enhancement.
Comment 1 Kyle M Hall 2019-06-06 19:35:52 UTC
Actually, we can use an environment variable so I'm increasing the scope to include setting the list of proxies!
Comment 2 Kyle M Hall 2019-06-06 19:42:31 UTC
Created attachment 90395 [details] [review]
Bug 23068: [DO NOT PUSH] Add debugging to C4::Context
Comment 3 Kyle M Hall 2019-06-06 19:42:47 UTC
Created attachment 90396 [details] [review]
Bug 23068 - Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. We can use Plack::Middleware::RealIP to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable so no Koha code need be modified.

We can take advantage of the module's trusted_proxy setting to allow Koha to use these feature even after going through *multiple* proxies.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs is what you put in the environment variable:
    KOHA_TRUSTED_PROXIES="172.22.0.1 1.1.1.1" koha-plack --restart kohadev
9) Repeat step 6
7) You should now see 32.32.32.32 as the remote address in your logs!
Comment 4 Kyle M Hall 2019-06-13 17:21:52 UTC
*** Bug 21477 has been marked as a duplicate of this bug. ***
Comment 5 Kyle M Hall 2019-06-13 17:46:40 UTC
Created attachment 90582 [details] [review]
Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file
Comment 6 Martin Renvoize 2019-07-02 06:13:56 UTC
QA Questions
1) Does this work with the Mojo derived API's too?
2) What if Koha is running under CGI and not plack - yes.. there are still some out there :P
Comment 7 Martin Renvoize 2019-07-02 06:16:12 UTC
Created attachment 91173 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. We can use Plack::Middleware::RealIP to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable so no Koha code need be modified.

We can take advantage of the module's trusted_proxy setting to allow Koha to use these feature even after going through *multiple* proxies.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs is what you put in the environment variable:
    KOHA_TRUSTED_PROXIES="172.22.0.1 1.1.1.1" koha-plack --restart kohadev
9) Repeat step 6
7) You should now see 32.32.32.32 as the remote address in your logs!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2019-07-02 06:16:15 UTC
Created attachment 91174 [details] [review]
Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2019-07-02 06:17:55 UTC
Fixed up the commit line to get the QA scripts happy and signing off.. I'll leave my two QA questions for a QA person to follow up but I didn't see any point in holding up SO.
Comment 10 Kyle M Hall 2019-07-10 15:24:24 UTC
Created attachment 91447 [details] [review]
Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Kyle M Hall 2019-07-10 18:05:39 UTC
Found a problem. The middleware is working via plack so we can the real ip address when we are on a script that uses plack. When we visit a script that doesn't use plack, this code doesn't get executed and so Koha sees the proxy ip instead.
And Koha does what it always does when a session changes ip addresses, it forces a re-login
Comment 12 Kyle M Hall 2019-08-23 18:23:15 UTC
Created attachment 92461 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work.
This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf:
    <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies>
9) Restart all the things!
10) Repeat step 6
11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs!
12) Disable plack so you are running in cgi mode, repeat step 6 again
13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!
Comment 13 Kyle M Hall 2019-08-23 18:23:27 UTC
Created attachment 92462 [details] [review]
Bug 23068: [DO NOT PUSH] Debugging code for test plan
Comment 14 Kyle M Hall 2019-08-23 18:24:30 UTC
Created attachment 92463 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work.
This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf:
    <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies>
9) Restart all the things!
10) Repeat step 6
11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs!
12) Disable plack so you are running in cgi mode, repeat step 6 again
13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!
Comment 15 Kyle M Hall 2019-08-23 18:24:50 UTC
Created attachment 92464 [details] [review]
Bug 23068: [DO NOT PUSH] Debugging code for test plan
Comment 16 Kyle M Hall 2019-10-28 13:37:35 UTC
Created attachment 94807 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work.
This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf:
    <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies>
9) Restart all the things!
10) Repeat step 6
11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs!
12) Disable plack so you are running in cgi mode, repeat step 6 again
13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Comment 17 Kyle M Hall 2019-10-28 13:37:48 UTC
Created attachment 94808 [details] [review]
Bug 23068: [DO NOT PUSH] Debugging code for test plan

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Comment 18 David Cook 2019-10-29 08:07:28 UTC
(In reply to Martin Renvoize from comment #6)
> QA Questions
> 1) Does this work with the Mojo derived API's too?
> 2) What if Koha is running under CGI and not plack - yes.. there are still
> some out there :P

I have these questions too, although I've been using CGI behind (sometimes multiple) reverse proxies for a long time, so I've already hacked around where necessary to support X-Forwarded-For. It's not really shareable code though as it's very specific.
Comment 19 David Cook 2019-10-29 08:14:56 UTC
Comment on attachment 94807 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Review of attachment 94807 [details] [review]:
-----------------------------------------------------------------

::: C4/Context.pm
@@ +1107,5 @@
> +
> +=cut
> +
> +sub set_remote_address {
> +    if ( C4::Context->config('koha_trusted_proxies') ) {

Wouldn't this still be called under Plack?

Couldn't we just look to see if it's Plack via environmental variables and only do this for CGI?
Comment 20 Kyle M Hall 2019-10-29 12:52:23 UTC
(In reply to David Cook from comment #19)
> Comment on attachment 94807 [details] [review] [review]
> Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so
> REMOTE_ADDR features work behind a proxy
> 
> Review of attachment 94807 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Context.pm
> @@ +1107,5 @@
> > +
> > +=cut
> > +
> > +sub set_remote_address {
> > +    if ( C4::Context->config('koha_trusted_proxies') ) {
> 
> Wouldn't this still be called under Plack?
> 
> Couldn't we just look to see if it's Plack via environmental variables and
> only do this for CGI?

That's a good thought. Is there an environment variable that is set for scripts running under plack?
Comment 21 Kyle M Hall 2019-10-29 13:22:06 UTC
(In reply to David Cook from comment #19)
> Comment on attachment 94807 [details] [review] [review]
> Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so
> REMOTE_ADDR features work behind a proxy
> 
> Review of attachment 94807 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Context.pm
> @@ +1107,5 @@
> > +
> > +=cut
> > +
> > +sub set_remote_address {
> > +    if ( C4::Context->config('koha_trusted_proxies') ) {
> 
> Wouldn't this still be called under Plack?
> 
> Couldn't we just look to see if it's Plack via environmental variables and
> only do this for CGI?

Actually, it only gets called under plack at startup, since C4::Auth is loaded at that time. So, yes, we could do that, but it doesn't actually hurt performance by any meaningful metric!
Comment 22 Kyle M Hall 2019-10-29 14:36:41 UTC
(In reply to Martin Renvoize from comment #6)
> QA Questions
> 1) Does this work with the Mojo derived API's too?

Yes it does!

> 2) What if Koha is running under CGI and not plack - yes.. there are still
> some out there :P

It does indeed! I had to engineer it specifically to work under CGI as well as we still have a handful of scripts that we still run under CGI instead of plack.
Comment 23 Martin Renvoize 2019-10-29 14:58:36 UTC
Created attachment 94838 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work.
This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf:
    <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies>
9) Restart all the things!
10) Repeat step 6
11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs!
12) Disable plack so you are running in cgi mode, repeat step 6 again
13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2019-10-29 15:00:23 UTC
Sorry dude.. lots of QA script failures.. and I think this also introduces a new dependancy that will need to be listed: 

 FAIL   C4/Auth.pm
   FAIL   valid
                Can't locate object method "set_remote_address" via package "C4::Context"

 OK     C4/Context.pm
 FAIL   Koha/Middleware/RealIP.pm
   FAIL   critic
                Variable declared in conditional statement at line 53, column 9. Declare variables outside of the condition.
   FAIL   pod
                *** WARNING:
                =head3 without preceding higher level

                 in file Koha/Middleware/RealIP.pm

                *** WARNING:
                =head3 without preceding higher level

                 in file Koha/Middleware/RealIP.pm

                *** WARNING:
                =head3 without preceding higher level

                 in file Koha/Middleware/RealIP.pm

                *** WARNING:
                =head3 without preceding higher level

                 in file Koha/Middleware/RealIP.pm

   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   FAIL   valid
                Can't locate Net/Netmask.pm in @INC (you may need to install the Net::Netmask module) (@INC contains: /home/martin/kohaclone /home/martin/kohaclone/C4/SIP /home/martin/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base)

 OK     etc/koha-conf.xml

Processing additional checks OK!
Comment 25 Kyle M Hall 2019-10-29 16:26:06 UTC
Created attachment 94839 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work.
This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf:
    <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies>
9) Restart all the things!
10) Repeat step 6
11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs!
12) Disable plack so you are running in cgi mode, repeat step 6 again
13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Comment 26 David Cook 2019-10-29 23:44:45 UTC
(In reply to Kyle M Hall from comment #21)
> Actually, it only gets called under plack at startup, since C4::Auth is
> loaded at that time. So, yes, we could do that, but it doesn't actually hurt
> performance by any meaningful metric!

I was wondering about that a bit. That's a good point hehe.

In any case, there should be a number of "psgi.*" environmental variables: https://metacpan.org/pod/PSGI#The-Environment.
Comment 27 Martin Renvoize 2019-10-31 16:07:02 UTC
Created attachment 94919 [details] [review]
Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy

Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work.
This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes.

TEST PLAN:
1) Apply this patch set
2) Install Plack::Middleware::RealIP via cpanm or your favorite utility
3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment )
4) Restart plack
5) Tail the plack error log for your instance
6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080
7) Note the logs output this address if you are unproxied
8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf:
    <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies>
9) Restart all the things!
10) Repeat step 6
11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs!
12) Disable plack so you are running in cgi mode, repeat step 6 again
13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2019-10-31 16:09:21 UTC
Nice work Kyle, all QA issues fixed.

Works well, dependancies is available in D8 by default so no need to worry about packaging.

Passing QA :)
Comment 29 Martin Renvoize 2019-10-31 16:10:43 UTC
Nice work!

Pushed to master for 19.11.00
Comment 30 David Cook 2019-11-04 23:58:46 UTC
Yay! Nice one, Kyle!
Comment 31 Ed Veal 2019-11-13 23:47:55 UTC
Great job Kyle.
Comment 32 Victor Grousset/tuxayo 2019-11-18 18:37:28 UTC
KohaDevBoxes (even new ones) and other development environments based on Debian or Ubuntu need to install the package "libnet-netmask-perl"
Comment 33 David Cook 2019-11-19 07:51:20 UTC
(In reply to Victor Grousset/tuxayo from comment #32)
> KohaDevBoxes (even new ones) and other development environments based on
> Debian or Ubuntu need to install the package "libnet-netmask-perl"

I've often wondered when the debian control files get updated with the dependencies...
Comment 34 Martin Renvoize 2019-11-19 08:12:34 UTC
(In reply to Victor Grousset/tuxayo from comment #32)
> KohaDevBoxes (even new ones) and other development environments based on
> Debian or Ubuntu need to install the package "libnet-netmask-perl"

Correct, it has been escalated to a 'required' dependancy. This is because for plack we need to load it before we load C4::Context and as such we can't use it optionally as initially thought.
Comment 35 Martin Renvoize 2019-11-19 08:17:51 UTC
(In reply to David Cook from comment #33)
> (In reply to Victor Grousset/tuxayo from comment #32)
> > KohaDevBoxes (even new ones) and other development environments based on
> > Debian or Ubuntu need to install the package "libnet-netmask-perl"
> 
> I've often wondered when the debian control files get updated with the
> dependencies...

The control files were updated almost immediately.. The process is to add a dependency to C4/Installer/PerlDependancies.pm and then push that up to master.

The evening the nightly build bot will spot the new dependency and add it to the control file and send the patch to bug 17084 which the release manager should be watching.

The next day, the release manager will do a quick QA on the resultant file and push the patch as necessary.

So, in this case this took a sum total of a day to get the new dependency into the debian control files (there was indeed a day of Jenkins getting upset about the missing dependency too as the packages were failing to build).

I can't entirely remember how the biblibre sandboxes work (koha-testing-docker, which koha-sandboxes-docker depends upon for it's base koha image, has already been updated).  Do you now re-run apt install koha-perldeps or anything regularly?  That should have caught the debian control change.. also, I presume if you are running such a command then you're also tracking the nightly repository to pickup the very latest changes.

Hope that helps, 

Martin
Comment 36 David Cook 2019-11-20 00:28:23 UTC
(In reply to Martin Renvoize from comment #34)
> Correct, it has been escalated to a 'required' dependancy. This is because
> for plack we need to load it before we load C4::Context and as such we can't
> use it optionally as initially thought.

I was thinking we could actually make good use of Net::Netmask in other parts of Koha as well. I'd love to use it for processing ILS-DI:AuthorizedIPs (could also use it for OpacSuppressionByIPRange, RestrictedPageLocalIPs, and surely other places that don't come to mind yet...).

So it being a required dependency could be a motivator for updating those too. *puts it on his infinite todo list*.
Comment 37 Victor Grousset/tuxayo 2019-11-20 14:01:54 UTC
> Do you now re-run apt install koha-perldeps or anything regularly?  That should have caught the debian control change.. also, I presume if you are running such a command then you're also tracking the nightly repository to pickup the very latest changes.

It was on a KohaDevBox, I guess the nightly repo wasn't up to date yet.