Use ephemeral ports for idp-fixture#40333
Conversation
This change removes the use of hardcoded port values for the idp-fixture in favor of the mapped ephemeral ports. This should prevent failures due to port conflicts in CI.
|
Pinging @elastic/es-core-infra |
|
We should also change to in |
| @@ -17,11 +20,29 @@ testFixtures.useFixture ":x-pack:test:idp-fixture" | |||
|
|
|||
| String outputDir = "${project.buildDir}/generated-resources/${project.name}" | |||
| task copyIdpCertificate(type: Copy) { | |||
There was a problem hiding this comment.
Rename task to copyFiles or something generic like that ?
| project.sourceSets.test.output.dir(outputDir, builtBy: copyIdpCertificate) | ||
| integTestCluster.dependsOn copyIdpCertificate | ||
|
|
||
| task setupPorts { |
There was a problem hiding this comment.
Thanks for noticing and fixing this @jaymode !
The code to look up the ports is not necessary TestFixturesPlugin already sets this up,
postProcessFixture will have extension properties with the ports, so you just need something like this: postProcessFixture.doLast { println ext."test.fixtures.shibboleth-idp.tcp.443" } Note that any testing task also gets passed a system property by the same name.
jkakavas
left a comment
There was a problem hiding this comment.
This will unfortunately not work as is. The port is part of the SingleSignOnService binding URL and that URL is set as the Destination element value in the SAML Authentication Response. So, even if we set this to a random port outside the container, the Shibboleth IDP will still be configured with 4443 internally so from its perspective, the SingleSignOnService HTTP-Redirect binding Location will still be
https://localhost:4443/idp/profile/SAML2/Redirect/SSO
and it will fail on an authentication request with a Destination set to
https://localhost:34567/idp/profile/SAML2/Redirect/SSO
Setting the Destination in the authn request is only mandatory when the the request is signed ( otherwise optional ) but even if we were to change this default behavior , the tests would work only for a subset of use cases and then again something else might break.
Shibboleth IDP is not easy to run behind a reverse proxy :/ , I think we'd have to use something like httpd or nginx to do proper request rewriting so that both our saml realm and the Shibboleth IDP can be happy.
Alternatively, maybe @atorok knows if this is possible, we might be able to get around this if we can replace the random ephemeral port in the mounted volumes config files, so that shibboleth / tomcat use randomized ports internally also
The idp-metadata.xml gets re-written with the updated port, which is how it works. Maybe shibboleth handles this better now? Or is the test broken? The push from yesterday was missing some of my changes since I committed in a sub directory instead (DOH!). |
This is the idp-metadata.xml file that we copy to Shibboleth internally still knows of the old |
|
The reason that this works is due to the way that Shibboleth gets the URL for comparison. It uses |
This change removes the use of hardcoded port values for the idp-fixture in favor of the mapped ephemeral ports. This should prevent failures due to port conflicts in CI.
This change removes the use of hardcoded port values for the idp-fixture in favor of the mapped ephemeral ports. This should prevent failures due to port conflicts in CI.
This change removes the use of hardcoded port values for the idp-fixture in favor of the mapped ephemeral ports. This should prevent failures due to port conflicts in CI.
This change removes the use of hardcoded port values for the
idp-fixture in favor of the mapped ephemeral ports. This should prevent
failures due to port conflicts in CI.