Fix GitOps policy-software resolution to fall back to hash when URL lookup fails#42816
Fix GitOps policy-software resolution to fall back to hash when URL lookup fails#42816
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42816 +/- ##
==========================================
+ Coverage 66.81% 66.82% +0.01%
==========================================
Files 2541 2544 +3
Lines 203971 204241 +270
Branches 9278 9278
==========================================
+ Hits 136285 136491 +206
- Misses 55340 55400 +60
- Partials 12346 12350 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
iansltx
left a comment
iansltx
left a comment
There was a problem hiding this comment.
Minor feedback here, and this probably makes sense as an improvement, but I'm concerned that we never seem to have gotten a snapshot of the policy YAML that exhibited this bug, so this is a speculative fix. Might not stop us from merging + closing the bug because we're definitely fixing a cause of this particular error, particularly since the bug was filed about a month ago, but we've gotten in trouble with speculative/partial fixes before.
| // false if no identifier matched. The caller must ensure | ||
| // policy.InstallSoftware.Other is non-nil before calling. |
There was a problem hiding this comment.
Why can't we nil-check this and skip the cases where dereferencing Other would create a nil panic?
| continue | ||
| } else { | ||
| if !dryRun { | ||
| logFn("[!] could not resolve software title ID for policy (url=%q, app_store_id=%q, hash=%q, slug=%q)\n", |
There was a problem hiding this comment.
This feels like debug-level logging where most of these values will be blank. Since each policy handles at most one software install, seems like referring to the policy by its name/filename would allow for more quickly narrowing down the issue.
2 participants
Fixes #40841
Summary
The root cause of the URL mismatch described in the issue is unknown. We couldn't reproduce it and couldn't find a deterministic code path that explains it.
What we fix in this PR is a code defect that turns an unknown transient condition into a hard failure. When a policy has both a URL and a hash (which is always the case for
package_pathreferences), and the URL lookup fails for any reason, a continue statement prevented the hash-based fallback from ever running.