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

feat(custom_targets): handle embedded credentials, enable tests #188

Merged
merged 27 commits into from
Dec 15, 2023

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Nov 23, 2023

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 #101
Related to #122
Based on #183
Depends on #183

Description of the change:

Re-enables the Custom Targets integration test that was ported over earlier. Along with this come some fixes to the initial 3.0 implementation so that the API responses match the pre-existing format, addition of handling of "embedded credentials" supplied with Custom Targets definitions, addition of handling to look up existing credentials from the keyring when opening a target connection, fixes to ensure that WebSocket messages are emitted in a more predictable order for clients to observe, some basic refactoring, and putting back the three instances of vertx-fib-demo like the old smoketest has.

How to manually test:

  1. Build PR locally, ./mvnw clean verify ; podman image prune -f and ensure all tests pass
  2. ./smoketest.bash -OXgtb or whichever combination of flags you like
  3. Go to Dashboard > Create Target view, try to create a new custom target. This will need to have a unique but existing/reachable JMX URL, so you can use service:jmx:rmi:///jndi/rmi://vertx-fib-demo-2:9094/jmxrmi. In the latest smoketest script, vertx-fib-demo-n and sample-app-n (n: [1, 2, 3]) are basically synonymous - both forms are resolvable hostnames from the perspective of the cryostat3 container. vertx-fib-demo-2:9094 and vertx-fib-demo-3:9095 are free to use as custom targets, the other URLs will be automatically discovered by JDP and the samples' Agent instances.
    image

With service:jmx:rmi:///jndi/rmi://vertx-fib-demo-2:9094/jmxrmi:

  1. Go to target creation view, enter this URL
  2. Test connection. It should fail due to missing credentials.
  3. Add the credentials admin:adminpass123 in the form, test the connection again. It should succeed.
  4. Replace the credentials with user:password, test again. It should still succeed.
  5. Replace the credentials with something else, test again. It should fail.
  6. Put back user:password and create the custom target. This should succeed and some notifications should appear. You will be brought back to the Topology view, which depends on GraphQL ([Story] GraphQL #11), so don't worry about that view being broken for now.
  7. Go to Recordings and try to create a new flight recording. This should fail with an HTTP 403.
  8. Go to Security and delete the credentials that match the JMX URL. Create new credentials with the same match expression, but this time using the admin:adminpass123 combination.
  9. Go back to Recordings and retry creating a recording. This should succeed.

With service:jmx:rmi:///jndi/rmi://vertx-fib-demo-3:9095/jmxrmi:

  1. Go to target creation view, enter this URL
  2. Test connection. It should fail due to "non-JRMP endpoint" - this is a little misleading but actually indicates the unknown SSL cert required by this test app instance.
  3. Creating the target should similarly fail.

With service:jmx:rmi:///jndi/rmi://cryostat3:9091/jmxrmi:

  1. Go to target creation view, enter this URL
  2. Test connection. This should succeed.
  3. Creating the target should succeed. Cryostat should succeed at automatically connecting to itself and via this URL you should also see the onstart recording that is already present.

Note: deleting a Custom Target is only exposed in the UI via the Topology view, so there is no way to undo this graphically yet. You can either tear down and restart the smoketest to get a fresh slate, or you can use curl/HTTPie and delete the custom target directly via the API:

$ http --auth=user:pass :8181/api/v3/targets
HTTP/1.1 200 OK
Cache-Control: no-cache
Content-Type: application/json;charset=UTF-8
content-encoding: gzip
content-length: 1762

[
    ...
    {
        "agent": false,
        "alias": "service%3Ajmx%3Armi%3A%2F%2F%2Fjndi%2Frmi%3A%2F%2Fvertx-fib-demo-2%3A9094%2Fjmxrmi",
        "annotations": {
            "cryostat": {
                "REALM": "Custom Targets"
            },
            "platform": {}
        },
        "connectUrl": "service:jmx:rmi:///jndi/rmi://vertx-fib-demo-2:9094/jmxrmi",
        "id": 9,
        "jvmId": "hk58o_x3gJT4jK0aLmd5XdBRrdIRKmHb3xKZKixAQDI=",
        "labels": {}
    }
]
$ http --auth=user:pass DELETE :8181/api/v3/targets/9
HTTP/1.1 204 No Content
content-encoding: identity

@andrewazores andrewazores added feat New feature or request safe-to-test labels Nov 23, 2023
@andrewazores andrewazores marked this pull request as draft November 23, 2023 16:50
@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 11/23/2023, 11:51:16 AM. View Actions Run.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/6972468439

1 similar comment
Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/6972468439

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 11/27/2023, 5:13:25 PM. View Actions Run.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7011255699

1 similar comment
Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7011255699

Copy link

This PR/issue depends on:

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 11/28/2023, 10:14:22 AM. View Actions Run.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7020796882

1 similar comment
Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7020796882

@andrewazores andrewazores marked this pull request as ready for review November 28, 2023 15:39
@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 11/28/2023, 10:39:38 AM. View Actions Run.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7021132421

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7021132421

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 12/15/2023, 2:40:39 PM. View Actions Run.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7226342002

1 similar comment
Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat3/actions/runs/7226342002

@andrewazores andrewazores merged commit 539940e into cryostatio:main Dec 15, 2023
8 checks passed
@andrewazores andrewazores deleted the custom-targets branch December 15, 2023 19:51
andrewazores added a commit to andrewazores/cryostat3 that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Remove gson in favour of jackson
2 participants