Repository navigation
Conversation
Signed-off-by: Zan Ozbot <zan.ozbot@gmail.com>
blacksoxx
left a comment
There was a problem hiding this comment.
Pulled the branch and ran it with Go 1.26.10, the version its go.mod pins: go vet ./... and go test ./... pass, including the 24 cases of the new table test, and go generate ./input/... leaves the tree unchanged, so the packaged input schema matches the type. The FunctionSkip/RefNotResolved condition follows the existing SkippedQuery and IntervalLimit reasons, and keeping everything other than a missing path fatal (wrong type, unsupported ref format, Graph errors) is the right boundary for a flag like this.
One optional thought: the re-wrap in resolveStringArrayRef works because every "not found" message ends with the sentinel's text. Threading refType into extractStringArrayFromMap instead of rewriting groupsRef in the message would remove both the string replacement and the re-wrap, but that pattern predates this PR and could be a follow-up. From my side this looks good; the call is the maintainers'.
Description of your changes
Adds an optional
skipQueryWhenRefMissinginput. When set, agroupRef,groupsRef,usersReforservicePrincipalsRefwhose path does not exist yet skips the query instead of returning a fatal result.Why: a fatal result aborts the whole Composition pipeline. When a ref points at an XR status field that a later pipeline step writes, for example the name of an Entra ID group created further down the pipeline, a new XR can never resolve the ref. Nothing is applied, the group is never created, and the XR is stuck.
Behaviour with the flag set and a missing ref path:
lastQueryTimestampsare not touched.cannot resolve groupRef: <ref> not found; skipping query (skipQueryWhenRefMissing).FunctionSkip=True/RefNotResolvedandFunctionSuccess=True/Successare set, the same way the existingSkippedQueryandIntervalLimitskips report.Still fatal with the flag set:
not a map,not a string array, a non-string groupRef)group not foundWithout the flag, behaviour and error messages are unchanged.
Added new tests to test the behaviour of the new field and updated README.
I have: