Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixed up Test configuration when using @ResourceArg #22025

Closed
GregJohnStewart opened this issue Dec 8, 2021 · 17 comments
Closed

Mixed up Test configuration when using @ResourceArg #22025

GregJohnStewart opened this issue Dec 8, 2021 · 17 comments
Labels
area/testing kind/bug Something isn't working kind/question Further information is requested

Comments

@GregJohnStewart
Copy link
Contributor

GregJohnStewart commented Dec 8, 2021

Describe the bug

TL:DR; When running tests with different @ResourceArgs to configure a TestLifecycleManager, the configuration of different tests get thrown around and override others, breaking tests meant to run with specific configurations. Originally on SO: https://stackoverflow.com/questions/70268026/quarkus-mixed-up-test-configuration-when-using-resourcearg

So, I have a service that has tests that run in different configuration setups. The main difference at the moment is the service can either manage its own authentication or get it from an external source (Keycloak).

I firstly control this using test profiles, which seem to work fine. Unfortunately, in order to support both cases, the ResourceLifecycleManager I have setup supports setting up a Keycloak instance and returns config values that break the config for self authentication (This is due primarily to the fact that I have not found out how to get the lifecycle manager to determine on its own what profile or config is currently running. If I could do this, I think I would be much better off than using @ResourceArg, so would love to know if I missed something here. Maybe a potential improvement for the future).

To remedy this shortcoming, I have attempted to use @ResourceArgs to convey to the lifecycle manager when to setup for external auth. However, I have noticed some really odd execution timings and the config that ends up at my test/service isn't what I intend based on the test class's annotations, where it is obvious the lifecycle manager has setup for external auth.

To be specific, the config value that breaks things for self-auth tests is mp.jwt.verify.publickey.location, as the code grabs the public key from Keycloak to verify jwt's (which breaks the verification of self-signed keys!)

Additionally, it should be noted that I have my tests ordered such that the profiles and configs shouldn't be running out of order; all the tests that don't care are run first, then the 'normal' tests with self auth, then the tests with the external auth profile. I can see this working appropriately when I run in intellij, and the fact I can tell the time is being taken to start up the new service instance between the test profiles.

Looking at the logs when I throw a breakpoint in places, some odd things are obvious:

  • When breakpoint on an erring test (before the external-configured tests run)
    • The start() method of my TestResourceLifecycleManager has been called twice
      • The first run ran with Keycloak starting, would override/break config
        • though the time I would expect to need to be taken to start up keycloak not happening, a little confused here
      • The second run is correct, not starting keycloak
    • The profile config is what is expected, except for what the keycloak setup overrides
  • When breakpoint on an external-configured test (after all self-configured tests run):
    • The start() method has now been called 4 times; appears that things were started in the same order as before again for the new run of the app

There could be some weirdness in how Intellij/Gradle shows logs, but I am interpreting this as:

  • Quarkus initting the two instances of LifecycleManager when starting the app for some reason, and one's config overrides the other, causing my woes.
  • The lifecycle manager is working as expected; it appropriately starts/ doesn't start keycloak when configured either way

At this point I can't tell if I'm doing something wrong, or if there's a bug.

Test class example for self-auth test (same annotations for all tests in this (test) profile):

@Slf4j
@QuarkusTest
@QuarkusTestResource(TestResourceLifecycleManager.class)
@TestHTTPEndpoint(Auth.class)
class AuthTest extends RunningServerTest {

Test class example for external auth test (same annotations for all tests in this (externalAuth) profile):

@Slf4j
@QuarkusTest
@TestProfile(ExternalAuthTestProfile.class)
@QuarkusTestResource(value = TestResourceLifecycleManager.class, initArgs = @ResourceArg(name=TestResourceLifecycleManager.EXTERNAL_AUTH_ARG, value="true"))
@TestHTTPEndpoint(Auth.class)
class AuthExternalTest extends RunningServerTest {

ExternalAuthTestProfile extends this, providing the appropriate profile name:

public class NonDefaultTestProfile implements QuarkusTestProfile {

    private final String testProfile;
    private final Map<String, String> overrides = new HashMap<>();

    protected NonDefaultTestProfile(String testProfile) {
        this.testProfile = testProfile;
    }

    protected NonDefaultTestProfile(String testProfile, Map<String, String> configOverrides) {
        this(testProfile);
        this.overrides.putAll(configOverrides);
    }

    @Override
    public Map<String, String> getConfigOverrides() {
        return new HashMap<>(this.overrides);
    }

    @Override
    public String getConfigProfile() {
        return testProfile;
    }

    @Override
    public List<TestResourceEntry> testResources() {
        return QuarkusTestProfile.super.testResources();
    }
}

Lifecycle manager:

@Slf4j
public class TestResourceLifecycleManager implements QuarkusTestResourceLifecycleManager {
    public static final String EXTERNAL_AUTH_ARG = "externalAuth";

    private static volatile MongodExecutable MONGO_EXE = null;
    private static volatile KeycloakContainer KEYCLOAK_CONTAINER = null;

	private boolean externalAuth = false;

    public synchronized Map<String, String> startKeycloakTestServer() {
        if(!this.externalAuth){
            log.info("No need for keycloak.");
            return Map.of();
        }
        if (KEYCLOAK_CONTAINER != null) {
            log.info("Keycloak already started.");
        } else {
            KEYCLOAK_CONTAINER = new KeycloakContainer()
//				.withEnv("hello","world")
                    .withRealmImportFile("keycloak-realm.json");
            KEYCLOAK_CONTAINER.start();
            log.info(
                    "Test keycloak started at endpoint: {}\tAdmin creds: {}:{}",
                    KEYCLOAK_CONTAINER.getAuthServerUrl(),
                    KEYCLOAK_CONTAINER.getAdminUsername(),
                    KEYCLOAK_CONTAINER.getAdminPassword()
            );

        }
        String clientId;
        String clientSecret;
        String publicKey = "";
        try (
                Keycloak keycloak = KeycloakBuilder.builder()
                        .serverUrl(KEYCLOAK_CONTAINER.getAuthServerUrl())
                        .realm("master")
                        .grantType(OAuth2Constants.PASSWORD)
                        .clientId("admin-cli")
                        .username(KEYCLOAK_CONTAINER.getAdminUsername())
                        .password(KEYCLOAK_CONTAINER.getAdminPassword())
                        .build();
        ) {
            RealmResource appsRealmResource = keycloak.realms().realm("apps");

            ClientRepresentation qmClientResource = appsRealmResource.clients().findByClientId("quartermaster").get(0);

            clientSecret = qmClientResource.getSecret();

            log.info("Got client id \"{}\" with secret: {}", "quartermaster", clientSecret);

            //get private key
            for (KeysMetadataRepresentation.KeyMetadataRepresentation curKey : appsRealmResource.keys().getKeyMetadata().getKeys()) {
                if (!SIG.equals(curKey.getUse())) {
                    continue;
                }
                if (!"RSA".equals(curKey.getType())) {
                    continue;
                }
                String publicKeyTemp = curKey.getPublicKey();
                if (publicKeyTemp == null || publicKeyTemp.isBlank()) {
                    continue;
                }
                publicKey = publicKeyTemp;
                log.info("Found a relevant key for public key use: {} / {}", curKey.getKid(), publicKey);
            }
        }
        // write public key
        // = new File(TestResourceLifecycleManager.class.getResource("/").toURI().toString() + "/security/testKeycloakPublicKey.pem");
        File publicKeyFile;
        try {
            publicKeyFile = File.createTempFile("oqmTestKeycloakPublicKey",".pem");
//            publicKeyFile = new File(TestResourceLifecycleManager.class.getResource("/").toURI().toString().replace("/classes/java/", "/resources/") + "/security/testKeycloakPublicKey.pem");
            log.info("path of public key: {}", publicKeyFile);
//            if(publicKeyFile.createNewFile()){
//                log.info("created new public key file");
//
//            } else {
//                log.info("Public file already exists");
//            }
            try (
                    FileOutputStream os = new FileOutputStream(
                            publicKeyFile
                    );
            ) {
                IOUtils.write(publicKey, os, UTF_8);
            } catch (IOException e) {
                log.error("Failed to write out public key of keycloak: ", e);
                throw new IllegalStateException("Failed to write out public key of keycloak.", e);
            }
        } catch (IOException  e) {
            log.error("Failed to create public key file: ", e);
            throw new IllegalStateException("Failed to create public key file", e);
        }

        String keycloakUrl = KEYCLOAK_CONTAINER.getAuthServerUrl().replace("/auth", "");

        return Map.of(
                "test.keycloak.url", keycloakUrl,
                "test.keycloak.authUrl", KEYCLOAK_CONTAINER.getAuthServerUrl(),
                "test.keycloak.adminName", KEYCLOAK_CONTAINER.getAdminUsername(),
                "test.keycloak.adminPass", KEYCLOAK_CONTAINER.getAdminPassword(),
                //TODO:: add config for server to talk to
                "service.externalAuth.url", keycloakUrl,
                "mp.jwt.verify.publickey.location", publicKeyFile.getAbsolutePath()


        );
    }

    public static synchronized void startMongoTestServer() throws IOException {
        if (MONGO_EXE != null) {
            log.info("Flapdoodle Mongo already started.");
            return;
        }
        Version.Main version = Version.Main.V4_0;
        int port = 27018;
        log.info("Starting Flapdoodle Test Mongo {} on port {}", version, port);
        IMongodConfig config = new MongodConfigBuilder()
                .version(version)
                .net(new Net(port, Network.localhostIsIPv6()))
                .build();
        try {
            MONGO_EXE = MongodStarter.getDefaultInstance().prepare(config);
            MongodProcess process = MONGO_EXE.start();
            if (!process.isProcessRunning()) {
                throw new IOException();
            }
        } catch (Throwable e) {
            log.error("FAILED to start test mongo server: ", e);
            MONGO_EXE = null;
            throw e;
        }
    }

    public static synchronized void stopMongoTestServer() {
        if (MONGO_EXE == null) {
            log.warn("Mongo was not started.");
            return;
        }
        MONGO_EXE.stop();
        MONGO_EXE = null;
    }

    public synchronized static void cleanMongo() throws IOException {
        if (MONGO_EXE == null) {
            log.warn("Mongo was not started.");
            return;
        }

        log.info("Cleaning Mongo of all entries.");
    }


    @Override
    public void init(Map<String, String> initArgs) {
        this.externalAuth = Boolean.parseBoolean(initArgs.getOrDefault(EXTERNAL_AUTH_ARG, Boolean.toString(this.externalAuth)));
    }

    @Override
    public Map<String, String> start() {
        log.info("STARTING test lifecycle resources.");
        Map<String, String> configOverride = new HashMap<>();
        try {
            startMongoTestServer();
        } catch (IOException e) {
            log.error("Unable to start Flapdoodle Mongo server");
        }

        configOverride.putAll(startKeycloakTestServer());

        return configOverride;
    }

    @Override
    public void stop() {
        log.info("STOPPING test lifecycle resources.");
        stopMongoTestServer();
    }
}

Expected behavior

In any particular test class, I expect the configuration of the running quarkus app to match what the annotations set.

Actual behavior

The configuration of the running tests isn't what the test class annotation set.

How to Reproduce?

The app can be found here: https://github.com/Epic-Breakfast-Productions/OpenQuarterMaster/tree/main/software/open-qm-base-station

The tests are currently failing in the ways I am describing, so feel free to look around.

Note that to run this, you will need to run ./gradlew build publishToMavenLocal in https://github.com/Epic-Breakfast-Productions/OpenQuarterMaster/tree/main/software/libs/open-qm-core to install a dependency locally.

Output of uname -a or ver

Linux GeneralDevBox 5.11.0-41-generic #45~20.04.1-Ubuntu SMP Wed Nov 10 10:20:10 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.11" 2021-04-20 OpenJDK Runtime Environment (build 11.0.11+9-Ubuntu-0ubuntu2.20.04) OpenJDK 64-Bit Server VM (build 11.0.11+9-Ubuntu-0ubuntu2.20.04, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.5.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 7.3

Additional information

No response

@GregJohnStewart GregJohnStewart added the kind/bug Something isn't working label Dec 8, 2021
@geoand
Copy link
Contributor

geoand commented Dec 8, 2021

Thanks for reporting.

I tried to run the reproducer but got:

Execution failed for task ':quarkusGenerateCodeTests'.
> Could not resolve all files for configuration ':testRuntimeClasspath'.
   > Could not find snakeyaml-1.29-android.jar (org.yaml:snakeyaml:1.29).
     Searched in the following locations:
         file:/home/gandrian/.m2/repository/org/yaml/snakeyaml/1.29/snakeyaml-1.29-android.jar


@GregJohnStewart
Copy link
Contributor Author

Try deleting that jar (/home/gandrian/.m2/repository/org/yaml/snakeyaml/1.29/snakeyaml-1.29-android.jar). I have seen this sporadically in the past but should work, I have confirmed it should work on clean systems. If deleting that doesn't work, let me know

@geoand
Copy link
Contributor

geoand commented Dec 8, 2021

Didn't work unfortunately, the issue persists

@GregJohnStewart
Copy link
Contributor Author

That's super odd.. I know it's coming from io.quarkus:quarkus-config-yaml, and really unsure what might be the fix. Is there a test for that extension you could try? I would try to reproduce but it's not an error I can seem to reproduce. I think the core issue is with the extension, but I am unable to reproduce due to gradle caching dependencies outside of the .m2 directory. Still poking around though

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

quarkus-config-yaml is used for example in https://github.com/quarkusio/quarkus/tree/2.6.0.CR1/integration-tests/smallrye-config.

Is there any chance you can reproduce the same problem in a simpler (preferably Maven) project?

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Just to try to get a sense of what is going on, which QuarkusTestResourceLifecycleManager implementations do you expect to be run?

@GregJohnStewart
Copy link
Contributor Author

There should be only one implementation to run: https://github.com/Epic-Breakfast-Productions/OpenQuarterMaster/blob/main/software/open-qm-base-station/src/test/java/com/ebp/openQuarterMaster/baseStation/testResources/TestResourceLifecycleManager.java

and I created a simpler reproducer:

https://github.com/Epic-Breakfast-Productions/OpenQuarterMaster/tree/main/software/plugins/open-qm-plugin-demo

It has two simple tests verifying the config is set correctly, and showing that config is overridden when it shouldn't be

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Great, thanks!

I'll try it out soon

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Is that reproducer part of a different project? Because if it isn't, I'm likely to face the same Gradle issue

@GregJohnStewart
Copy link
Contributor Author

Same git repo, different quarkus project, much fewer dependencies (no yaml!)

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Cool, thanks

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

I want to get some clarity on what your expectations are on when these QuarkusTestResourceLifecycleManager will be executed.
Can you describe what you expect will happen? Asking because I am pretty sure this is a misalignment of how QuarkusTestResourceLifecycleManager is handled in practive and how you expect it is handled.

@GregJohnStewart
Copy link
Contributor Author

GregJohnStewart commented Dec 9, 2021

I expect that the tests within a test class are run against a quarkus service configured in the way described via the class annotations.

I feel like it might be a common use case to provide options to the lifecycle manager that would have an effect on running configuration (that differs between tests/classes) and therefore need to be test-class-specific. As in my op, I wish I didn't have to use the args but AFAIK there is no way to determine anything about the run from the lifecycle manager (know current profile, access to existing configs, etc). If I could know there if Keycloak is needed or not, I could avoid needing the arg to tell me.

@GregJohnStewart
Copy link
Contributor Author

Additionally, I am personally okay with the caveat of "the server will need restarted between tests of different configs" as we already do this with test profiles.

Perhaps there could be a ClassOrderer that can be provided to help automate this, and order tests based on test profile/ @QuarkusTest*/@TestProfile annotations so devs wouldn't have to create their own to optimize the ordering for # of server restarts?

geoand added a commit to geoand/quarkus that referenced this issue Dec 9, 2021
This applies for implementations that don't specify the order

Relates to: quarkusio#22025
@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

I think there is a misconception about when QuarkusTestResourceLifecycleManager is applied. Basically, any use of @QuarkusTestResource() without restrictToAnnotatedClass set to true, means that the QuarkusTestResourceLifecycleManager will be applied to all tests no matter where the annotation is placed. I know this is non-intuitive behavior, but it can't be changed now, and furthermore it's documented.

So in your case, it seems like two TestResourceLifecycleManager are being used - so depending on which is run last, you get different results. #22068 will at least make the order predictable.

@GregJohnStewart
Copy link
Contributor Author

Okay, that does make sense, I was unaware of the restrictToAnnotatedClass option, and seems to resolve my issues.

That being said, I feel it's a little bit of an imperfect solution as I need to apply this argument to all tests of the separate profile. I might still ask for a config value to apply this globally or make the logic smarter to know when to use what lifecycle manager based on profile/ lifecycle manager/ manager args

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

I would suggest opening a new issue asking for exactly that and can consider it.

@geoand geoand closed this as completed Dec 9, 2021
@geoand geoand added the kind/question Further information is requested label Dec 9, 2021
Postremus pushed a commit to Postremus/quarkus that referenced this issue Dec 10, 2021
This applies for implementations that don't specify the order

Relates to: quarkusio#22025
gsmet pushed a commit to gsmet/quarkus that referenced this issue Dec 13, 2021
This applies for implementations that don't specify the order

Relates to: quarkusio#22025

(cherry picked from commit 232f3de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working kind/question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants