Handle unimplemented Cloud Run v1 plugin handlers gracefully#6986
Handle unimplemented Cloud Run v1 plugin handlers gracefully#6986rootp1 wants to merge 2 commits into
Conversation
Signed-off-by: rootp1 <arnav.iitr@gmail.com>
|
@ffjlabo @khanhtc1202 @Warashi Please have a look, whenever you have a chance to |
|
AFAIK, Google Cloud Run Plugin has been planned to be developed in near future (maybe in GSoC or LFX Mentorship) , not now |
Hi, @armistcxy |
@khanhtc1202, we are planning to having it for Term 3 right 👀 |
|
for ref - #6114 |
|
Thanks for the context and for linking #6114. My intent with this PR was only to remove the current panic path in the unimplemented Cloud Run v1 handlers, not to define the Cloud Run plugin itself. If you would prefer to keep all Cloud Run-related changes under #6114 / the planned Term 3 work, I am fine to close this PR. If a small defensive fix is still useful before then, I can also re-scope the change accordingly. |
What this PR does:
Prevents the piped v1 Cloud Run plugin from panicking in its unimplemented deployment and livestate entry points by returning explicit errors instead, and adds regression tests for each handler.
Why we need it:
The plugin binary currently registers Cloud Run deployment and livestate handlers that still call
panic("implement me"), which can crash the plugin process when those exported methods are invoked.Which issue(s) this PR fixes:
Fixes #6985
Does this PR introduce a user-facing change?:
Summary
panic("implement me")stubs with explicit returned errorsLinked Issue
Fixes #6985
What Changed
deployment.Plugin.ExecuteStage,DetermineVersions, andDetermineStrategynow return explicit errorslivestate.Plugin.GetLivestatenow returns an explicit errorVerification
go test ./...(inpkg/app/pipedv1/plugin/cloudrun) — passedgo test -race ./...(inpkg/app/pipedv1/plugin/cloudrun) — passedmake build/go— passedmake build/plugin PLUGINS=cloudrun— passed./hack/ensure-dco.sh— passedmake check— not run successfully: failed in the requiredbuild/webstep because this environment does not haveyarn; later requiredlint/goandgen/codesteps also requiredocker, which is unavailable hereScope