Unify cache keys, use field manager ownership for runtime cache, & add integration tests - #422
Merged
Conversation
byjustina
commented
May 29, 2026
byjustina
force-pushed
the
reconciler-tests
branch
from
June 1, 2026 20:05
940e947 to
54d1292
Compare
byjustina
force-pushed
the
reconciler-tests
branch
from
June 2, 2026 21:06
54d1292 to
1c0095b
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
byjustina
force-pushed
the
reconciler-tests
branch
from
June 2, 2026 21:39
75ce6fb to
a1b41c8
Compare
byjustina
changed the title
rlenglet
requested changes
Jun 5, 2026
byjustina
force-pushed
the
reconciler-tests
branch
from
June 9, 2026 18:19
2fe0e61 to
e0fdf30
Compare
byjustina
changed the title
byjustina
commented
Jun 9, 2026
byjustina
marked this pull request as ready for review
June 9, 2026 18:55
rigdomin-illumio
requested changes
Jun 16, 2026
rigdomin-illumio
left a comment
rigdomin-illumio
left a comment
Contributor
There was a problem hiding this comment.
Great work! Nice coverage.
There are some things that I'm a bit hesitant on and want to think further on, for now I left a few comments.
Contributor
|
Please update your PR with latest changes from main |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
pavankumarinnamuri
dismissed
their stale review
June 16, 2026 21:50
Reposting as inline comments per file/line — please disregard this consolidated version.
pavankumarinnamuri
previously requested changes
Jun 16, 2026
![semgrep-app[bot]](https://avatars.githubusercontent.com/in/60555?s=60&v=4){kind=small}
The merge of main (#452) into reconciler-tests kept the specField usages but dropped its declaration during conflict resolution in policy.go, breaking the build (undefined: specField). Re-add the constant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The checked-in generated code registered OneofWrappers against msgTypes[62]/[64], but the messages carrying those oneofs are at [63]/[65] — off by one because a message was added to the .proto but the .pb.go was never fully regenerated after the merge. At init the runtime attached oneof metadata to the wrong messages, so marshaling ConfiguredKubernetesObjectMutation hit a missing wrapper and paniced (fieldInfoForOneof nil deref / SIGSEGV) in envtest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rigdomin-illumio
self-requested a review
August 3, 2026 20:54
rigdomin-illumio
previously approved these changes
Aug 3, 2026
Add MinVersion: tls.VersionTLS13 to satisfy missing-ssl-minversion, and suppress the bypass-tls-verification finding with a reviewed nosemgrep comment: DialGRPC is test-only, dials the in-process fakeserver's self-signed cert, so cert verification is intentionally skipped and no MITM surface exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # go.mod # go.sum
Contributor
|
Need to update this PR with latest changes in Main. |
rigdomin-illumio
previously approved these changes
Aug 10, 2026
rigdomin-illumio
self-requested a review
August 10, 2026 21:49
rigdomin-illumio
previously approved these changes
Aug 10, 2026
byjustina
dismissed stale reviews from sungkyunchung, rlenglet, and pavankumarinnamuri
August 10, 2026 23:24
resolved
rigdomin-illumio
self-requested a review
August 11, 2026 04:46
rigdomin-illumio
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace server-assigned CloudSecure IDs with deterministic kind/namespace/name cache keys across both config and runtime caches. This ensures the reconciler can directly compare desired state (from CloudSecure) against observed state (from Kubernetes) without needing labels, which are subject to external controller changes.
Key changes:
kind/namespace/nameinstead of opaque server IDsmanagedFieldscheck for determining if a K8s object is operator-managed