diff --git a/README.md b/README.md index 0bcf824..6b4cdfd 100644 --- a/README.md +++ b/README.md @@ -316,27 +316,28 @@ function emits a non-fatal warning when both are configured. ### Query Interval `queryInterval` accepts a Go duration string (for example `10m`, `1h` or `90s`). -When set, the function records the timestamp of its last successful query -alongside the result stored at the status target and skips querying Microsoft -Graph again until the interval has elapsed — regardless of how frequently the -Composition reconciles. +When set, the function records the timestamp of its last successful query and +skips querying Microsoft Graph again until the interval has elapsed, regardless +of how frequently the Composition reconciles. ```yaml target: "status.validatedUsers" queryInterval: "10m" ``` -The timestamp is persisted as an extra `lastQueryTime` element appended to the -result list at the status target, for example: +The query result at the target is left untouched. The timestamp is persisted in +a separate `status.lastQueryTimestamps` map, keyed by target, so the result +stays a clean list for downstream consumers: ```yaml status: - validatedUsers: + validatedUsers: # unchanged: a plain list of results - id: "a1b2c3" displayName: "Jane Doe" userPrincipalName: "jane@example.com" mail: "jane@example.com" - - lastQueryTime: "2026-07-16T10:00:00Z" + lastQueryTimestamps: # separate metadata, keyed by target + validatedUsers: "2026-07-16T10:00:00Z" ``` Because the interval check reads the persisted timestamp back from the XR status @@ -346,9 +347,9 @@ is not persisted across reconciles) or in Operation mode, where the query cadence is controlled by the `CronOperation` schedule or `WatchOperation` instead. -> Note: consumers of the result list should ignore the trailing element that -> carries `lastQueryTime` (it has no `id`), as it is metadata rather than a query -> result. +> Note: the XRD status schema must allow the `lastQueryTimestamps` field (for +> example via `x-kubernetes-preserve-unknown-fields`), the same as any +> function-managed status field. ## Using Reference Fields diff --git a/example/README.md b/example/README.md index 37540f3..70f8657 100644 --- a/example/README.md +++ b/example/README.md @@ -124,20 +124,20 @@ crossplane render xr.yaml service-principal-example-spec-ref.yaml functions.yaml ### 5. Query Interval (Throttling) -Throttle calls to Microsoft Graph with `queryInterval` (a Go duration string, e.g. `10m`). On a successful query the function appends a `lastQueryTime` timestamp to the results at the status target and, on later reconciles, skips querying until the interval has elapsed. It is only effective in Composition mode with a `status.` target. +Throttle calls to Microsoft Graph with `queryInterval` (a Go duration string, e.g. `10m`). On a successful query the function records a timestamp under `status.lastQueryTimestamps` (keyed by target), leaving the result list clean, and on later reconciles skips querying until the interval has elapsed. It is only effective in Composition mode with a `status.` target. -Run the query and observe the appended `lastQueryTime` element under `status.validatedUsers`: +Run the query and observe the recorded timestamp under `status.lastQueryTimestamps` (the result list at `status.validatedUsers` stays a plain list): ```shell crossplane render xr.yaml user-validation-example-query-interval.yaml functions.yaml --function-credentials=./secrets/azure-creds.yaml -r ``` -To observe the skip, render against an XR that already holds a recent `lastQueryTime`. `crossplane render` treats the XR file as the observed composite, so the function reads that timestamp and, while the interval has not elapsed, skips the query and emits a `FunctionSkip`/`IntervalLimit` condition instead of calling Graph: +To observe the skip, render against an XR that already records a recent timestamp in `status.lastQueryTimestamps`. `crossplane render` treats the XR file as the observed composite, so the function reads that timestamp and, while the interval has not elapsed, skips the query and emits a `FunctionSkip`/`IntervalLimit` condition instead of calling Graph: ```shell crossplane render xr-with-last-query-time.yaml user-validation-example-query-interval.yaml functions.yaml --function-credentials=./secrets/azure-creds.yaml -r ``` -> `xr-with-last-query-time.yaml` uses a far-future `lastQueryTime` so the skip is deterministic; set it to a real recent time to test the natural elapsed boundary. The credentials are not used on the skip path (no Graph call is made), so they need not be valid for this command. +> `xr-with-last-query-time.yaml` uses a far-future timestamp in `status.lastQueryTimestamps` so the skip is deterministic; set it to a real recent time to test the natural elapsed boundary. The credentials are not used on the skip path (no Graph call is made), so they need not be valid for this command. > **macOS note:** these commands use `-r` (function results) rather than `-rc`. The `-c`/`--include-context` flag makes `crossplane render` v2.x run an internal context-extraction step over a unix socket bind-mounted into its Docker helper container, which Docker Desktop for macOS does not support (`connect: operation not supported`) — the render then hangs with no output. Since the query-interval results are written to a `status.` target, `-c` is unnecessary here. This is a known CLI bug ([crossplane/cli#161](https://github.com/crossplane/cli/issues/161)), fixed by [#163](https://github.com/crossplane/cli/pull/163) (context function now listens on TCP) but not yet in a tagged release as of CLI v2.4.0. Until then, drop `-c` on macOS, or run `crossplane render` on Linux / a CLI built from `main` if you need context output. diff --git a/example/definition.yaml b/example/definition.yaml index 250b803..f018e88 100644 --- a/example/definition.yaml +++ b/example/definition.yaml @@ -103,6 +103,10 @@ spec: items: type: object x-kubernetes-preserve-unknown-fields: true + lastQueryTimestamps: + description: queryInterval last successful query timestamps, keyed by target + type: object + x-kubernetes-preserve-unknown-fields: true required: - spec type: object diff --git a/example/e2e/README.md b/example/e2e/README.md index b0dbef8..20228ad 100644 --- a/example/e2e/README.md +++ b/example/e2e/README.md @@ -35,9 +35,12 @@ kubectl apply -f example/e2e/xr.yaml ## Verify ```shell -# status carries the results plus a lastQueryTime element +# the result list stays a clean list of results kubectl get xr msgraph-query-interval-e2e -o jsonpath='{.status.validatedUsers}' | jq +# the query timestamp is recorded separately, keyed by target +kubectl get xr msgraph-query-interval-e2e -o jsonpath='{.status.lastQueryTimestamps}' | jq + # within the interval, reconciles skip (condition is set on the XR) kubectl get xr msgraph-query-interval-e2e -o jsonpath='{.status.conditions}' | jq # FunctionSkip/IntervalLimit @@ -49,7 +52,7 @@ kubectl logs -n crossplane-system "$POD" | grep -c 'interval limit' # skips Force reconciles with `kubectl annotate xr msgraph-query-interval-e2e poke=$(date +%s) --overwrite`. Poking repeatedly inside the 2m window leaves the query count flat; after 2m -elapses the next reconcile re-queries and `lastQueryTime` advances. +elapses the next reconcile re-queries and `status.lastQueryTimestamps.validatedUsers` advances. ## Notes diff --git a/example/e2e/function.yaml b/example/e2e/function.yaml index 0f7287b..9b9844c 100644 --- a/example/e2e/function.yaml +++ b/example/e2e/function.yaml @@ -4,4 +4,4 @@ kind: Function metadata: name: function-msgraph spec: - package: xpkg.upbound.io/upbound/function-msgraph:v0.7.0-rc3 + package: xpkg.upbound.io/upbound/function-msgraph:v0.7.0-rc4 diff --git a/example/xr-with-last-query-time.yaml b/example/xr-with-last-query-time.yaml index 6221379..dee4a52 100644 --- a/example/xr-with-last-query-time.yaml +++ b/example/xr-with-last-query-time.yaml @@ -1,11 +1,11 @@ -# XR that already carries a lastQueryTime in status.validatedUsers, used to -# demonstrate queryInterval skipping. `crossplane render` treats this XR as the -# observed composite, so the function reads the timestamp below and skips the -# Microsoft Graph query while the interval has not elapsed. +# XR that already records a lastQueryTimestamps entry for status.validatedUsers, +# used to demonstrate queryInterval skipping. `crossplane render` treats this XR +# as the observed composite, so the function reads the timestamp below and skips +# the Microsoft Graph query while the interval has not elapsed. # -# The lastQueryTime here is set far in the future so the skip is deterministic -# regardless of when you run the command. Change it to a real recent time -# (within your queryInterval window) to exercise the natural elapsed boundary. +# The timestamp is set far in the future so the skip is deterministic regardless +# of when you run the command. Change it to a real recent time (within your +# queryInterval window) to exercise the natural elapsed boundary. apiVersion: example.crossplane.io/v1 kind: XR metadata: @@ -16,4 +16,5 @@ status: displayName: "Existing User" userPrincipalName: "user@example.onmicrosoft.com" mail: "user@example.onmicrosoft.com" - - lastQueryTime: "2099-01-01T00:00:00Z" + lastQueryTimestamps: + validatedUsers: "2099-01-01T00:00:00Z" diff --git a/fn.go b/fn.go index 4ac17d0..09320c3 100644 --- a/fn.go +++ b/fn.go @@ -55,6 +55,14 @@ const ( LastExecutionQueryDriftDetectedAnnotation = "function-msgraph/last-execution-query-drift-detected" ) +const ( + // LastQueryTimestampsField is the XR status field under which the function + // records the timestamp of the last successful query, keyed by target. It is + // used by queryInterval to decide whether to skip a query, and is kept + // separate from the target so the query result itself stays a clean list. + LastQueryTimestampsField = "lastQueryTimestamps" +) + // GraphQueryInterface defines the methods required for querying Microsoft Graph API. type GraphQueryInterface interface { graphQuery(ctx context.Context, azureCreds map[string]string, in *v1beta1.Input) (interface{}, error) @@ -1019,17 +1027,18 @@ func (f *Function) putQueryResultToStatus(req *fnv1.RunFunctionRequest, rsp *fnv return err } - // Embed the query timestamp when interval limiting is configured so it can - // be read back on subsequent reconciles. - resultData := f.appendLastQueryTime(in, results) - - // Update the specific status field + // Update the specific status field with the raw query result. The result is + // written as-is so downstream consumers see a clean value at the target. statusField := strings.TrimPrefix(in.Target, "status.") - err = SetNestedKey(xrStatus, statusField, resultData) - if err != nil { - return errors.Wrapf(err, "cannot set status field %s to %v", statusField, resultData) + if err := SetNestedKey(xrStatus, statusField, results); err != nil { + return errors.Wrapf(err, "cannot set status field %s to %v", statusField, results) } + // When interval limiting is configured, record the query timestamp in a + // separate metadata field (keyed by target) so it can be read back on + // subsequent reconciles without polluting the result. + f.recordLastQueryTimestamp(in, xrStatus, statusField) + // Write the updated status field back into the composite resource if err := dxr.Resource.SetValue("status", xrStatus); err != nil { return errors.Wrap(err, "cannot write updated status back into composite resource") @@ -1042,32 +1051,26 @@ func (f *Function) putQueryResultToStatus(req *fnv1.RunFunctionRequest, rsp *fnv return nil } -// appendLastQueryTime embeds the current timestamp into the query result so the -// interval-based skip logic can read it back on subsequent reconciles. It is a -// no-op unless queryInterval is configured. Array results (the intended -// structure) get an extra {"lastQueryTime": ...} element; map results get a -// lastQueryTime field. -func (f *Function) appendLastQueryTime(in *v1beta1.Input, results interface{}) interface{} { +// recordLastQueryTimestamp stores the current timestamp under the +// LastQueryTimestampsField status map, keyed by the target field, when +// queryInterval is configured. Keeping it separate from the target leaves the +// query result untouched for downstream consumers. It is a no-op otherwise. +func (f *Function) recordLastQueryTimestamp(in *v1beta1.Input, xrStatus map[string]interface{}, statusField string) { if _, ok, _ := parseQueryInterval(in); !ok { - return results - } - - timestamp := f.timer.now() - switch data := results.(type) { - case []interface{}: - f.log.Debug("Added lastQueryTime element to array result", "target", in.Target, "queryInterval", *in.QueryInterval) - return append(data, map[string]interface{}{"lastQueryTime": timestamp}) - case map[string]interface{}: - data["lastQueryTime"] = timestamp - f.log.Debug("Added lastQueryTime to map result", "target", in.Target, "queryInterval", *in.QueryInterval) - return data - default: - f.log.Debug("Result data is neither array nor map, cannot add lastQueryTime", - "target", in.Target, - "resultType", fmt.Sprintf("%T", results), - "queryInterval", *in.QueryInterval) - return results + return } + + timestamps, ok := xrStatus[LastQueryTimestampsField].(map[string]interface{}) + if !ok { + timestamps = make(map[string]interface{}) + } + timestamps[statusField] = f.timer.now() + xrStatus[LastQueryTimestampsField] = timestamps + + f.log.Debug("Recorded last query timestamp", + "field", LastQueryTimestampsField, + "target", statusField, + "queryInterval", *in.QueryInterval) } func putQueryResultToContext(req *fnv1.RunFunctionRequest, rsp *fnv1.RunFunctionResponse, in *v1beta1.Input, results interface{}, f *Function) error { @@ -1397,8 +1400,8 @@ func parseQueryInterval(in *v1beta1.Input) (time.Duration, bool, error) { // shouldSkipQueryDueToInterval checks whether the configured queryInterval has // not yet elapsed since the last successful query, in which case the query -// should be skipped. Interval limiting relies on reading the timestamp persisted -// in the status target on a previous reconcile, so it only applies to +// should be skipped. Interval limiting relies on the timestamp persisted under +// the LastQueryTimestampsField on a previous reconcile, so it only applies to // Composition mode with a "status." target. func (f *Function) shouldSkipQueryDueToInterval(req *fnv1.RunFunctionRequest, in *v1beta1.Input, rsp *fnv1.RunFunctionResponse, inOperation bool) bool { interval, ok, err := parseQueryInterval(in) @@ -1413,104 +1416,42 @@ func (f *Function) shouldSkipQueryDueToInterval(req *fnv1.RunFunctionRequest, in return false } - targetData, err := f.getStatusTargetData(req, in, inOperation) + lastQueryTime, err := f.getLastQueryTimestamp(req, in, inOperation) if err != nil { - f.log.Debug("No existing target data for interval check, running query", "target", in.Target, "error", err) - return false - } - - lastQueryTime, err := f.extractLastQueryTime(targetData) - if err != nil { - f.log.Debug("No previous lastQueryTime found for interval check, running query", "target", in.Target, "error", err) + f.log.Debug("No previous query timestamp for interval check, running query", "target", in.Target, "error", err) return false } return f.checkIntervalLimit(lastQueryTime, interval, *in.QueryInterval, in.Target, rsp) } -// getStatusTargetData retrieves the current value stored at the status target -// from the observed XR status. -func (f *Function) getStatusTargetData(req *fnv1.RunFunctionRequest, in *v1beta1.Input, inOperation bool) (interface{}, error) { +// getLastQueryTimestamp reads the timestamp recorded for the target from the +// LastQueryTimestampsField map on the observed XR status. +func (f *Function) getLastQueryTimestamp(req *fnv1.RunFunctionRequest, in *v1beta1.Input, inOperation bool) (time.Time, error) { xrStatus, _, err := f.getOXRAndStatus(req, inOperation) if err != nil { - return nil, errors.Wrap(err, "cannot get XR status for interval check") + return time.Time{}, errors.Wrap(err, "cannot get XR status for interval check") } - statusField := strings.TrimPrefix(in.Target, "status.") - parts, err := ParseNestedKey(statusField) - if err != nil { - return nil, err - } - - currentValue := interface{}(xrStatus) - for _, k := range parts { - nestedMap, ok := currentValue.(map[string]interface{}) - if !ok { - return nil, errors.New("invalid nested structure") - } - nextValue, exists := nestedMap[k] - if !exists { - return nil, errors.New("no existing data") - } - currentValue = nextValue - } - - return currentValue, nil -} - -// extractLastQueryTime extracts and parses the lastQueryTime from target data, -// supporting both array results (the intended structure) and map results. -func (f *Function) extractLastQueryTime(targetData interface{}) (time.Time, error) { - if dataArray, ok := targetData.([]interface{}); ok { - return f.extractLastQueryTimeFromArray(dataArray) - } - if dataMap, ok := targetData.(map[string]interface{}); ok { - return f.extractLastQueryTimeFromMap(dataMap) - } - return time.Time{}, errors.New("target data is neither array nor map") -} - -// extractLastQueryTimeFromArray extracts lastQueryTime from the special element -// appended to array results. -func (f *Function) extractLastQueryTimeFromArray(dataArray []interface{}) (time.Time, error) { - for i := len(dataArray) - 1; i >= 0; i-- { - element, ok := dataArray[i].(map[string]interface{}) - if !ok { - continue - } - lastQueryTimeStr, exists := element["lastQueryTime"] - if !exists { - continue - } - lastQueryTimeString, ok := lastQueryTimeStr.(string) - if !ok { - continue - } - lastQueryTime, err := time.Parse(time.RFC3339, lastQueryTimeString) - if err != nil { - f.log.Debug("Cannot parse lastQueryTime from array element", "error", err) - return time.Time{}, err - } - return lastQueryTime, nil + timestamps, ok := xrStatus[LastQueryTimestampsField].(map[string]interface{}) + if !ok { + return time.Time{}, errors.Errorf("no %s recorded", LastQueryTimestampsField) } - return time.Time{}, errors.New("no lastQueryTime element found in array") -} -// extractLastQueryTimeFromMap extracts lastQueryTime from a map result. -func (f *Function) extractLastQueryTimeFromMap(dataMap map[string]interface{}) (time.Time, error) { - lastQueryTimeStr, exists := dataMap["lastQueryTime"] - if !exists { - return time.Time{}, errors.New("no lastQueryTime field") + statusField := strings.TrimPrefix(in.Target, "status.") + raw, ok := timestamps[statusField] + if !ok { + return time.Time{}, errors.Errorf("no query timestamp recorded for target %q", statusField) } - lastQueryTimeString, ok := lastQueryTimeStr.(string) + value, ok := raw.(string) if !ok { - return time.Time{}, errors.New("lastQueryTime is not a string") + return time.Time{}, errors.New("recorded query timestamp is not a string") } - lastQueryTime, err := time.Parse(time.RFC3339, lastQueryTimeString) + lastQueryTime, err := time.Parse(time.RFC3339, value) if err != nil { - f.log.Debug("Cannot parse lastQueryTime", "error", err) + f.log.Debug("Cannot parse recorded query timestamp", "error", err) return time.Time{}, err } diff --git a/fn_test.go b/fn_test.go index 15942d4..0042956 100644 --- a/fn_test.go +++ b/fn_test.go @@ -2993,11 +2993,11 @@ func TestRunFunction(t *testing.T) { "displayName": "Existing User", "userPrincipalName": "existing@example.com", "mail": "existing@example.com" - }, - { - "lastQueryTime": "2024-12-31T23:55:00+01:00" } - ] + ], + "lastQueryTimestamps": { + "validatedUsers": "2024-12-31T23:55:00+01:00" + } } }`), }, @@ -3039,11 +3039,11 @@ func TestRunFunction(t *testing.T) { "displayName": "Existing User", "userPrincipalName": "existing@example.com", "mail": "existing@example.com" - }, - { - "lastQueryTime": "2024-12-31T23:55:00+01:00" } - ] + ], + "lastQueryTimestamps": { + "validatedUsers": "2024-12-31T23:55:00+01:00" + } }}`), }, }, @@ -3076,11 +3076,11 @@ func TestRunFunction(t *testing.T) { "displayName": "Existing User", "userPrincipalName": "existing@example.com", "mail": "existing@example.com" - }, - { - "lastQueryTime": "2024-12-31T22:00:00+01:00" } - ] + ], + "lastQueryTimestamps": { + "validatedUsers": "2024-12-31T22:00:00+01:00" + } } }`), }, @@ -3122,11 +3122,11 @@ func TestRunFunction(t *testing.T) { "displayName": "Test User", "userPrincipalName": "user@example.com", "mail": "user@example.com" - }, - { - "lastQueryTime": "2025-01-01T00:00:00+01:00" } - ] + ], + "lastQueryTimestamps": { + "validatedUsers": "2025-01-01T00:00:00+01:00" + } }}`), }, }, @@ -3134,7 +3134,7 @@ func TestRunFunction(t *testing.T) { }, }, "RunQueryFirstTimeWithInterval": { - reason: "The Function should run the query and embed a lastQueryTime element on the first run when queryInterval is set and no prior data exists", + reason: "The Function should run the query and record the query timestamp on the first run when queryInterval is set and no prior data exists", args: args{ ctx: context.Background(), req: &fnv1.RunFunctionRequest{ @@ -3195,11 +3195,11 @@ func TestRunFunction(t *testing.T) { "displayName": "Test User", "userPrincipalName": "user@example.com", "mail": "user@example.com" - }, - { - "lastQueryTime": "2025-01-01T00:00:00+01:00" } - ] + ], + "lastQueryTimestamps": { + "validatedUsers": "2025-01-01T00:00:00+01:00" + } }}`), }, }, @@ -3327,11 +3327,11 @@ func TestRunFunction(t *testing.T) { "displayName": "Test User", "userPrincipalName": "user@example.com", "mail": "user@example.com" - }, - { - "lastQueryTime": "2025-01-01T00:00:00+01:00" } - ] + ], + "lastQueryTimestamps": { + "validatedUsers": "2025-01-01T00:00:00+01:00" + } }}`), }, },