Arvados

The upgrade causes these failures:

----------------------------------------------------------------------
FAIL: login_oidc_test.go:764: OIDCLoginSuite.TestValidateLoginRedirectTarget

trial {permit:true trustPrivate:false url:https://127.0.0.1:43397/}
trial {permit:true trustPrivate:false url:https://127.0.0.1:57277/}
trial {permit:true trustPrivate:false url:https://app.example.com/}
trial {permit:true trustPrivate:false url:https://app.example.com:443/foo?bar=baz}
trial {permit:false trustPrivate:false url:https://bad.example/}
trial {permit:false trustPrivate:true url:https://bad.example/}
trial {permit:false trustPrivate:true url:https://1.2.3.4/}
trial {permit:false trustPrivate:true url:https://1.2.3.4/}
trial {permit:false trustPrivate:true url:https://[ab::cd]:1234/}
trial {permit:false trustPrivate:false url:https://localhost/}
trial {permit:true trustPrivate:true url:https://localhost/}
trial {permit:false trustPrivate:false url:https://[10.9.8.7]:80/foo}
trial {permit:true trustPrivate:true url:https://[10.9.8.7]:80/foo}
login_oidc_test.go:809:
    c.Check(err == nil, check.Equals, trial.permit)
... obtained bool = false
... expected bool = true

trial {permit:false trustPrivate:false url:https://[::1]:80/foo}
trial {permit:true trustPrivate:true url:https://[::1]:80/foo}
trial {permit:true trustPrivate:true url:http://192.168.1.1/}
trial {permit:true trustPrivate:true url:http://172.17.2.0/}
trial {permit:false trustPrivate:true url:https://10.1.1.1:blorp/foo}
trial {permit:false trustPrivate:true url:https://app.example.com:blorp/foo}
trial {permit:false trustPrivate:true url:https://]:443}
trial {permit:false trustPrivate:true url:https://}
trial {permit:false trustPrivate:true url:https:}
trial {permit:false trustPrivate:true url:}
trial {permit:false trustPrivate:true url:http://app.example.com/}
trial {permit:false trustPrivate:true url:http://app.example.com:443/}
trial {permit:false trustPrivate:true url:https://app.example.com:80/}
trial {permit:false trustPrivate:true url:https://app.example.com:4433/}
trial {permit:false trustPrivate:true url:https://u:p@app.example.com:443/foo?bar=baz}

----------------------------------------------------------------------
FAIL: login_test.go:18: loginSuite.TestValidateLoginRedirectTarget

trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted: target:https://wb2.example/}
trial {pass:true wb1:https://wb1.example:443/ wb2:https://wb2.example:443/ trusted: target:https://wb2.example/}
trial {pass:true wb1:https://wb1.example:443/ wb2:https://wb2.example:443/ trusted: target:https://wb2.example}
trial {pass:true wb1:https://wb1.example:443 wb2:https://wb2.example:443 trusted: target:https://wb2.example/}
trial {pass:true wb1:http://wb1.example:80/ wb2:http://wb2.example:80/ trusted: target:http://wb2.example/}
trial {pass:false wb1:https://wb1.example:80/ wb2:https://wb2.example:80/ trusted: target:https://wb2.example/}
trial {pass:false wb1:https://wb1.example:1234/ wb2:https://wb2.example:1234/ trusted: target:https://wb2.example/}
trial {pass:false wb1:https://wb1.example/ wb2:https://wb2.example/ trusted: target:https://bad.wb2.example/}
trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://good.wb2.example/ target:https://good.wb2.example}
trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://good.wb2.example:443/ target:https://good.wb2.example}
trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://good.wb2.example:443 target:https://good.wb2.example/}
trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://*.wildcard.example target:https://ok.wildcard.example/}
trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://*.wildcard.example target:https://ok.ok.wildcard.example/}
trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://*.wildcard.example target:https://[ok.ok.wildcard.example]:443/}
login_test.go:73:
    c.Check(err == nil, check.Equals, trial.pass)
... obtained bool = false
... expected bool = true

trial {pass:true wb1:https://wb1.example/ wb2:https://wb2.example/ trusted:https://[*.wildcard.example]:443 target:https://ok.ok.wildcard.example/}
login_test.go:70:
    c.Assert(err, check.IsNil)
... value *url.Error = &url.Error{Op:"parse", URL:"https://[*.wildcard.example]:443", Err:(*fmt.wrapError)(0xc002897460)} ("parse \"https://[*.wildcard.example]:443\": invalid host: ParseAddr(\"*.wildcard.example\"): unexpected character (at \"*.wildcard.example\")")

Sent SIGTERM to 34927 (/tmp/workspace/run-tests-remainder/tmp/keep0.pid)
Sent SIGTERM to 34943 (/tmp/workspace/run-tests-remainder/tmp/keep1.pid)
keep0.pid is 34989
keep1.pid is 35005
OOPS: 102 passed, 2 FAILED
--- FAIL: Test (188.98s)
FAIL
coverage: 70.2% of statements
FAIL    git.arvados.org/arvados.git/lib/controller/localdb    188.997s
FAIL
======= lib/controller/localdb tests -- FAILED

At first glance it looks like there might be some change to the way bracketed hosts are treated.

Lisa Knox wrote in #note-6:

They've been like this since #19240 and I only noticed because you changed the line right below it. I can't imagine any reason they should be like this, but maybe there is a reason I just don't know about?

Nope, I'm with you. Looking at the original diff I'd guess it's a copy+paste error. Other test cases test both true and false values for configuration, so I have differentiated these cases that way too.

On re-review I noticed my original commit had a confusing typo in the commit message and moved a test case that didn't need to be moved. So:

• 025ff5e4de866c749bd60b8e673f7b8c3ed2e581 is functionally identical to the original code changes you reviewed, with two small nonfunctional changes for better clarity.
• 42ea77410866f1a061c8e1eac2f5bdb7842510d6 is the completely unchanged upgrade note.
• fbcdae170baa3dc812e2635fb559644274a1147c fixes the duplicate test case based on your comment.

And with that this is merged. Thanks.