fix(instanceidhandler): exclude transient pod fields in CronJob template hash calculation

[shellyco-code]

Overview

This PR resolves an issue where transient runtime fields (.nodeName, container / initContainer APM injector variables) cause DeepHashObject in instanceidhandler/v1 to calculate divergent hashes for consecutive runs of the same CronJob.

Changes Made

• Defined defaultExcludedJSONPaths containing .hostname, .nodeName, and container/initContainer Datadog injection timestamps and install IDs (DD_INJECT_START_TIME, DD_INSTRUMENTATION_INSTALL_ID).
• Updated DeepHashObject to exclude default JSON paths while continuing to accept custom extraJsonPaths.
• Removed resolved TODO comments in initializers.go and initializers2.go.
• Added TestDeepHashObject_ExcludedFields in initializers_test.go to verify hash equality when transient fields differ.
• Updated test fixture hashes to match the sanitized pod spec.

How to Test

Run all unit tests:

go test -v ./instanceidhandler/v1/...
go test ./...

Related issues/PRs:
Fixes #175
Checklist before requesting a review
 My code follows the style guidelines of this project
 I have performed a self-review of my code
 I have added thorough tests
 New and existing unit tests pass locally with my changes

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c01ca63d-9a0e-4978-a222-f68ce08a6a1d

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[matthyx]

Request changes: the normalization is needed for #175, but the new filters introduce caller-visible input corruption (inline finding).

Reviewed head 42109c0 against main 444ee4d. Rechecked immediately before submission: open, non-draft, mergeable, same head/base, no existing human reviews or inline threads.

History: #102 introduced CronJob hashing; #132 added configurable JSONPath exclusions; #137 added the runtime-object path. All merged and already present on main; none covers these additional defaults. Paginated all-state PR titles/bodies and issue titles/bodies using CronJob, DeepHashObject, Datadog, JSONPath, template hash, transient/nodeName, instance ID and slug terms; inspected relevant discussions. Closed #75 discussed redundant slug tests and was retained for education, not a rejection of normalization. No duplicate/superseding fix or explicit rejection of this approach found. Search is limited to this repository and indexed discussion/API material; absence is not proof.

Validation in credential-free, network-isolated bubblewrap:

• go test ./instanceidhandler/v1/...: passes on reviewed head (original suite).
• go vet ./instanceidhandler/v1/..., changed-file gofmt and diff whitespace checks: pass.
• Reviewer-added go test ./instanceidhandler/v1 -run TestReview -count=1: node-only hash equality fails on main (998602395 vs 12512454), passes on head; runtime input preservation passes on main and fails on head, blanking both env entries.
• Latest PR CI run 36701415431 reports success for this head, including build/Basic-Test. License Compliance reports error, “2 issues found”; cause not established, so not classified as infrastructure or a code defect. Main requires one approval and has no required status contexts.

No new dependency or unrelated implementation changes. Public CronJob hash/slug changes are expected but should be documented for consumers coordinating persisted identifiers (nonblocking). New equality test omits init-container start time and negative controls; full local suite/race/lint and real admission-webhook integration were not run. Independent correctness lane requests changes; architecture lane reports WATCH for identifier transition/ownership concerns. No merge or source-branch modification performed.

[matthyx]

P2 / medium — preserve caller-owned env entries before hashing. For a CronJob child Pod with a Job owner such as cron-28677846, GenerateInstanceIDFromRuntimeObj(pod, nil) now replaces the original Pod's DD_INSTRUMENTATION_INSTALL_ID and init-container Datadog entries with empty EnvVar{}. GetPodSpecFromRuntimeObj returns only a shallow struct copy (initializers2.go:57), so the new filters write through shared slice storage via reflect.Set (initializers.go:184). Subsequent users of the same object see missing environment names/values; writing that object back also sends invalid empty-name env entries.

Confirmed with an isolated regression containing regular-container install ID and init-container start time: input-preservation assertion passes on current main and fails on this head, printing empty entries for both. Existing regular-container start-time mutation predates this PR; this finding concerns the newly affected fields. Deep-copy the PodSpec before sanitization and add a regression asserting the public runtime-object API leaves the supplied object unchanged.

[shellyco-code]

Great catch, thanks @matthyx! Updated DeepHashObject to deep-copy the PodSpec before sanitizing and added a regression test to make sure the input object stays untouched.

[matthyx]

Rechecked latest head 8e8d553 against main 444ee4d: approved. The deep copy before sanitization resolves the input-mutation blocker, and the new public-API regression covers regular and init-container env preservation. No remaining blockers found by independent correctness and architecture reviews.

Fresh validation in a credential-free, network-isolated bubblewrap environment:

• go test ./instanceidhandler/v1/... -count=1: passed, including the author regression and the original reviewer reproduction.
• go test ./instanceidhandler/v1/... -race -count=1: passed.
• go vet ./instanceidhandler/v1/..., changed-file gofmt, and diff whitespace checks: passed.
• CI 36763619265 confirms build/Basic-Test success for this head. License Compliance still reports error (“2 issues found”), with cause unverified; it is not a required status check. Main requires one approval and no status contexts.

Need and related history remain as established in the previous review (#175; merged #102/#132/#137; closed #75 is not a rejection of normalization). Refreshed all-state PR/issue history since that review found no new competing change. Identifier-transition documentation remains an optional suggestion. Full local repository suite, full lint and real webhook integration were not run.

Immediately before submission, the PR was open, non-draft and conflict-free with unchanged reviewed head/base and no new review concerns. No merge, push or source-branch changes.