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

fix(discovery): leave Custom Targets enabled when builtins are disabled #1114

Merged
merged 5 commits into from
Oct 13, 2022

Conversation

andrewazores
Copy link
Member

Fixes #1113

  • use non-sa user by default for h2 setups
  • fix(discovery): leave Custom Targets enabled when built-ins are disabled
  • refactor to clean up instanceof check

- use non-sa user by default for h2 setups is not directly related to this PR but comes from discussions here:
cryostatio/cryostat-operator#474 (comment)

In describing how things worked I realized I had left sa (system administrator) as the default JDBC user, which was not intentional and is not necessary. Even for the H2 database scenarios,
Cryostat should not require the use of a system administrator account.

To test, try CRYOSTAT_DISABLE_BUILTIN_DISCOVERY=true sh smoketest.sh before and after this PR. In either case, JDP targets should not be discovered, only the quarkus-test targets that self-publish via the discovery plugin API. With this PR applied you should also be able to define custom targets either through the web-client or by API request, whereas without this PR applied this functionality will be broken and defining a custom target will appear to be a no-op (200 success status code but not notification and no custom target visible in UI).

@andrewazores andrewazores marked this pull request as ready for review October 13, 2022 21:42
Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :D

@andrewazores andrewazores merged commit 6dc4047 into cryostatio:main Oct 13, 2022
@andrewazores andrewazores deleted the builtin-undisable-customs branch October 13, 2022 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Task] DISABLE_BUILTIN_DISCOVERY env var should not disable Custom Targets
2 participants