From 109ca1b671ebd18d6ddb1a1d55de527bc4bf7eb5 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 19:11:56 -0400 Subject: [PATCH] fix(plugin/external): tolerate Unimplemented GetManifest for strict-cutover IaC plugins NewExternalPluginAdapter currently fails outright when GetManifest returns codes.Unimplemented. PR #623 added the iacPluginServiceBridge that wires GetContractRegistry for strict-cutover IaC plugins (DO v1.0.0+), but the bridge intentionally leaves GetManifest unimplemented (per the bridge's inline comment). Loader-side adapter.go didn't get a matching relaxation, so DO v1.0.1 loads its gRPC server, then immediately fails: rpc error: code = Unimplemented desc = method GetManifest not implemented load plugin "digitalocean": get manifest from plugin digitalocean This blocks every consumer running typed-IaC dispatch against a strict-cutover plugin. Observed end-to-end via core-dump strict-contracts-smoke run 25642339926 against staging. Fix: when GetManifest returns codes.Unimplemented, synthesize a minimal Manifest{Name: name} from the param-passed plugin name. Downstream accessors (Name/Version/Description/etc.) return sensible values; the loader already keys off the param-passed name everywhere meaningful (adapter.go:111). Non-Unimplemented errors still fail as before. Tests: - TestNewExternalPluginAdapter_GetManifestUnimplemented_SynthesizesFromName - TestNewExternalPluginAdapter_GetManifestNonUnimplementedError_Fails Co-Authored-By: Claude Opus 4.7 (1M context) --- plugin/external/adapter.go | 10 +++++- plugin/external/adapter_test.go | 58 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/plugin/external/adapter.go b/plugin/external/adapter.go index 9f582c9c..5fd29ae2 100644 --- a/plugin/external/adapter.go +++ b/plugin/external/adapter.go @@ -86,7 +86,15 @@ func NewExternalPluginAdapter(name string, client *PluginClient) (*ExternalPlugi ctx := context.Background() manifest, err := client.client.GetManifest(ctx, &emptypb.Empty{}) if err != nil { - return nil, fmt.Errorf("get manifest from plugin %s: %w", name, err) + if status.Code(err) != codes.Unimplemented { + return nil, fmt.Errorf("get manifest from plugin %s: %w", name, err) + } + // Strict-cutover IaC plugins (e.g. workflow-plugin-digitalocean v1.0.0+) + // register only PluginService.GetContractRegistry via the iacPluginServiceBridge + // and leave GetManifest unimplemented. Synthesize a minimal manifest from the + // plugin name so adapter.Name() / Version() / Description() accessors return + // sensible values; downstream code keys off the param-passed name anyway. + manifest = &pb.Manifest{Name: name} } var triggerSetupErr error triggerTypes, triggerErr := client.client.GetTriggerTypes(ctx, &emptypb.Empty{}) diff --git a/plugin/external/adapter_test.go b/plugin/external/adapter_test.go index 6358b153..dd58f616 100644 --- a/plugin/external/adapter_test.go +++ b/plugin/external/adapter_test.go @@ -795,3 +795,61 @@ func malformedContractFileDescriptorSet() *descriptorpb.FileDescriptorSet { func stringPtr(v string) *string { return &v } func int32Ptr(v int32) *int32 { return &v } + +// unimplementedManifestClient simulates a strict-cutover IaC plugin whose +// PluginService bridge implements GetContractRegistry but leaves GetManifest +// unimplemented (workflow-plugin-digitalocean v1.0.0+ behavior). +type unimplementedManifestClient struct { + adapterTestPluginServiceClient +} + +func (c *unimplementedManifestClient) GetManifest(_ context.Context, _ *emptypb.Empty, _ ...grpc.CallOption) (*pb.Manifest, error) { + return nil, status.Error(codes.Unimplemented, "method GetManifest not implemented") +} + +// TestNewExternalPluginAdapter_GetManifestUnimplemented_SynthesizesFromName +// asserts that NewExternalPluginAdapter tolerates GetManifest returning +// codes.Unimplemented and synthesizes a minimal manifest from the param name. +// Regression coverage for strict-cutover IaC plugins (DO v1.0.0+) whose +// iacPluginServiceBridge only wires GetContractRegistry. +func TestNewExternalPluginAdapter_GetManifestUnimplemented_SynthesizesFromName(t *testing.T) { + client := &unimplementedManifestClient{ + adapterTestPluginServiceClient: adapterTestPluginServiceClient{ + registry: &pb.ContractRegistry{}, + }, + } + a, err := NewExternalPluginAdapter("digitalocean", &PluginClient{client: client}) + if err != nil { + t.Fatalf("NewExternalPluginAdapter must tolerate Unimplemented GetManifest: %v", err) + } + if a.Name() != "digitalocean" { + t.Fatalf("expected synthesized manifest Name=digitalocean, got %q", a.Name()) + } + if a.Version() != "" { + t.Fatalf("expected empty synthesized manifest Version, got %q", a.Version()) + } +} + +// TestNewExternalPluginAdapter_GetManifestNonUnimplementedError_Fails asserts +// that non-Unimplemented errors from GetManifest still surface — only +// Unimplemented is tolerated. +func TestNewExternalPluginAdapter_GetManifestNonUnimplementedError_Fails(t *testing.T) { + client := &adapterTestPluginServiceClient{} + // Override GetManifest to return Internal. + failingClient := &failingManifestClient{adapterTestPluginServiceClient: *client} + _, err := NewExternalPluginAdapter("broken-plugin", &PluginClient{client: failingClient}) + if err == nil { + t.Fatal("expected error from non-Unimplemented GetManifest failure") + } + if !strings.Contains(err.Error(), "get manifest from plugin broken-plugin") { + t.Fatalf("expected wrapped error mentioning plugin name, got: %v", err) + } +} + +type failingManifestClient struct { + adapterTestPluginServiceClient +} + +func (c *failingManifestClient) GetManifest(_ context.Context, _ *emptypb.Empty, _ ...grpc.CallOption) (*pb.Manifest, error) { + return nil, status.Error(codes.Internal, "boom") +}