Skip to content

add _lastModifiedTime field to secret snapshots#810

Open
EldarShalev wants to merge 4 commits into
jetstack:masterfrom
EldarShalev:feature/secret-last-modified-time
Open

add _lastModifiedTime field to secret snapshots#810
EldarShalev wants to merge 4 commits into
jetstack:masterfrom
EldarShalev:feature/secret-last-modified-time

Conversation

@EldarShalev
Copy link
Copy Markdown

Extract the most recent time from metadata.managedFields and include it as a synthetic _lastModifiedTime field on Secret objects sent to the backend. This enables detection of stale or unrotated secrets.

eshalev and others added 2 commits May 26, 2026 16:35
Extract the most recent time from metadata.managedFields and include it
as a synthetic _lastModifiedTime field on Secret objects sent to the
backend. This enables detection of stale or unrotated secrets.
Comment thread pkg/datagatherer/k8sdynamic/dynamic.go Outdated
}
}

setLastModifiedTime(resource)
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.

suggestion: this unilaterally sets the last modified time in all cases. This repo contains 3 agents which we release, and this change is only for the disco-agent and not the others. I think we'd need to change this PR so it only affects the disco agent.

Comment thread pkg/datagatherer/k8sdynamic/dynamic.go Outdated
Comment on lines +643 to +645
if timeVal > latestTime {
latestTime = timeVal
}
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.

suggestion: treating the values as strings means that Go will compare lexicographically, not by the times represented.

It should be possible to write a test case which detects the wrong latestTime based on this comparison. You'll have to parse the timestamps into time.Time values to compare them safely + properly.

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