Skip to content

Commit d10d7ea

Browse files
ndeloofndeloof
committed
fix(provider): get-service-config resolves a build-only service's image
A service declaring only build: (no image:) answered get-service-config with an empty "image" field: service.Image is the YAML-declared value, and compose-go never fills it in for this case (every other caller of GetImageNameOrDefault in this package exists precisely because of that). The provider only ever sees this response, never the model compose builds internally, so an sbx-style provider building its own runtime image rejected the service outright with "defines no image". The response now carries the resolved name (api.GetImageNameOrDefault) instead - the same one the image phase already built and tagged by the time a provider asks. The build directive itself is cleared before marshaling: a provider has no builder to run it against, only an image identity to run. Pinned by a regression test using the existing fake-provider harness, verified by ablation to fail on either half of the fix reverted. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
1 parent 20adf11 commit d10d7ea

2 files changed

Lines changed: 70 additions & 1 deletion

File tree

‎pkg/compose/plugins.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,18 @@ func (s *composeService) handlePluginMessage(
416416
}
417417
variables.raw[key] = val
418418
case GetServiceConfigType:
419-
payload, err := json.Marshal(service)
419+
// service.Image is the YAML-declared value: empty for a build-only
420+
// service (compose-go never fills it in — see api.GetImageNameOrDefault's
421+
// own callers throughout this package). The provider only ever sees
422+
// this response, never the model compose builds internally, so it
423+
// must get the resolved name, the same one the image phase built and
424+
// tagged — never the build directive itself: a provider has no
425+
// builder to run it against, and by the time it asks, the image
426+
// phase has already built and tagged the image this config now names.
427+
resolved := service
428+
resolved.Image = api.GetImageNameOrDefault(service, project.Name)
429+
resolved.Build = nil
430+
payload, err := json.Marshal(resolved)
420431
if err != nil {
421432
return fmt.Errorf("failed to answer get-service-config: %w", err)
422433
}

‎pkg/compose/plugins_control_test.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,64 @@ func TestHelperProviderConfig(t *testing.T) {
9999
os.Exit(0)
100100
}
101101

102+
// TestExecutePlugin_GetServiceConfigResolvesBuildOnlyImage is a regression
103+
// test: a service declaring only build: (no image:) must still answer
104+
// get-service-config with a non-empty image — the name the image phase
105+
// built and tagged (api.GetImageNameOrDefault), not the YAML-declared
106+
// service.Image, which compose-go never fills in for this case. A provider
107+
// consuming a build-only service (e.g. a sandbox provider building its own
108+
// runtime image) otherwise sees an empty "image" field and rejects the
109+
// service outright. The build directive itself must NOT be forwarded: a
110+
// provider has no builder to run it against, only an image identity to run.
111+
func TestExecutePlugin_GetServiceConfigResolvesBuildOnlyImage(t *testing.T) {
112+
mockCtrl := gomock.NewController(t)
113+
cli := mocks.NewMockCli(mockCtrl)
114+
cli.EXPECT().Client().Return(mocks.NewMockAPIClient(mockCtrl)).AnyTimes()
115+
svc, err := NewComposeService(cli, WithEventProcessor(noopEventProcessor{}))
116+
assert.NilError(t, err)
117+
118+
cmd := exec.Command(os.Args[0], "-test.run=TestHelperProviderConfigImage")
119+
cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
120+
121+
service := types.ServiceConfig{
122+
Name: "api",
123+
WorkloadSpec: types.WorkloadSpec{Build: &types.BuildConfig{Context: "."}},
124+
Provider: &types.ServiceProviderConfig{
125+
Type: "sbx",
126+
},
127+
}
128+
variables, err := svc.(*composeService).executePlugin(t.Context(), &types.Project{Name: "proj"}, cmd, "up", service)
129+
assert.NilError(t, err)
130+
assert.Equal(t, variables.prefixed["IMAGE"], "proj-api")
131+
assert.Equal(t, variables.prefixed["HAS_BUILD"], "false")
132+
}
133+
134+
// TestHelperProviderConfigImage is not a test: it is the fake provider
135+
// process spawned by TestExecutePlugin_GetServiceConfigResolvesBuildOnlyImage.
136+
func TestHelperProviderConfigImage(t *testing.T) {
137+
if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
138+
t.Skip("helper process for TestExecutePlugin_GetServiceConfigResolvesBuildOnlyImage")
139+
}
140+
responses := json.NewDecoder(os.Stdin)
141+
emit := func(msg JsonMessage) {
142+
if err := json.NewEncoder(os.Stdout).Encode(msg); err != nil {
143+
os.Exit(1)
144+
}
145+
}
146+
emit(JsonMessage{Type: GetServiceConfigType})
147+
var config struct {
148+
Image string `json:"image"`
149+
Build json.RawMessage `json:"build"`
150+
}
151+
if err := responses.Decode(&config); err != nil {
152+
emit(JsonMessage{Type: ErrorType, Message: fmt.Sprintf("reading service config: %v", err)})
153+
os.Exit(0)
154+
}
155+
emit(JsonMessage{Type: SetEnvType, Message: "IMAGE=" + config.Image})
156+
emit(JsonMessage{Type: SetEnvType, Message: fmt.Sprintf("HAS_BUILD=%t", config.Build != nil)})
157+
os.Exit(0)
158+
}
159+
102160
// TestExecutePlugin_GetRelayInfo runs executePlugin against a fake provider
103161
// (this test binary re-executed, see TestHelperProviderRelayInfo): the
104162
// get-relay-info message must be answered with one JSON line listing the

0 commit comments

Comments
 (0)