Remove empty ssl file upload #518

jan-law · 2021-06-16T16:43:58Z

Fixes #494

I'd appreciate some feedback to make the code more readable.

After the certValidator tries to parse the cert and throws an exception,filePath.toFile().delete() returns false if it failed. I'm not sure what's the cleanest way to handle this error since it's already in a try/catch block?

jan-law · 2021-06-16T16:52:23Z

If I try to run the integration tests, the ClientAssetsIT throws a 500 error when it tries to download index.html:

ClientAssetsIT.setup:62 » Execution java.lang.Exception: HTTP 500

I haven't altered this test file. Any ideas about what happened?

andrewazores · 2021-06-16T17:57:04Z

If I try to run the integration tests, the ClientAssetsIT throws a 500 error when it tries to download index.html:

ClientAssetsIT.setup:62 » Execution java.lang.Exception: HTTP 500

I haven't altered this test file. Any ideas about what happened?

This will happen if you run itests with -Dcryostat.minimal=false, since a minimal build does not include the web-client assets. If you didn't include that -D property then something else is up, but the CI runner passed the integration tests so it looks okay from here.

src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java

jan-law · 2021-06-16T18:04:40Z

This will happen if you run itests with -Dcryostat.minimal=false, since a minimal build does not include the web-client assets. If you didn't include that -D property then something else is up, but the CI runner passed the integration tests so it looks okay from here.

Ah, that's exactly what happened, thanks.

src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java

src/test/java/io/cryostat/net/web/http/api/v2/CertificatePostHandlerTest.java

andrewazores

Looks good other than that last tiny unit test comment. Would you like to try your hand at writing an integration test to exercise this as well?

jan-law · 2021-06-18T14:42:30Z

Looks good other than that last tiny unit test comment. Would you like to try your hand at writing an integration test to exercise this as well?

Yes, that would be useful to learn. I'll let you know if I have questions

src/test/java/itest/UploadCertificateIT.java

* Delete empty file when handling exception * Add unit test, apply formatting * Handle case when file.delete() fails * Prevent empty file creation * Close outstream when finished writing * Refactor file write, verify file not created * Pass outputStreamFunction to constructor * Apply spotless formatting * Add integration test * Move empty cert to test resources * Rename test * Remove unneeded assertion

* fix(rules): rule archiving and re-activation bugfixes * refactor * blocking * activate concurrently * cancel job when target disappears * fixup! cancel job when target disappears * apply existing rules to newly appearing targets

jan-law requested a review from andrewazores June 16, 2021 16:53