Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.oauth.TokenIntrospectionTest#testUnsupportedToken

Keycloak CI - Base IT (6)

java.lang.NullPointerException: Cannot invoke "com.fasterxml.jackson.databind.JsonNode.asBoolean()" because the return value of "com.fasterxml.jackson.databind.JsonNode.get(String)" is null
	at org.keycloak.testsuite.oauth.TokenIntrospectionTest.testUnsupportedToken(TokenIntrospectionTest.java:313)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
...

Report flaky test

@vmuzikar this is roughly what we talked about. I wasn't sure if we even need to have an env option for bootstrap-admin user username - we can have just the direct --username input, just --username:env, or allow either. I left it initially as just the direct --username input under the assumption that it's much less of a security concern that it appears on the cli.

Also do we want a password confirmation prompt?

we can have just the direct --username input, just --username:env, or allow either

It'd allow either. Plus, if someone doesn't specify neither, they will be prompted.

Also do we want a password confirmation prompt?

Sounds like a good idea.

It'd allow either. Plus, if someone doesn't specify neither, they will be prompted.

Ok, this now supports either --username or --username:env

Sounds like a good idea.

Added a confirmation prompt as well.

I still have not removed the old env / sys handling - that will come later. The expiration logic was left in place, it's just hidden.

@vmuzikar adding bootstrap-admin service account handling as well.

Some additional thoughts / questions:

• if the user specifies both the env variable for the admin user and service, should we create both or just prefer one?
• it could further simplify things if we dropped accepting the password / client secret at all from the cli - and instead just use a generated one that we can show on the console or put into a temp file.

@shawkins Thank you for the PR and sorry for the late review. I think the changes are a good start.

Added some comments.

Noticed also help is a bit misbehaving with list of commands:

kc.sh bootstrap-admin --help

...

Commands:

  service  Add an admin user with a password
  service  Add an admin user with a password

This appears to be due to our HelpFactory - it doesn't work well with multiple sub commands.

• if the user specifies both the env variable for the admin user and service, should we create both or just prefer one?

As a user, I'd expected both.

Ok, I'll remove the TODO.

• it could further simplify things if we dropped accepting the password / client secret at all from the cli - and instead just use a generated one that we can show on the console or put into a temp file.

CLI is required for automated scenarios without using env vars, e.g. the test framework.

Are we going to be testing the bootstrap-admin user and service cli in this way? I was thinking the cli options were only a hard requirement for the initial bootstrapping.

The help should be fixed now. Also starting the operator support.

Are we going to be testing the bootstrap-admin user and service cli in this way? I was thinking the cli options were only a hard requirement for the initial bootstrapping.

Yes, the CLI options are a requirement for bootstrapping server in general for the new test framework.

Also starting the operator support.

Nice! I'd suggest splitting that to a separate PR. This PR is getting quite big. :)

Are we going to be testing the bootstrap-admin user and service cli in this way? I was thinking the cli options were only a hard requirement for the initial bootstrapping.

Yes, the CLI options are a requirement for bootstrapping server in general for the new test framework.

Discussed offline and there is still a desire to let users specify the recover password / client secret.

Also starting the operator support.

Nice! I'd suggest splitting that to a separate PR. This PR is getting quite big. :)

Yes, it will be separate once this one is out of draft.

@shawkins I think we can move forward with the PR, I don't have any additional comments at the moment.

I think we can move forward with the PR, I don't have any additional comments at the moment.

Ok, I'll add some service account tests, then remove the operator draft stuff before moving this out of draft.

@mabartos This should be correct. The command basically starts the server and connects to DB etc. It's the same problem as with import/export commands.

@vmuzikar Ahh, you're right.

Using the service account is still failing.

@shawkins Created shawkins#13. I broke a few tests by my previous PR.