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(jmx): restore error codes for auth and TLS connection failures #350

Merged
merged 9 commits into from
Apr 15, 2024

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Apr 2, 2024

Welcome to Cryostat3! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Fixes: #348

Description of the change:

Adds exception handling to the TargetConnectionManager's internal Uni pipeline, to catch and transform particular types of connection exceptions into HttpException with the correct status codes that the previous API defined and which the UI expects.

Motivation for the change:

This re-enables the ability for the UI to display the JMX auth and cert warning modals when attempting to connect to targets for which there are no correct stored credentials or which present an untrusted cert.

How to manually test:

  1. Check out and build PR, smoketest.bash -Ot
  2. Select 9093 sample app, go to Events view. Templates and types should load.
  3. Select 9094 sample app, go to Events view. Connection should fail and UI should display auth modal.
  4. Select 9095 sample app, go to Events view. Connection should fail and UI should display cert modal.

Current "bugs":

  • modal does not appear on the Recordings view when listing active recordings, but will appear if you click the "Create" button because the recording creation form will cause a query for event templates (triggering the same behaviour as the manual test above). This is because the GET /api/v3/targets/{id}/recordings endpoint for Cryostat 3.0 no longer initiates a direct JMX connection to the target as it did in previous releases. Instead, this endpoint only queries Cryostat's database for known ActiveRecordings. The JMX auth/cert failures would have been detected in the background when Cryostat discovered the target and attempted to connect. Related to feat(agent): enable Agent HTTP communications #342 (some of the refactoring done to do more synchronization of "external" active recordings) and [Epic] Redefine Target data model to allow multiple Connection URLs #71 (where targets' connection URLs would have a "connectable" state which could be used to flag and detect these cases).

@andrewazores andrewazores marked this pull request as ready for review April 3, 2024 14:28
@andrewazores
Copy link
Member Author

/build_test

Copy link

github-actions bot commented Apr 3, 2024

Workflow started at 4/3/2024, 10:54:30 AM. View Actions Run.

Copy link

github-actions bot commented Apr 3, 2024

CI build and push: All tests pass ✅ (JDK21)
https://github.com/cryostatio/cryostat3/actions/runs/8540870839

Copy link

github-actions bot commented Apr 3, 2024

No OpenAPI schema changes detected.

@andrewazores
Copy link
Member Author

/build_test

Copy link

github-actions bot commented Apr 3, 2024

Workflow started at 4/3/2024, 10:59:15 AM. View Actions Run.

Copy link

github-actions bot commented Apr 3, 2024

CI build and push: At least one test failed ❌ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8540870839

Copy link

github-actions bot commented Apr 3, 2024

CI build and push: All tests pass ✅ (JDK21)
https://github.com/cryostatio/cryostat3/actions/runs/8540939740

Copy link

github-actions bot commented Apr 3, 2024

No OpenAPI schema changes detected.

Copy link

github-actions bot commented Apr 3, 2024

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8540939740

Copy link
Member

@mwangggg mwangggg 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- just needs a rebase

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 4/15/2024, 9:02:02 AM. View Actions Run.

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8689604625

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Connection failures due to JMX Auth or JMX Untrusted TLS cert simply result in "Gateway Timeout"
2 participants