Closed rsvoboda closed 2 years ago
Hi @rsvoboda, #127 solves the issue for all the tests and we've been discussing it both there in comments and in chat with @mnovak1. In the end I pushed a final commit to apply the pattern everywhere - even if some considerations still stand on the fact we're required NOT to close the client this way, as it would close the wrapped ModelControllerClient
instance, which is provided - and used - by Arquillian.
I'd like for you to have a look and let us know whether the status is ok now.
Regarding ListenerSecurityConfigurationTest
I agree on removing those two references to localhost
for certificate creation but... do we need this? How would you specify different configuration for it? System properties? Let me know your thoughts.
My idea was that enableHTTPSListener
is called from test so URL of server is known at that time so localhost can be changed to real address.
But agree we probably don't need it atm.
Main aim of this issue is to discuss this topic and come to a conclusion. "Keeping as is" is possible conclusion :/
Hi @mnovak1, currently - and regarding the only MP OpenAPI test module - the localhost
literal is occurring only in ListenerSecurityConfigurationTest
$ find . -name '*.java' -type f -print0 | xargs -0 grep --color -n -e 'localhost'
./microprofile-open-api/src/test/java/org/jboss/eap/qe/microprofile/openapi/security/ListenerSecurityConfigurationTest.java:137: "alias=localhost,algorithm=RSA,key-size=1024,validity=365," +
./microprofile-open-api/src/test/java/org/jboss/eap/qe/microprofile/openapi/security/ListenerSecurityConfigurationTest.java:138: "credential-reference={clear-text=secret},distinguished-name=\"CN=localhost\")");
After reviewing several examples of HTTPS listener configuration I find that is common practice to use localhost, e.g. https://docs.jboss.org/author/display/WFLY/WildFly+Elytron+Security#WildFlyElytronSecurity-SSL%2FTLS, https://github.com/wildfly/wildfly/blob/master/docs/src/main/asciidoc/_elytron/Using_the_Elytron_Subsystem.adoc#using-elytron-subsystem-commands
I am wondering whther we could just leave this as it is now and close the issue. WDYT?
That being said, I've found the constant has been used somewhere else too:
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/MultipleDeploymentsNamefellowMetricsTest.java:35: return "http://localhost:8080/dep1/ping-one";
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/MultipleDeploymentsNamefellowMetricsTest.java:40: return "http://localhost:8080/dep2/ping-two";
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/SubdeploymentNamefellowMetricsTest.java:58: return "http://localhost:8080/dep1/ping-one";
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/SubdeploymentNamefellowMetricsTest.java:63: return "http://localhost:8080/dep1/ping-two";
./tooling-docker/src/main/java/org/jboss/eap/qe/ts/common/docker/DockerContainers.java:36: .withCmdArg("--reporter.grpc.host-port=localhost:14250")
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/keycloak/KeycloakIntegrationHighLevelScenarioTest.java:38: private static final String KEYCLOAK_INSTANCE_HOSTNAME = "localhost";
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/keycloak/KeycloakIntegrationHighLevelScenarioTest.java:77: //visit http://localhost:8179/auth/realms/foobar/.well-known/openid-configuration with running keycloak for values
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/keycloak/KeycloakIntegrationHighLevelScenarioTest.java:78: final String mpProperties = "mp.jwt.verify.publickey.location=http://localhost:8179/auth/realms/%1$s/protocol/openid-connect/certs%n"
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/keycloak/KeycloakIntegrationHighLevelScenarioTest.java:80: "mp.jwt.verify.issuer=http://localhost:8179/auth/realms/%1$s";
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/keycloak/KeycloakConfigurator.java:79: .add("redirectUris", Json.createArrayBuilder().add("http://localhost:8085")).build().toString())
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/security/publickeylocation/PublicKeyLocationPropertyTestCase.java:61: .addHttpListener(8123, "localhost")
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/security/publickeylocation/PublicKeyLocationPropertyTestCase.java:77: return initDeployment("http://localhost:8123/",
./microprofile-jwt/src/test/java/org/jboss/eap/qe/microprofile/jwt/security/publickeylocation/PublicKeyLocationPropertyTestCase.java:83: return initDeployment("http://localhost:1111/",
What should we do? I suggest to point this issue to those modules currently using it. WDYT?
I think that in case of test cases from metrics module:
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/MultipleDeploymentsNamefellowMetricsTest.java:35: return "http://localhost:8080/dep1/ping-one";
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/MultipleDeploymentsNamefellowMetricsTest.java:40: return "http://localhost:8080/dep2/ping-two";
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/SubdeploymentNamefellowMetricsTest.java:58: return "http://localhost:8080/dep1/ping-one";
./microprofile-metrics/src/test/java/org/jboss/eap/qe/microprofile/metrics/SubdeploymentNamefellowMetricsTest.java:63: return "http://localhost:8080/dep1/ping-two";
there might be evaluated whether:
@ArquillianResource
@OperateOnDeployment(...)
URL baseApplicationUrl;
might be used.
In case of docker module there is not much what can be done I would let it be.
In case of jwt module I think there is not much what can be done but let's ask @honza-kasik :-)
It is clear, that I can easily obtain hostname (URL) for Arquillian resources. But what about when I am using Docker container or a HTTP server. Is there a common approach how to get local hostname?
Hi @mnovak1, I agree with your opinion about metrics and docker related modules. @honza-kasik I am currently not aware of a common approach to be used to get the host name for the use cases you mentioned. For the one related to MP OpenAPI module I am wondering what would be the added value of replacing the literal with the proper host name.
@mnovak1 regarding OpenAPI, actually it could be useful to replace ‘localhost’ with the address of the actual Arquillian container on which the certificate generation will happen. WDYT?
Closed as resolved via #202.
There is localhost dependency in openapi tests
Is there a way to eliminate this and depend on resolved deployment address ?
In MicroprofileConfigIntegrationTests it's probably impossible because of the need to define it before deploying, but let's see if anybody has an idea.
ListenerSecurityConfigurationTest could be adjusted to reflect non-localhost address if specified. Maybe we just need to discuss if we need configurable
CN
or not.