Skip to content

feat: add skipQueryWhenRefMissing input - #128

Open
zanozbot wants to merge 1 commit into
upbound:mainfrom
zanozbot:main
Open

zanozbot wants to merge 1 commit into
upbound:mainfrom
zanozbot:main

Conversation

@zanozbot

Copy link
Copy Markdown

Description of your changes

Adds an optional skipQueryWhenRefMissing input. When set, a groupRef, groupsRef, usersRef or servicePrincipalsRef whose 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.

queryType: GroupMembership
groupRef: status.groups.owners.name   # written by a later step
target: status.ownerMembers
skipQueryWhenRefMissing: true

Behaviour with the flag set and a missing ref path:

  • Microsoft Graph is not called. The target and lastQueryTimestamps are not touched.
  • The desired XR and context pass through unchanged.
  • A Warning result is emitted: cannot resolve groupRef: <ref> not found; skipping query (skipQueryWhenRefMissing).
  • FunctionSkip=True/RefNotResolved and FunctionSuccess=True/Success are set, the same way the existing SkippedQuery and IntervalLimit skips report.
  • In Operation mode, the drift annotations are not updated.

Still fatal with the flag set:

  • unsupported ref formats
  • paths that exist but hold the wrong type (not a map, not a string array, a non-string groupRef)
  • any Graph error, including group not found

Without the flag, behaviour and error messages are unchanged.

Added new tests to test the behaviour of the new field and updated README.

I have:

Signed-off-by: Zan Ozbot <zan.ozbot@gmail.com>

@blacksoxx blacksoxx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants