update submission received event and delete v1 azure events #15990

brick-green · 2024-09-24T17:53:16Z

This PR deletes the v1 azure event data classes and improves the event emitted from the submission controller. It now includes queryParamters, httpMethod, and full request url.

Test Steps:

./gradlew bootRun to make sure it compiles
run submissionController tests
run submissionControllerIntegration tests

Changes

update SubmissionReceivedEvent
add retrieval and filtering for query parameters
retrieve url and http method
delete unused azure event data classes

Checklist

Testing

Tested locally?
Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?

Linked Issues

Linked Update ReportFunction ReportReceivedEvent #13161

…mission-received-event

github-actions · 2024-09-24T17:53:32Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

github-actions · 2024-09-24T18:05:09Z

Test Results

1 232 tests ±0 1 228 ✅ ±0 7m 21s ⏱️ -18s
162 suites ±0 4 💤 ±0
162 files ±0 0 ❌ ±0

Results for commit 14f52b8. ± Comparison against base commit 2294f83.

♻️ This comment has been updated with latest results.

github-actions · 2024-09-24T18:25:10Z

Integration Test Results

53 files ±0 53 suites ±0 27m 26s ⏱️ -2s
410 tests ±0 401 ✅ ±0 9 💤 ±0 0 ❌ ±0
413 runs ±0 404 ✅ ±0 9 💤 ±0 0 ❌ ±0

Results for commit 14f52b8. ± Comparison against base commit 2294f83.

♻️ This comment has been updated with latest results.

jalbinson · 2024-09-25T14:38:11Z

prime-router/docs/observability/azure-events.md

-| mv-expand conditions
-| extend conditionDisplay = tostring(conditions.display)
+| where name == "ITEM_ACCEPTED"
+| extend conditionDisplay = tostring(parse_json(tostring(parse_json(tostring(parse_json(tostring(parse_json(tostring(parse_json(tostring(customDimensions.params)).bundleDigest)).observationSummaries))[0].testSummary))[0].conditions))[0].display)


there has to be a better way 😆

If it works, it works though.

I just used the UI in azure to extend on that field and that's what it gave me. I agree it looks kinda messy though

jalbinson · 2024-09-25T14:42:24Z

...s/src/main/kotlin/gov/cdc/prime/reportstream/submissions/controllers/SubmissionController.kt

-            blobUrl = blobClient.blobUrl
+            requestParameters = SubmissionDetails(
+                filterHeaders(headers),
+                filterQueryParameters(request.parameterMap.mapValues { it.value.joinToString(",") })


Would we break these headers back apart in the KQL query?

I don't understand. Those two filter functions return maps of the headers and parameters so that they are separated for querying

if we have the query string ?param=a&param=b&param=c then your code will serialize it to:

mapOf( "param" to "a,b,c" )

So then say we want to look for all events where param contains b we would have to break out "a,b,c" in KQL.

My suggestion is make the fields in SubmissionDetails to Map<String, List<String>> and let jackson do the serialization work so its easier to parse it back to json in KQL.

I set it up to account for a single query parameter to have multiple values. I did not set it up for headers though as I don't see a use case for that yet. I also think it's a little out of scope for this ticket as I would need to update the queue message then also update the FHIRReceiver to account for the new type. If we ever actually need to accept query parameters we'd also need to do the same.

works for me.

jalbinson · 2024-09-25T14:43:09Z

...s/src/main/kotlin/gov/cdc/prime/reportstream/submissions/controllers/SubmissionController.kt

@@ -237,6 +247,11 @@ class SubmissionController(
        return headers.filter { it.key.lowercase() in headersToInclude }
    }

+    private fun filterQueryParameters(queryParameters: Map<String, String>): Map<String, String> {
+        val headersToInclude = emptyList<String>() // update this list as new QueryParameters are needed


This should be moved to configuration.

Same with headers as well.

moved both to configuration based filtering. Really good call here!

… have multiple values

sonarqubecloud · 2024-09-25T18:22:11Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jalbinson · 2024-09-25T19:04:27Z

submissions/src/main/resources/application.properties

+
+# Allowed headers
+allowed.headers.client_id=client_id


Few things:

This being a map doesn't seem to serve a purpose since both key/value are the same(ish) and you ignore the key in code anyway. In code the type should just be List<String>.

I think adding an additional prefix would help clarify the configuration as well. Something like event.allowed.headers?

Last bit is I think yml really simplifies this file over a properties file. Here's what the result would look like:

event.allowed: headers: - client_id - content-type - payloadname - x-azure-clientip - content-length queryParameters: - ???

With a properties file there is much more duplication and you need to add indices.

event.allowed.headers[0]=client_id event.allowed.headers[1]=content-type ...

got it fixed like this. I did leave it as just allowed since this list is also used to filter out parameters for the queue message

jalbinson

Looks good! Thanks for bearing with me over the config stuff. Sets us up nicely for down the road.

arnejduranovic · 2024-09-26T14:44:14Z

...ons/src/main/kotlin/gov/cdc/prime/reportstream/submissions/config/AllowedParametersConfig.kt

+     * A list of allowed HTTP headers that can be accepted by the API.
+     * Each entry in the list represents a header name expected in the incoming request.
+     */
+    var headers: List<String> = emptyList()


This shouldn't be empty, right? Only queryParams for now? Off the top of my head, payloadName should be supported, along with a few others?

This is the field that holds the injected list of allowable header. It's defaulted to an empty list so that if someone in the future wants to allow all headers then they can. The list of allowable headers, including payloadName, is in the application.yml file.

…eived-event

brick-green added 4 commits September 23, 2024 15:53

remove ReportRouteEvent.kt

a5f38b1

remove events and update md

39df98d

update to include queryParameter

fa93fa0

Merge branch 'refs/heads/master' into platform/brick/13161-update-sub…

4421fed

…mission-received-event

brick-green added the platform Platform Team label Sep 24, 2024

brick-green temporarily deployed to staging September 24, 2024 17:53 — with GitHub Actions Inactive

update unit test

33ef013

brick-green temporarily deployed to staging September 24, 2024 19:11 — with GitHub Actions Inactive

brick-green marked this pull request as ready for review September 24, 2024 19:47

brick-green requested a review from a team as a code owner September 24, 2024 19:47

jalbinson self-assigned this Sep 25, 2024

jalbinson reviewed Sep 25, 2024

View reviewed changes

brick-green added 2 commits September 25, 2024 13:41

transition to config based filtering and allow one query parameter to…

925c812

… have multiple values

account for empty parameter filter list

202aa7f

brick-green temporarily deployed to staging September 25, 2024 17:57 — with GitHub Actions Inactive

jalbinson reviewed Sep 25, 2024

View reviewed changes

transition to application.yml

0111e61

brick-green temporarily deployed to staging September 25, 2024 20:14 — with GitHub Actions Inactive

jalbinson approved these changes Sep 25, 2024

View reviewed changes

arnejduranovic reviewed Sep 26, 2024

View reviewed changes

Merge branch 'master' into platform/brick/13161-update-submission-rec…

b215b6b

…eived-event

brick-green temporarily deployed to staging September 26, 2024 17:39 — with GitHub Actions Inactive

arnejduranovic approved these changes Sep 26, 2024

View reviewed changes

Merge branch 'master' into platform/brick/13161-update-submission-rec…

c7bb542

…eived-event

brick-green enabled auto-merge (squash) September 26, 2024 18:27

brick-green temporarily deployed to staging September 26, 2024 18:29 — with GitHub Actions Inactive

Merge branch 'master' into platform/brick/13161-update-submission-rec…

14f52b8

…eived-event

brick-green temporarily deployed to staging September 26, 2024 19:01 — with GitHub Actions Inactive

brick-green merged commit 1bf38e8 into master Sep 26, 2024
20 of 21 checks passed

brick-green deleted the platform/brick/13161-update-submission-received-event branch September 26, 2024 21:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update submission received event and delete v1 azure events #15990

update submission received event and delete v1 azure events #15990

brick-green commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

jalbinson Sep 25, 2024

brick-green Sep 25, 2024

jalbinson Sep 25, 2024

brick-green Sep 25, 2024

jalbinson Sep 25, 2024

brick-green Sep 25, 2024

jalbinson Sep 25, 2024

jalbinson Sep 25, 2024

jalbinson Sep 25, 2024

brick-green Sep 25, 2024

sonarqubecloud bot commented Sep 25, 2024

jalbinson Sep 25, 2024 •

edited

Loading

brick-green Sep 25, 2024

jalbinson left a comment

arnejduranovic Sep 26, 2024

brick-green Sep 26, 2024

update submission received event and delete v1 azure events #15990

update submission received event and delete v1 azure events #15990

Conversation

brick-green commented Sep 24, 2024 • edited Loading

Changes

Checklist

Testing

Linked Issues

github-actions bot commented Sep 24, 2024 • edited Loading

Dependency Review

OpenSSF Scorecard

Scanned Manifest Files

github-actions bot commented Sep 24, 2024 • edited Loading

Test Results

github-actions bot commented Sep 24, 2024 • edited Loading

Integration Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Sep 25, 2024

Quality Gate passed

jalbinson Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jalbinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brick-green commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

jalbinson Sep 25, 2024 •

edited

Loading