feat(server): migrate EventList to google.protobuf.Struct for K8s 1.3.5 compatibility (#25767)

[chansuke]

Closes #25767

Change the return type of ListResourceEvents and ListEvents gRPC APIs from k8s.io.api.core.v1.EventList to google.protobuf.Struct to avoid protobuf compatibility issues with Kubernetes 1.35+.

Affected endpoints

• application.ListResourceEvents
• applicationset.ListResourceEvents
• project.ListEvents

Breaking Changes

• gRPC clients must handle google.protobuf.Struct instead of EventList.
• REST API responses remain unchanged.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• The title of the PR conforms to the Title of the PR
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[bunnyshell]

❌ Preview Environment undeployed from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[ppapapetrou76]

I have a few questions/comments before I dive into more details

1. Why the implementation is not following the suggestion with the wrapped struct here
2. from a design point of view having GRPC and Rest returning different structs for the same call is not ideal
3. Following question 1 the code now is moving away from a strongly-typed EventList to an unstructured google.protobuf.Struct. This removes compile-time type safety and additionally the grpc clients now need to parse the returned struct, know/assume the expected structure...
4. I'm also skeptical about performance especially if the event list is quite big because the code now is taking several steps to transform it.

[chansuke]

Thank you for your comment.

1. As noted in upgrade to K8s 1.35 (K8s Go client 0.35) #25767 (comment), wrapping K8s types directly doesn't work due to protobuf library incompatibility (gogo/protobuf vs google.golang.org/protobuf). Following the Istio approach, I used google.protobuf.Struct directly to avoid this issue.
2. gRPC and REST already return the same content and grpc-gateway converts google.protobuf.Struct to JSON automatically. A wrapped struct would make the gRPC API more explicit but doesn't change the underlying data.
3. I can wrap the Struct in a dedicated message type (e.g., message ResourceEventsResponse { google.protobuf.Struct events = 1; }). This won't restore compile-time type safety, but it gives gRPC clients a known　message type rather than a raw Struct.
4. I'll use protojson.Unmarshal to eliminate the intermediate map[string]interface{} step.

[reggie-k]

Thanks for the PR!
We should be minded to how we manage the breaking change this introduces, attaching a PR that dealt with a different breaking change in the past, it may be relevant as a strategy:
#23695

[chansuke]

@reggie-k
Thank you for the reference! I looked into #23695 and the V2 endpoint approach makes sense for maintaining backwards compatibility.
However, the current EventList return type will break anyway when upgrading to K8s 1.35 so, I'm considering the below approach:

1. V2 endpoint: Add a new endpoint returning Struct, deprecate the old one, remove it in a future releease
2. Current approach: Change the existing endpoint with clear documentation in the upgrade guide

[codecov]

Codecov Report

❌ Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.09%. Comparing base (63a009e) to head (d279127).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
server/events/events.go	69.23%	2 Missing and 2 partials ⚠️
server/project/project.go	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #26322      +/-   ##
==========================================
+ Coverage   63.04%   63.09%   +0.04%     
==========================================
  Files         414      415       +1     
  Lines       56286    56302      +16     
==========================================
+ Hits        35484    35521      +37     
+ Misses      17427    17410      -17     
+ Partials     3375     3371       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ppapapetrou76]

However, the current EventList return type will break anyway when upgrading to K8s 1.35 so, I'm considering the below approach:

This is not entirely true. There will be a breaking change when we upgrade the client to 1.35.
ArgoCD will work fine with K8s 1.35 server

[chansuke]

Thanks, i understood now. the immediate break here is the gRPC API signature change and not k8s server compatibility itself.

i’ll follow the #23695 strategy:

• keep existing List*Events methods unchanged,
• add V2 methods returning Struct (or a wrapper message),
• deprecate V1 with migration notes,
• migrate internal callers to V2,
• remove V1 in a future major release.

[crenshaw-dev]

keep existing List*Events methods unchanged

How is that possible? My understanding is that the problem is using the upstream k8s structs that are no longer compatible with protobuf. So if we upgrade the libs to 0.35, won't these "v1" endpoints immediately break anyway?

[reggie-k]

@chansuke I apologize, looks like I have provided a misleading guidance with regard to how we should manage this breaking change earlier.

We discussed this with @crenshaw-dev and on the weekly contributors meeting on Thursday, and the latest consensus is that we don't have a good alternative but breaking our API following the breaking change in K8s.

It means that we will not go in the direction of maintaining 2 versions of endpoints and should instead document the breaking change and it's implications clearly (which methods break for gRPC, in what way and possible workarounds, like running the CLI with --grpc-web etc). We also need to document that for this version, a matching CLI version needs to be used, so an upgrade to this version will require using upgraded CLI (older CLI will not work with this version).
All this is based on the assumption that our REST API wouldn't break with this change, but just gRPC API, which is used mostly internally so has a smaller impact radius, plus we assume that listing events is not a largely used API operation by users.
Lemme know if you think this change can make it into 3.4 and if you need any help with the implementation/docs/testing etc (RC comes out at March 16).

If not feasible for 3.4, we will aim for 3.5 instead.

[chansuke]

@reggie-k
Thanks, that makes sense.
I’ll rework the PR in that direction. I’ll drop the dual-endpoint approach, keep the gRPC breaking change, and make sure the impact is clearly documented in the 3.4 upgrade notes. I’ll also remove the local /v2 compatibility work.
Given the timing, I think this still has a chance to make 3.4 since this is actually simpler than carrying the compatibility layer. I’ll focus on docs and validation next, and I’ll raise it quickly if anything looks too risky for the RC.

[reggie-k]

Thanks for the updates, looks like a couple of changes are unintended and may be a result of git merge?

[reggie-k]

This section looks removed due to possibly a local git merge?

[chansuke]

fixed now.

[reggie-k]

It still shows as deleted

[dudinea]

Since now we return a general structure instead of v1EventList it would be great to add here comment on the expected response structure

[chansuke]

Thanks for the feedback. I agree this would be useful. My idea is to add swagger annotations to the .proto files. I’ll create a separate follow-up PR for that