allthingssecurity commented on code in PR #28036:
URL: https://github.com/apache/camel/pull/28036#discussion_r4306582931
##########
components/camel-aws/camel-aws-secrets-manager/src/main/java/org/apache/camel/component/aws/secretsmanager/vault/CloudTrailReloadTriggerTask.java:
##########
@@ -273,26 +285,43 @@ public void run() {
if (!useSqsNotification) {
try {
- LookupEventsRequest.Builder eventsRequestBuilder =
LookupEventsRequest.builder()
- .maxResults(100)
-
.lookupAttributes(LookupAttribute.builder().attributeKey(LookupAttributeKey.EVENT_SOURCE)
-
.attributeValue(SECRETSMANAGER_AMAZONAWS_COM).build());
-
- if (lastTime != null) {
- eventsRequestBuilder.startTime(lastTime.plusMillis(1000));
- }
-
- LookupEventsRequest lookupEventsRequest =
eventsRequestBuilder.build();
-
- LookupEventsResponse response =
cloudTrailClient.lookupEvents(lookupEventsRequest);
- List<Event> events = response.events();
-
- if (!events.isEmpty()) {
- lastTime = events.get(0).eventTime();
- }
+ // read every page of the events since the last check, so that
an update is not left behind when
+ // there are more events (such as GetSecretValue calls) than
fit in one page
+ List<Event> events = new ArrayList<>();
+ String nextToken = null;
+ do {
+ LookupEventsRequest.Builder eventsRequestBuilder =
LookupEventsRequest.builder()
+ .maxResults(LOOKUP_MAX_RESULTS)
+
.lookupAttributes(LookupAttribute.builder().attributeKey(LookupAttributeKey.EVENT_SOURCE)
+
.attributeValue(SECRETSMANAGER_AMAZONAWS_COM).build());
Review Comment:
Good point, thanks. Done in 6be0d46e4684: each check now sends one lookup
with `LookupAttributeKey.EVENT_NAME` = `PutSecretValue` and one with
`UpdateSecret`, both from the same `lastTime`, reading every page of each, and
the merged events go through the same newest-time/event-id loop as before. The
`eventSource` check in the loop stays as a guard.
So reads of secrets no longer cost anything: a check sends 2 requests, plus
one per further 50 updates. The new test `readsOfSecretsDoNotAddLookupRequests`
(200 `GetSecretValue`/`DescribeSecret` events and one `UpdateSecret`) checks
that a check sends exactly those two requests, each with one `EVENT_NAME`
attribute, and that the `UpdateSecret` on the tracked secret reloads the
context. It fails on the previous commit (5 requests). The fake `LookupEvents`
in the test now applies the lookup attribute and fails on more than one. I also
updated the upgrade-guide sentence about the requests per check. The
camel-aws-secrets-manager tests pass (10 run, 0 failures).
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
