diff --git a/README.md b/README.md index 76aa89b..d42313a 100644 --- a/README.md +++ b/README.md @@ -348,6 +348,7 @@ spec: | `servicePrincipalsRef` | string | Reference to resolve a list of service principal names from `spec`, `status` or `context` (e.g., `spec.servicePrincipalConfig.names`) | | `target` | string | Required. Where to store the query results. Can be `status.` or `context.` | | `skipQueryWhenTargetHasData` | bool | Optional. When true, will skip the query if the target already has data | +| `skipQueryWhenRefMissing` | bool | Optional. When true, a `groupRef`, `groupsRef`, `usersRef` or `servicePrincipalsRef` whose path does not exist yet skips the query with a warning instead of failing the pipeline. The target is left untouched. Default is false. See [Skipping when a reference is not resolvable yet](#skipping-when-a-reference-is-not-resolvable-yet). | | `queryInterval` | string | Optional. Minimum interval between queries as a Go duration string (e.g. `10m`, `1h`, `90s`). Skips querying Microsoft Graph until the interval has elapsed since the last successful query, independent of reconcile frequency. Only effective in Composition mode with a `status.` target. | | `failOnEmpty` | bool | Optional. When true, the function will fail if the `users`, `groups`, or `servicePrincipals` lists are empty, or if their respective reference fields are empty lists. | | `activeAccount` | bool | Optional. `UserValidation` only. When true, only users whose Entra ID `accountEnabled` attribute is true are stored at the target; disabled users, and users whose account state Graph does not report, are omitted. | @@ -471,6 +472,39 @@ servicePrincipalsRef: "spec.servicePrincipalConfig.names" # Get service princip target: "status.servicePrincipals" ``` +### Skipping when a reference is not resolvable yet + +By default, a reference whose path does not exist is a fatal error, which aborts +the whole Composition pipeline. That can deadlock an XR when the referenced field +is written later in the pipeline or on a later reconcile, for example when a +later step creates the Entra ID group and records its name in the XR status. + +Set `skipQueryWhenRefMissing: true` to skip the query instead: + +```yaml +apiVersion: msgraph.fn.crossplane.io/v1alpha1 +kind: Input +queryType: GroupMembership +groupRef: "status.groups.owners.name" # Written by a later step once the group exists +target: "status.ownerMembers" +skipQueryWhenRefMissing: true +``` + +While the path is missing, the function does not call Microsoft Graph, leaves the +target untouched, passes the desired XR and context through unchanged, emits a +Warning result and sets a `FunctionSkip` condition with reason `RefNotResolved`. +The rest of the pipeline runs, and the query runs on the first reconcile where the +reference resolves. In Operation mode, a skipped run does not update the drift +annotations. + +Only a missing path is skipped. These remain fatal even with the flag set: + +- an empty or unsupported reference (for example one that does not start with + `spec.`, `status.` or `context.`) +- a path that exists but holds the wrong type (not a map, not a string array, or + a non-string value for `groupRef`) +- any Microsoft Graph error, including a group that does not exist + ## Using data from EnvironmentConfig To use data from a Crossplane `EnvironmentConfig`, first load the data into the context using [function-environment-configs](https://github.com/crossplane-contrib/function-environment-configs). This function should be in the pipeline before you use function-msgraph. diff --git a/example/README.md b/example/README.md index 2c81b37..d9c85ac 100644 --- a/example/README.md +++ b/example/README.md @@ -157,3 +157,13 @@ crossplane render xr.yaml user-validation-example.yaml functions.yaml --function ``` > The example intentionally omits `skipQueryWhenTargetHasData`. Enabling `activeAccount` on an XR whose target already holds disabled users does not purge them while the query is being skipped, so clear the target once (or drop the skip for one reconcile) when adopting the flag on an existing deployment. + +### 7. Skip When a Reference Is Missing + +With `skipQueryWhenRefMissing: true`, a reference whose path does not exist yet skips the query instead of failing the pipeline. `xr.yaml` has no `status.ownersGroup.name`, so the render below emits a Warning result and a `FunctionSkip`/`RefNotResolved` condition, makes no Graph call and leaves `status.ownerMembers` unset: + +```shell +crossplane render xr.yaml group-membership-example-skip-ref-missing.yaml functions.yaml --function-credentials=./secrets/azure-creds.yaml -r +``` + +Remove `skipQueryWhenRefMissing` from the example to see the default, fatal `cannot resolve groupRef: status.ownersGroup.name not found`. diff --git a/example/group-membership-example-skip-ref-missing.yaml b/example/group-membership-example-skip-ref-missing.yaml new file mode 100644 index 0000000..f3eebb4 --- /dev/null +++ b/example/group-membership-example-skip-ref-missing.yaml @@ -0,0 +1,28 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: group-membership-example-skip-ref-missing +spec: + compositeTypeRef: + apiVersion: example.crossplane.io/v1 + kind: XR + mode: Pipeline + pipeline: + - step: get-owner-members + functionRef: + name: function-msgraph + input: + apiVersion: msgraph.fn.crossplane.io/v1alpha1 + kind: Input + queryType: GroupMembership + # Written to the XR status by a later step once the group exists. + groupRef: status.ownersGroup.name + target: "status.ownerMembers" + # Skip (with a warning) instead of failing while the reference is missing. + skipQueryWhenRefMissing: true + credentials: + - name: azure-creds + source: Secret + secretRef: + namespace: crossplane-system + name: azure-account-creds diff --git a/fn.go b/fn.go index 1830c1f..4baa073 100644 --- a/fn.go +++ b/fn.go @@ -83,6 +83,10 @@ const ( unknownType = "unknown" ) +// errRefNotFound marks a reference whose path does not exist. Its text keeps +// the existing "cannot resolve ...: not found" messages unchanged. +var errRefNotFound = errors.New("not found") + // graphFieldNameRE matches the shape of a Microsoft Graph property name, // including OData extension attributes (extension__). var graphFieldNameRE = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_]*$`) @@ -1168,31 +1172,48 @@ func ParseNestedKey(key string) ([]string, error) { // GetNestedKey retrieves a nested string value from a map using dot notation keys. func GetNestedKey(context map[string]interface{}, key string) (string, bool) { - parts, err := ParseNestedKey(key) + value, err := lookupNestedValue(context, key) if err != nil { return "", false } + result, ok := value.(string) + return result, ok +} + +// lookupNestedValue returns the value at a nested key. It returns +// errRefNotFound only when a key along the path is absent. +func lookupNestedValue(root map[string]interface{}, key string) (interface{}, error) { + parts, err := ParseNestedKey(key) + if err != nil { + return nil, err + } - currentValue := interface{}(context) + currentValue := interface{}(root) for _, k := range parts { - // Check if the current value is a map - if nestedMap, ok := currentValue.(map[string]interface{}); ok { - // Get the next value in the nested map - if nextValue, exists := nestedMap[k]; exists { - currentValue = nextValue - } else { - return "", false - } - } else { - return "", false + nestedMap, ok := currentValue.(map[string]interface{}) + if !ok { + return nil, errors.Errorf("%s is not a map", k) + } + nextValue, exists := nestedMap[k] + if !exists { + return nil, errRefNotFound } + currentValue = nextValue } + return currentValue, nil +} - // Convert the final value to a string - if result, ok := currentValue.(string); ok { - return result, true +// resolveGroupRefValue resolves a groupRef string value from a map. +func resolveGroupRefValue(data map[string]interface{}, field, refKey string) (string, error) { + value, err := lookupNestedValue(data, field) + if errors.Is(err, errRefNotFound) { + return "", errors.Errorf("cannot resolve groupRef: %s %w", refKey, errRefNotFound) } - return "", false + result, ok := value.(string) + if err != nil || !ok { + return "", errors.Errorf("cannot resolve groupRef: %s not found", refKey) + } + return result, nil } // SetNestedKey sets a value to a nested key from a map using dot notation keys. @@ -1531,7 +1552,7 @@ func (f *Function) processGroupRef(req *fnv1.RunFunctionRequest, in *v1beta1.Inp groupName, err := f.resolveGroupRef(req, in.GroupRef, inOperation) if err != nil { - response.Fatal(rsp, err) + f.failOrSkipOnRef(in, rsp, "groupRef", *in.GroupRef, err) return false } in.Group = &groupName @@ -1547,7 +1568,7 @@ func (f *Function) processGroupsRef(req *fnv1.RunFunctionRequest, in *v1beta1.In groupNames, err := f.resolveGroupsRef(req, in.GroupsRef, inOperation) if err != nil { - response.Fatal(rsp, err) + f.failOrSkipOnRef(in, rsp, "groupsRef", *in.GroupsRef, err) return false } in.Groups = groupNames @@ -1563,7 +1584,7 @@ func (f *Function) processUsersRef(req *fnv1.RunFunctionRequest, in *v1beta1.Inp userNames, err := f.resolveUsersRef(req, in.UsersRef, inOperation) if err != nil { - response.Fatal(rsp, err) + f.failOrSkipOnRef(in, rsp, "usersRef", *in.UsersRef, err) return false } in.Users = userNames @@ -1579,7 +1600,7 @@ func (f *Function) processServicePrincipalsRef(req *fnv1.RunFunctionRequest, in spNames, err := f.resolveServicePrincipalsRef(req, in.ServicePrincipalsRef, inOperation) if err != nil { - response.Fatal(rsp, err) + f.failOrSkipOnRef(in, rsp, "servicePrincipalsRef", *in.ServicePrincipalsRef, err) return false } in.ServicePrincipals = spNames @@ -1587,6 +1608,25 @@ func (f *Function) processServicePrincipalsRef(req *fnv1.RunFunctionRequest, in return true } +// failOrSkipOnRef reports a reference resolution error. With +// skipQueryWhenRefMissing set, a missing reference path skips the query with a +// warning instead of failing; every other error remains fatal. +func (f *Function) failOrSkipOnRef(in *v1beta1.Input, rsp *fnv1.RunFunctionResponse, refType, refKey string, err error) { + if !ptr.Deref(in.SkipQueryWhenRefMissing, false) || !errors.Is(err, errRefNotFound) { + response.Fatal(rsp, err) + return + } + + f.log.Info("Reference not resolvable yet, skipping query", refType, refKey) + response.Warning(rsp, errors.Errorf("%s; skipping query (skipQueryWhenRefMissing)", err.Error())). + TargetCompositeAndClaim() + response.ConditionTrue(rsp, "FunctionSkip", "RefNotResolved"). + WithMessage(fmt.Sprintf("Query skipped: %s %s could not be resolved yet", refType, refKey)). + TargetCompositeAndClaim() + response.ConditionTrue(rsp, "FunctionSuccess", "Success"). + TargetCompositeAndClaim() +} + // executeAndProcessQuery executes the query and processes the results func (f *Function) executeAndProcessQuery(ctx context.Context, req *fnv1.RunFunctionRequest, in *v1beta1.Input, azureCreds map[string]string, rsp *fnv1.RunFunctionResponse, inOperation bool) bool { // Execute the query @@ -1818,22 +1858,14 @@ func (f *Function) resolveFromStatus(req *fnv1.RunFunctionRequest, refKey string } statusField := strings.TrimPrefix(refKey, "status.") - value, ok := GetNestedKey(xrStatus, statusField) - if !ok { - return "", errors.Errorf("cannot resolve groupRef: %s not found", refKey) - } - return value, nil + return resolveGroupRefValue(xrStatus, statusField, refKey) } // resolveFromContext resolves a reference from function context func (f *Function) resolveFromContext(req *fnv1.RunFunctionRequest, refKey string) (string, error) { contextMap := req.GetContext().AsMap() contextField := strings.TrimPrefix(refKey, "context.") - value, ok := GetNestedKey(contextMap, contextField) - if !ok { - return "", errors.Errorf("cannot resolve groupRef: %s not found", refKey) - } - return value, nil + return resolveGroupRefValue(contextMap, contextField, refKey) } // resolveFromSpec resolves a reference from XR spec @@ -1852,11 +1884,7 @@ func (f *Function) resolveFromSpec(req *fnv1.RunFunctionRequest, refKey string, } specField := strings.TrimPrefix(refKey, "spec.") - value, ok := GetNestedKey(xrSpec, specField) - if !ok { - return "", errors.Errorf("cannot resolve groupRef: %s not found", refKey) - } - return value, nil + return resolveGroupRefValue(xrSpec, specField, refKey) } // resolveStringArrayRef resolves a list of string values from a reference in spec, status or context @@ -1887,8 +1915,12 @@ func (f *Function) resolveStringArrayRef(req *fnv1.RunFunctionRequest, ref *stri // If we got an error and it contains "groupsRef" but we're looking for a different ref type, // replace it with the correct ref type if err != nil && refType != "groupsRef" && strings.Contains(err.Error(), "groupsRef") { - errMsg := err.Error() - return nil, errors.New(strings.ReplaceAll(errMsg, "groupsRef", refType)) + errMsg := strings.ReplaceAll(err.Error(), "groupsRef", refType) + if errors.Is(err, errRefNotFound) { + // Re-wrap so the sentinel survives the rewrite; the message is unchanged. + return nil, errors.Errorf("%s%w", strings.TrimSuffix(errMsg, errRefNotFound.Error()), errRefNotFound) + } + return nil, errors.New(errMsg) } return result, err @@ -1958,7 +1990,7 @@ func (f *Function) extractStringArrayFromMap(dataMap map[string]interface{}, fie if nextValue, exists := nestedMap[k]; exists { currentValue = nextValue } else { - return nil, errors.Errorf("cannot resolve groupsRef: %s not found", refKey) + return nil, errors.Errorf("cannot resolve groupsRef: %s %w", refKey, errRefNotFound) } } else { return nil, errors.Errorf("cannot resolve groupsRef: %s not a map", refKey) diff --git a/fn_test.go b/fn_test.go index debf402..9b8c120 100644 --- a/fn_test.go +++ b/fn_test.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "slices" "testing" "time" @@ -12,6 +13,7 @@ import ( "github.com/upbound/function-msgraph/input/v1beta1" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/durationpb" + "google.golang.org/protobuf/types/known/structpb" "k8s.io/utils/ptr" "github.com/crossplane/function-sdk-go/errors" @@ -43,6 +45,7 @@ const ( // Result messages emitted per query type. msgUserValidationQueryType = `QueryType: "UserValidation"` + msgGroupMembershipQueryType = `QueryType: "GroupMembership"` msgGroupObjectIDsQueryType = `QueryType: "GroupObjectIDs"` msgServicePrincipalDetailsQueryType = `QueryType: "ServicePrincipalDetails"` ) @@ -726,7 +729,7 @@ func TestResolveGroupRef(t *testing.T) { Results: []*fnv1.Result{ { Severity: fnv1.Severity_SEVERITY_NORMAL, - Message: `QueryType: "GroupMembership"`, + Message: msgGroupMembershipQueryType, Target: fnv1.Target_TARGET_COMPOSITE.Enum(), }, }, @@ -804,7 +807,7 @@ func TestResolveGroupRef(t *testing.T) { Results: []*fnv1.Result{ { Severity: fnv1.Severity_SEVERITY_NORMAL, - Message: `QueryType: "GroupMembership"`, + Message: msgGroupMembershipQueryType, Target: fnv1.Target_TARGET_COMPOSITE.Enum(), }, }, @@ -893,7 +896,7 @@ func TestResolveGroupRef(t *testing.T) { Results: []*fnv1.Result{ { Severity: fnv1.Severity_SEVERITY_NORMAL, - Message: `QueryType: "GroupMembership"`, + Message: msgGroupMembershipQueryType, Target: fnv1.Target_TARGET_COMPOSITE.Enum(), }, }, @@ -2656,7 +2659,7 @@ func TestRunFunction(t *testing.T) { Results: []*fnv1.Result{ { Severity: fnv1.Severity_SEVERITY_NORMAL, - Message: `QueryType: "GroupMembership"`, + Message: msgGroupMembershipQueryType, Target: fnv1.Target_TARGET_COMPOSITE.Enum(), }, }, @@ -5117,3 +5120,439 @@ func TestNormalizeGraphValue(t *testing.T) { }) } } + +// TestSkipQueryWhenRefMissing tests that skipQueryWhenRefMissing turns an +// unresolvable reference into a non-fatal skip, and nothing else. +func TestSkipQueryWhenRefMissing(t *testing.T) { + creds := &fnv1.CredentialData{ + Data: map[string][]byte{ + testCredentialsKey: []byte(`{"clientId":"c","clientSecret":"s","tenantId":"t"}`), + }, + } + credentials := map[string]*fnv1.Credentials{ + testAzureCredsName: {Source: &fnv1.Credentials_CredentialData{CredentialData: creds}}, + } + meta := &fnv1.ResponseMeta{Tag: testRequestTag, Ttl: durationpb.New(response.DefaultTTL)} + successCond := &fnv1.Condition{ + Type: condTypeFunctionSuccess, + Status: fnv1.Status_STATUS_CONDITION_TRUE, + Reason: condReasonSuccess, + Target: fnv1.Target_TARGET_COMPOSITE_AND_CLAIM.Enum(), + } + skipCond := func(msg string) *fnv1.Condition { + return &fnv1.Condition{ + Type: "FunctionSkip", + Status: fnv1.Status_STATUS_CONDITION_TRUE, + Reason: "RefNotResolved", + Message: ptr.To(msg), + Target: fnv1.Target_TARGET_COMPOSITE_AND_CLAIM.Enum(), + } + } + warning := func(msg string) *fnv1.Result { + return &fnv1.Result{ + Severity: fnv1.Severity_SEVERITY_WARNING, + Message: msg, + Target: fnv1.Target_TARGET_COMPOSITE_AND_CLAIM.Enum(), + } + } + fatal := func(msg string) *fnv1.Result { + return &fnv1.Result{ + Severity: fnv1.Severity_SEVERITY_FATAL, + Message: msg, + Target: fnv1.Target_TARGET_COMPOSITE.Enum(), + } + } + + // A brand-new XR: the referenced status field has not been written yet, + // but other status fields already exist and must be preserved. + newXR := `{ + "apiVersion": "example.org/v1", + "kind": "XR", + "metadata": {"name": "cool-xr"}, + "spec": {"count": 2}, + "status": {"existing": "keep-me"} + }` + input := func(fields string) *structpb.Struct { + return resource.MustStructJSON(`{"apiVersion": "msgraph.fn.crossplane.io/v1alpha1", "kind": "Input", ` + fields + `}`) + } + xrState := func(xr string) *fnv1.State { + return &fnv1.State{Composite: &fnv1.Resource{Resource: resource.MustStructJSON(xr)}} + } + skipped := func(refType, refKey string) *fnv1.RunFunctionResponse { + return &fnv1.RunFunctionResponse{ + Meta: meta, + Conditions: []*fnv1.Condition{ + skipCond(fmt.Sprintf("Query skipped: %s %s could not be resolved yet", refType, refKey)), + successCond, + }, + Results: []*fnv1.Result{ + warning(fmt.Sprintf("cannot resolve %s: %s not found; skipping query (skipQueryWhenRefMissing)", refType, refKey)), + }, + Desired: xrState(newXR), + } + } + failed := func(msg string) *fnv1.RunFunctionResponse { + return &fnv1.RunFunctionResponse{ + Meta: meta, + Results: []*fnv1.Result{fatal(msg)}, + Desired: xrState(newXR), + } + } + + cases := map[string]struct { + reason string + req *fnv1.RunFunctionRequest + want *fnv1.RunFunctionResponse + wantGraphCalls int + }{ + "GroupRefStatusMissingSkips": { + reason: "With the flag set, a missing groupRef status path should skip the query with a warning", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.groups.owners.name", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: skipped("groupRef", "status.groups.owners.name"), + }, + "GroupRefStatusMissingSkipsWithoutRecordingTimestamp": { + reason: "A skipped query must not record a lastQueryTimestamps entry", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.groups.owners.name", "target": "status.groupMembers", "queryInterval": "10m", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: skipped("groupRef", "status.groups.owners.name"), + }, + "GroupRefSpecMissingSkips": { + reason: "With the flag set, a missing groupRef spec path should skip the query with a warning", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "spec.groupName", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: skipped("groupRef", "spec.groupName"), + }, + "GroupRefContextMissingSkips": { + reason: "With the flag set, a missing groupRef context path should skip the query and preserve the context", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "context.groups.owners", "target": "context.groupMembers", "skipQueryWhenRefMissing": true`), + Context: resource.MustStructJSON(`{"other": "value"}`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: func() *fnv1.RunFunctionResponse { + rsp := skipped("groupRef", "context.groups.owners") + rsp.Context = resource.MustStructJSON(`{"other": "value"}`) + return rsp + }(), + }, + "GroupRefMissingWithoutFlagIsFatal": { + reason: "Without the flag, a missing groupRef path should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.groups.owners.name", "target": "status.groupMembers"`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve groupRef: status.groups.owners.name not found"), + }, + "GroupRefMissingWithFlagFalseIsFatal": { + reason: "With the flag explicitly false, a missing groupRef path should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.groups.owners.name", "target": "status.groupMembers", "skipQueryWhenRefMissing": false`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve groupRef: status.groups.owners.name not found"), + }, + "GroupRefNonStringValueIsFatal": { + reason: "With the flag set, a groupRef path holding a non-string value should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "spec.count", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve groupRef: spec.count not found"), + }, + "GroupRefPathThroughNonMapIsFatal": { + reason: "With the flag set, a groupRef path that traverses a non-map value should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.existing.name", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve groupRef: status.existing.name not found"), + }, + "GroupRefUnsupportedFormatIsFatal": { + reason: "With the flag set, an unsupported groupRef format is a config error and should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "metadata.name", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("unsupported groupRef format: metadata.name"), + }, + "GroupRefEmptyIsNotSkipped": { + reason: "With the flag set, an empty groupRef is ignored as today and the query fails without a group", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("no names provided"), + wantGraphCalls: 1, + }, + "GroupRefResolvesWithFlag": { + reason: "With the flag set, a resolvable groupRef should run the query and write the result as usual", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.groups.owners.name", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Observed: xrState(`{ + "apiVersion": "example.org/v1", "kind": "XR", "metadata": {"name": "cool-xr"}, "spec": {"count": 2}, + "status": {"groups": {"owners": {"name": "Owners"}}} + }`), + Credentials: credentials, + }, + want: &fnv1.RunFunctionResponse{ + Meta: meta, + Conditions: []*fnv1.Condition{successCond}, + Results: []*fnv1.Result{{ + Severity: fnv1.Severity_SEVERITY_NORMAL, + Message: msgGroupMembershipQueryType, + Target: fnv1.Target_TARGET_COMPOSITE.Enum(), + }}, + Desired: xrState(`{ + "apiVersion": "example.org/v1", "kind": "XR", "metadata": {"name": "cool-xr"}, "spec": {"count": 2}, + "status": {"groups": {"owners": {"name": "Owners"}}, "groupMembers": ["Owners"]} + }`), + }, + wantGraphCalls: 1, + }, + "GroupsRefStatusMissingSkips": { + reason: "With the flag set, a missing groupsRef path should skip the query with a warning", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupObjectIDs", "groupsRef": "status.groupNames", "target": "status.groupObjectIDs", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: skipped("groupsRef", "status.groupNames"), + }, + "UsersRefSpecMissingSkips": { + reason: "With the flag set, a missing usersRef path should skip the query with a warning naming usersRef", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "UserValidation", "usersRef": "spec.users", "target": "status.validatedUsers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: skipped("usersRef", "spec.users"), + }, + "ServicePrincipalsRefContextMissingSkips": { + reason: "With the flag set, a missing servicePrincipalsRef path should skip the query with a warning naming servicePrincipalsRef", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "ServicePrincipalDetails", "servicePrincipalsRef": "context.sps", "target": "status.servicePrincipals", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: skipped("servicePrincipalsRef", "context.sps"), + }, + "GroupsRefMissingWithoutFlagIsFatal": { + reason: "Without the flag, a missing groupsRef path should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupObjectIDs", "groupsRef": "status.groupNames", "target": "status.groupObjectIDs"`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve groupsRef: status.groupNames not found"), + }, + "UsersRefMissingWithoutFlagIsFatal": { + reason: "Without the flag, a missing usersRef path should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "UserValidation", "usersRef": "spec.users", "target": "status.validatedUsers", "skipQueryWhenRefMissing": false`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve usersRef: spec.users not found"), + }, + "ServicePrincipalsRefMissingWithoutFlagIsFatal": { + reason: "Without the flag, a missing servicePrincipalsRef path should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "ServicePrincipalDetails", "servicePrincipalsRef": "context.sps", "target": "status.servicePrincipals"`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve servicePrincipalsRef: context.sps not found"), + }, + "GroupsRefNotStringArrayIsFatal": { + reason: "With the flag set, a groupsRef path holding a non-array value should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupObjectIDs", "groupsRef": "status.existing", "target": "status.groupObjectIDs", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve groupsRef: status.existing not a string array"), + }, + "UsersRefNotAMapIsFatal": { + reason: "With the flag set, a usersRef path that traverses a non-map value should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "UserValidation", "usersRef": "status.existing.list", "target": "status.validatedUsers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve usersRef: status.existing.list not a map"), + }, + "ServicePrincipalsRefNotStringArrayIsFatal": { + reason: "With the flag set, a servicePrincipalsRef path holding a non-array value should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "ServicePrincipalDetails", "servicePrincipalsRef": "spec.count", "target": "status.servicePrincipals", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("cannot resolve servicePrincipalsRef: spec.count not a string array"), + }, + "GroupsRefUnsupportedFormatIsFatal": { + reason: "With the flag set, an unsupported groupsRef format is a config error and should stay fatal", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupObjectIDs", "groupsRef": "groupNames", "target": "status.groupObjectIDs", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("unsupported groupsRef format: groupNames"), + }, + "UsersRefEmptyIsNotSkipped": { + reason: "With the flag set, an empty usersRef is ignored as today and the query fails without users", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "UserValidation", "usersRef": "", "target": "status.validatedUsers", "skipQueryWhenRefMissing": true`), + Observed: xrState(newXR), + Credentials: credentials, + }, + want: failed("no names provided"), + wantGraphCalls: 1, + }, + "ArrayRefsResolveWithFlag": { + reason: "With the flag set, a resolvable usersRef should run the query and write the result as usual", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "UserValidation", "usersRef": "spec.users", "target": "status.validatedUsers", "skipQueryWhenRefMissing": true`), + Observed: xrState(`{ + "apiVersion": "example.org/v1", "kind": "XR", "metadata": {"name": "cool-xr"}, + "spec": {"users": ["a@example.com", "b@example.com"]} + }`), + Credentials: credentials, + }, + want: &fnv1.RunFunctionResponse{ + Meta: meta, + Conditions: []*fnv1.Condition{successCond}, + Results: []*fnv1.Result{{ + Severity: fnv1.Severity_SEVERITY_NORMAL, + Message: msgUserValidationQueryType, + Target: fnv1.Target_TARGET_COMPOSITE.Enum(), + }}, + Desired: xrState(`{ + "apiVersion": "example.org/v1", "kind": "XR", "metadata": {"name": "cool-xr"}, + "spec": {"users": ["a@example.com", "b@example.com"]}, + "status": {"validatedUsers": ["a@example.com", "b@example.com"]} + }`), + }, + wantGraphCalls: 1, + }, + "OperationGroupRefMissingSkipsWithoutAnnotations": { + reason: "In Operation mode, a skipped query must not fail or update the drift annotations", + req: &fnv1.RunFunctionRequest{ + Meta: &fnv1.RequestMeta{Tag: testRequestTag}, + Input: input(`"queryType": "GroupMembership", "groupRef": "status.groups.owners.name", "target": "status.groupMembers", "skipQueryWhenRefMissing": true`), + Credentials: credentials, + RequiredResources: map[string]*fnv1.Resources{ + watchedResourceKey: {Items: []*fnv1.Resource{{Resource: resource.MustStructJSON(`{ + "apiVersion": "example.org/v1", "kind": "XR", + "metadata": { + "name": "cool-xr", + "finalizers": ["composite.apiextensions.crossplane.io"], + "annotations": {"function-msgraph/last-execution-query-drift-detected": "true"} + }, + "spec": {"count": 2}, + "status": {"existing": "keep-me"} + }`)}}}, + }, + }, + want: &fnv1.RunFunctionResponse{ + Meta: meta, + Conditions: []*fnv1.Condition{ + skipCond("Query skipped: groupRef status.groups.owners.name could not be resolved yet"), + successCond, + }, + Results: []*fnv1.Result{ + warning("cannot resolve groupRef: status.groups.owners.name not found; skipping query (skipQueryWhenRefMissing)"), + }, + Desired: xrState(`{ + "apiVersion": "example.org/v1", "kind": "XR", + "metadata": { + "name": "cool-xr", + "annotations": {"function-msgraph/last-execution-query-drift-detected": "true"} + } + }`), + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + graphCalls := 0 + mockQuery := &MockGraphQuery{ + // Echoes the resolved names so tests can see what the ref resolved to. + GraphQueryFunc: func(_ context.Context, _ map[string]string, in *v1beta1.Input) (interface{}, error) { + graphCalls++ + names := slices.Concat(in.Groups, in.Users, in.ServicePrincipals) + if in.Group != nil && *in.Group != "" { + names = append(names, in.Group) + } + if len(names) == 0 { + return nil, errors.New("no names provided") + } + results := make([]interface{}, 0, len(names)) + for _, n := range names { + results = append(results, *n) + } + return results, nil + }, + } + + f := &Function{ + graphQuery: mockQuery, + timer: &MockTimer{}, + log: logging.NewNopLogger(), + } + rsp, err := f.RunFunction(context.Background(), tc.req) + if err != nil { + t.Fatalf("%s\nf.RunFunction(...): unexpected error: %v", tc.reason, err) + } + + if diff := cmp.Diff(tc.want, rsp, protocmp.Transform()); diff != "" { + t.Errorf("%s\nf.RunFunction(...): -want rsp, +got rsp:\n%s", tc.reason, diff) + } + + if graphCalls != tc.wantGraphCalls { + t.Errorf("%s\nMicrosoft Graph calls: want %d, got %d", tc.reason, tc.wantGraphCalls, graphCalls) + } + }) + } +} diff --git a/input/v1beta1/input.go b/input/v1beta1/input.go index bb19a2a..8cd7c9a 100644 --- a/input/v1beta1/input.go +++ b/input/v1beta1/input.go @@ -67,6 +67,14 @@ type Input struct { // +optional SkipQueryWhenTargetHasData *bool `json:"skipQueryWhenTargetHasData,omitempty"` + // SkipQueryWhenRefMissing skips the query instead of failing when a reference + // (groupRef, groupsRef, usersRef, servicePrincipalsRef) points at a field that + // does not exist yet. The target is left untouched. Useful when the referenced + // field is populated later in the pipeline or on a later reconcile. + // Default is false (unresolvable references are fatal). + // +optional + SkipQueryWhenRefMissing *bool `json:"skipQueryWhenRefMissing,omitempty"` + // QueryInterval is an optional minimum interval between queries, expressed as // a Go duration string (for example "10m", "1h" or "90s"). When set, the // function records the timestamp of its last successful query alongside the diff --git a/input/v1beta1/zz_generated.deepcopy.go b/input/v1beta1/zz_generated.deepcopy.go index 8631069..4f311eb 100644 --- a/input/v1beta1/zz_generated.deepcopy.go +++ b/input/v1beta1/zz_generated.deepcopy.go @@ -91,6 +91,11 @@ func (in *Input) DeepCopyInto(out *Input) { *out = new(bool) **out = **in } + if in.SkipQueryWhenRefMissing != nil { + in, out := &in.SkipQueryWhenRefMissing, &out.SkipQueryWhenRefMissing + *out = new(bool) + **out = **in + } if in.QueryInterval != nil { in, out := &in.QueryInterval, &out.QueryInterval *out = new(string) diff --git a/package/input/msgraph.fn.crossplane.io_inputs.yaml b/package/input/msgraph.fn.crossplane.io_inputs.yaml index 9301ada..375d275 100644 --- a/package/input/msgraph.fn.crossplane.io_inputs.yaml +++ b/package/input/msgraph.fn.crossplane.io_inputs.yaml @@ -122,6 +122,14 @@ spec: ServicePrincipalsRef is a reference to retrieve the service principal names (e.g., from status or context) Overrides ServicePrincipals field if used type: string + skipQueryWhenRefMissing: + description: |- + SkipQueryWhenRefMissing skips the query instead of failing when a reference + (groupRef, groupsRef, usersRef, servicePrincipalsRef) points at a field that + does not exist yet. The target is left untouched. Useful when the referenced + field is populated later in the pipeline or on a later reconcile. + Default is false (unresolvable references are fatal). + type: boolean skipQueryWhenTargetHasData: description: |- SkipQueryWhenTargetHasData controls whether to skip the query when the target already has data