Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matthyx merged 2 commits into from
Oct 1, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42109c0
fix(instanceidhandler): exclude transient pod fields in CronJob templ…
Sep 30, 2026
8e8d553
fix(instanceidhandler): deep copy pod spec before hashing to preserve…
Sep 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions instanceidhandler/v1/initializers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func GenerateInstanceID(w workloadinterface.IWorkload, jsonPaths []string) ([]in
// if the Pod is created by a CronJob, its parent is a Job named after the CronJob
// with the scheduled timestamp appended to it (unix time in minutes).
// https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/cronjob/utils.go#L277
// TODO add a json path to exclude some fields
s := strings.Split(ownerReference.Name, "-")
if len(s) > 1 && isUnixTimeInMinutes(s[len(s)-1]) {
// calculate pod template hash
Expand Down Expand Up @@ -110,13 +109,23 @@ func GenerateInstanceID(w workloadinterface.IWorkload, jsonPaths []string) ([]in
return convertContainersToIInstanceID(c), nil
}

var defaultExcludedJSONPaths = []string{
".hostname",
".nodeName",
".containers[*].env[?(@.name==\"DD_INJECT_START_TIME\")]",
".containers[*].env[?(@.name==\"DD_INSTRUMENTATION_INSTALL_ID\")]",

@matthyx matthyx Sep 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 shellyco-code Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

".initContainers[*].env[?(@.name==\"DD_INJECT_START_TIME\")]",
".initContainers[*].env[?(@.name==\"DD_INSTRUMENTATION_INSTALL_ID\")]",
}

func DeepHashObject(hasher hash.Hash32, pod *corev1.PodSpec, extraJsonPaths []string) {
// sanitize pod sped
dropProjectedVolumesAndMounts(pod)
jsonPaths := []string{
".hostname",
".containers[*].env[?(@.name==\"DD_INJECT_START_TIME\")]",
if pod == nil {
return
}
pod = pod.DeepCopy()
// sanitize pod spec
dropProjectedVolumesAndMounts(pod)
jsonPaths := append([]string{}, defaultExcludedJSONPaths...)
jsonPaths = append(jsonPaths, extraJsonPaths...)
if err := dropFieldsByJSONPath(pod, jsonPaths); err != nil {
logger.L().Error("failed to drop fields by JSONPath", helpers.Error(err), helpers.Interface("jsonPath", jsonPaths))
Expand Down
1 change: 0 additions & 1 deletion instanceidhandler/v1/initializers2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ func GenerateInstanceIDFromRuntimeObj(w runtime.Object, jsonPaths []string) ([]i
// if the Pod is created by a CronJob, its parent is a Job named after the CronJob
// with the scheduled timestamp appended to it (unix time in minutes).
// https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/cronjob/utils.go#L277
// TODO add a json path to exclude some fields
s := strings.Split(ownerReference.Name, "-")
if len(s) > 1 && isUnixTimeInMinutes(s[len(s)-1]) {
// calculate pod template hash
Expand Down
49 changes: 49 additions & 0 deletions instanceidhandler/v1/initializers2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/kubescape/k8s-interface/instanceidhandler"
"github.com/kubescape/k8s-interface/instanceidhandler/v1/containerinstance"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -298,3 +299,51 @@ func TestGenerateInstanceIDFromRuntime2(t *testing.T) {
})
}
}

func TestGenerateInstanceIDFromRuntimeObj_PreservesInput(t *testing.T) {
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "hello-28686821-dj5bf",
Namespace: "default",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "batch/v1",
Kind: "Job",
Name: "hello-28686821",
},
},
},
Spec: v1.PodSpec{
NodeName: "kind-control-plane",
Hostname: "test-hostname",
Containers: []v1.Container{
{
Name: "hello",
Env: []v1.EnvVar{
{Name: "DD_INSTRUMENTATION_INSTALL_ID", Value: "0ca6045d-9232-4ad0-bdf0-12330094bcc3"},
{Name: "DD_INJECT_START_TIME", Value: "1746527100"},
{Name: "CUSTOM_VAR", Value: "custom-value"},
},
},
},
InitContainers: []v1.Container{
{
Name: "init-hello",
Env: []v1.EnvVar{
{Name: "DD_INSTRUMENTATION_INSTALL_ID", Value: "init-id-123"},
{Name: "DD_INJECT_START_TIME", Value: "1746527200"},
},
},
},
},
}

podCopy := pod.DeepCopy()

ids, err := GenerateInstanceIDFromRuntimeObj(pod, nil)
require.NoError(t, err)
require.NotEmpty(t, ids)

// Verify that the caller-owned runtime object was not mutated by hashing or sanitization
assert.Equal(t, podCopy, pod, "GenerateInstanceIDFromRuntimeObj must not mutate the input runtime.Object")
}
52 changes: 44 additions & 8 deletions instanceidhandler/v1/initializers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
_ "embed"
"encoding/json"
"fmt"
"hash/fnv"
"testing"

"github.com/kubescape/k8s-interface/instanceidhandler"
Expand Down Expand Up @@ -142,11 +143,11 @@ func TestGenerateInstanceID(t *testing.T) {
Name: "kubevuln-scheduler-28677846",
ContainerName: "kubevuln-scheduler",
InstanceType: Container,
AlternateName: "kubevuln-scheduler-5976555c87",
TemplateHash: "5976555c87",
AlternateName: "kubevuln-scheduler-5f99858564",
TemplateHash: "5f99858564",
},
},
wantSlug: "job-kubevuln-scheduler-5976555c87",
wantSlug: "job-kubevuln-scheduler-5f99858564",
wantErr: assert.NoError,
},
{
Expand Down Expand Up @@ -272,11 +273,11 @@ func TestGenerateInstanceIDFromRuntime(t *testing.T) {
Name: "kubevuln-scheduler-28677846",
ContainerName: "kubevuln-scheduler",
InstanceType: Container,
AlternateName: "kubevuln-scheduler-5976555c87",
TemplateHash: "5976555c87",
AlternateName: "kubevuln-scheduler-5f99858564",
TemplateHash: "5f99858564",
},
},
wantSlug: "job-kubevuln-scheduler-5976555c87",
wantSlug: "job-kubevuln-scheduler-5f99858564",
wantErr: assert.NoError,
},
{
Expand Down Expand Up @@ -325,10 +326,10 @@ func TestSameSlug(t *testing.T) {
require.NoError(t, err)
slug, err := ins[0].(*containerinstance.InstanceID).GetSlug(true)
require.NoError(t, err)
assert.Equal(t, "job-hello-64c875c864", slug)
assert.Equal(t, "job-hello-77bdd46fc5", slug)
slugFull, err := ins[0].(*containerinstance.InstanceID).GetSlug(false)
require.NoError(t, err)
assert.Equal(t, "job-hello-64c875c864-hello-f699-a8be", slugFull)
assert.Equal(t, "job-hello-77bdd46fc5-hello-6f4e-3f75", slugFull)
}
}

Expand Down Expand Up @@ -367,3 +368,38 @@ func compare(t *testing.T, a, b *containerinstance.InstanceID) {
assert.Equal(t, a.Name, b.Name)
assert.Equal(t, a.ContainerName, b.ContainerName)
}

func TestDeepHashObject_ExcludedFields(t *testing.T) {
pod1 := &corev1.Pod{}
err := json.Unmarshal([]byte(cronjob), pod1)
require.NoError(t, err)

pod2 := &corev1.Pod{}
err = json.Unmarshal([]byte(cronjob), pod2)
require.NoError(t, err)

// Simulate pod scheduled to a different node with different dynamic/injected env vars
pod1.Spec.NodeName = "node-alpha"
pod1.Spec.Hostname = "host-alpha"

pod2.Spec.NodeName = "node-beta"
pod2.Spec.Hostname = "host-beta"
pod2.Spec.Containers[0].Env[0].Value = "different-uuid-value"
pod2.Spec.Containers[0].Env[1].Value = "9999999999"

// Also verify initContainers with transient Datadog env vars
initEnv1 := []corev1.EnvVar{{Name: "DD_INSTRUMENTATION_INSTALL_ID", Value: "init-id-1"}}
initEnv2 := []corev1.EnvVar{{Name: "DD_INSTRUMENTATION_INSTALL_ID", Value: "init-id-2"}}
pod1.Spec.InitContainers = []corev1.Container{{Name: "init-c", Env: initEnv1}}
pod2.Spec.InitContainers = []corev1.Container{{Name: "init-c", Env: initEnv2}}

hasher1 := fnv.New32a()
DeepHashObject(hasher1, &pod1.Spec, nil)
hash1 := hasher1.Sum32()

hasher2 := fnv.New32a()
DeepHashObject(hasher2, &pod2.Spec, nil)
hash2 := hasher2.Sum32()

assert.Equal(t, hash1, hash2, "hashes must match even when nodeName or Datadog inject env vars differ")
}
Loading