Skip to content

Commit 4c7c542

Browse files
fix: refuse a service nothing serves; reset every listener at Init; no HTTP in a listener-less image
Follow-ups to #113's review (r76): - Load (builder and runtime) refuses a service with no HTTP endpoint and no gRPC endpoint for grpc-server to serve, by name: a manifest that lost its endpoints no longer starts a process nothing reaches. - Init resets the gRPC and Connect ports and addresses with the HTTP ones, so a reload that drops an endpoint cannot leave Start binding or waiting on it. The gRPC-only readiness path is now tested through Init and Start. - The Dockerfile of a service with no HTTP endpoint starts uvicorn on a loopback port the system picks, as a native start does, instead of binding 0.0.0.0:8080. The goldens of an HTTP service are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 6a4433e commit 4c7c542

5 files changed

Lines changed: 169 additions & 0 deletions

File tree

‎builder.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ func (s *Builder) Load(ctx context.Context, req *builderv0.LoadRequest) (*builde
100100
if err != nil {
101101
return s.Base.Builder.LoadError(err)
102102
}
103+
if err := s.FastAPI.unservedError(ctx, s.Endpoints); err != nil {
104+
return s.Base.Builder.LoadError(err)
105+
}
103106
if err := s.FastAPI.Settings.connectSettingError(); err != nil {
104107
return s.Base.Builder.LoadError(err)
105108
}
@@ -267,6 +270,10 @@ type DockerTemplating struct {
267270
// GitSources are the repositories of the git commits the lock pins, which
268271
// the builder stage reads from the caller's mirrors (uvgitsources.go).
269272
GitSources []uvGitMirror
273+
// NoHTTPEndpoint starts uvicorn on a loopback port the system picks, as a
274+
// native start does (uvicornArgs): a service with no HTTP endpoint serves
275+
// no HTTP in the image either.
276+
NoHTTPEndpoint bool
270277
}
271278

272279
func (s *Builder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) {
@@ -342,6 +349,7 @@ func (s *Builder) Build(ctx context.Context, req *builderv0.BuildRequest) (*buil
342349
UVImage: uvImage,
343350
ProjectWorkdir: defaultProjectWorkdir,
344351
GitSources: uvGitMirrors(gitSources),
352+
NoHTTPEndpoint: s.FastAPI.HTTPEndpoint == nil,
345353
}
346354
if assembled != nil {
347355
docker.ServicePrefix = assembled.ServicePrefix

‎connect_only_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,137 @@ func TestConnectOnlyServiceStarts(t *testing.T) {
331331
_, err = runtime.Stop(ctx, &runtimev0.StopRequest{})
332332
require.NoError(t, err)
333333
}
334+
335+
// rewriteManifest applies edit to a scaffolded service's manifest.
336+
func rewriteManifest(t *testing.T, root string, edit func(document map[string]any)) {
337+
t.Helper()
338+
manifest := filepath.Join(root, "service.codefly.yaml")
339+
content, err := os.ReadFile(manifest)
340+
require.NoError(t, err)
341+
var document map[string]any
342+
require.NoError(t, yaml.Unmarshal(content, &document))
343+
edit(document)
344+
content, err = yaml.Marshal(document)
345+
require.NoError(t, err)
346+
require.NoError(t, os.WriteFile(manifest, content, 0o644))
347+
}
348+
349+
// TestUnservedServiceIsRefusedAtLoad: a manifest that lost its endpoints, or
350+
// whose gRPC server has no endpoint to serve, is refused by both Loads by name
351+
// instead of starting a process nothing reaches.
352+
func TestUnservedServiceIsRefusedAtLoad(t *testing.T) {
353+
ctx := context.Background()
354+
for name, tc := range map[string]struct {
355+
answers map[string]*agentv0.Answer
356+
}{
357+
"no endpoints, no gRPC server": {answers: nil},
358+
"gRPC server with no gRPC endpoint": {answers: map[string]*agentv0.Answer{
359+
HotReload: confirmAnswer(false), PublicEndpoint: confirmAnswer(false), GRPCServer: confirmAnswer(true),
360+
}},
361+
} {
362+
t.Run(name, func(t *testing.T) {
363+
created, root := createServiceForTest(t, tc.answers)
364+
rewriteManifest(t, root, func(document map[string]any) { delete(document, "endpoints") })
365+
identity, err := created.Identity.Proto()
366+
require.NoError(t, err)
367+
368+
loaded, err := NewBuilder(NewService()).Load(ctx, &builderv0.LoadRequest{Identity: identity})
369+
require.NoError(t, err)
370+
require.Equal(t, builderv0.LoadStatus_ERROR, loaded.GetState().GetState())
371+
require.Contains(t, loaded.GetState().GetMessage(), "nothing would serve this service")
372+
373+
runtime := NewRuntime(NewService())
374+
t.Cleanup(runtime.Base.StopWatcher)
375+
resp, err := runtime.Load(ctx, &runtimev0.LoadRequest{Identity: identity, Environment: localEnvironment(t), DisableCatch: true})
376+
require.NoError(t, err)
377+
require.Equal(t, runtimev0.LoadStatus_ERROR, resp.GetStatus().GetState())
378+
require.Contains(t, resp.GetStatus().GetMessage(), "nothing would serve this service")
379+
})
380+
}
381+
}
382+
383+
// grpcOnlyInit stands a runtime up at Init for a service whose one face is its
384+
// gRPC server, with the proposed mapping Init resolves it from.
385+
func grpcOnlyInit(t *testing.T) (*Runtime, *fakeRunnerEnvironment, *runtimev0.InitRequest, string) {
386+
t.Helper()
387+
runtime, req := initTestRuntime(t)
388+
env := &fakeRunnerEnvironment{}
389+
runtime.runnerEnvironment = env
390+
runtime.cacheLocation = t.TempDir()
391+
grpcEndpoint := &basev0.Endpoint{Module: "mod", Service: "probe", Name: "grpc", Api: standards.GRPC}
392+
connectEndpoint := &basev0.Endpoint{Module: "mod", Service: "probe", Name: "connect", Api: standards.CONNECT}
393+
runtime.FastAPI.HTTPEndpoint = nil
394+
runtime.FastAPI.GRPCEndpoint = grpcEndpoint
395+
mappings, _ := connectOnlyMappings(t, grpcEndpoint, connectEndpoint)
396+
req.ProposedNetworkMappings = mappings[:1]
397+
return runtime, env, req, mappings[0].GetInstances()[0].GetAddress()
398+
}
399+
400+
// TestGRPCOnlyStartWaitsOnTheGRPCServer: with neither an HTTP endpoint nor a
401+
// Connect face, Start's readiness is the gRPC server's handshake at the address
402+
// Init resolved for it.
403+
func TestGRPCOnlyStartWaitsOnTheGRPCServer(t *testing.T) {
404+
isolateWorkingDir(t)
405+
ctx := context.Background()
406+
runtime, _, req, grpcAddress := grpcOnlyInit(t)
407+
var asked []string
408+
runtime.answers = func(_ context.Context, target string) error {
409+
asked = append(asked, target)
410+
return nil
411+
}
412+
initialized, err := runtime.Init(ctx, req)
413+
require.NoError(t, err)
414+
require.Equal(t, runtimev0.InitStatus_READY, initialized.GetStatus().GetState(), initialized.GetStatus().GetMessage())
415+
started, err := runtime.Start(ctx, &runtimev0.StartRequest{})
416+
require.NoError(t, err)
417+
require.Equal(t, runtimev0.StartStatus_STARTED, started.GetStatus().GetState(), started.GetStatus().GetMessage())
418+
t.Cleanup(func() { _, _ = runtime.Stop(context.Background(), &runtimev0.StopRequest{}) })
419+
require.Equal(t, []string{grpcAddress}, asked)
420+
}
421+
422+
// TestInitDropsTheListenersAReloadRemoved: an Init after a reload that dropped
423+
// the Connect endpoint publishes no Connect port or address, so Start neither
424+
// hands the process the old port nor waits on the old face.
425+
func TestInitDropsTheListenersAReloadRemoved(t *testing.T) {
426+
isolateWorkingDir(t)
427+
ctx := context.Background()
428+
runtime, env, req, grpcAddress := grpcOnlyInit(t)
429+
connectEndpoint := &basev0.Endpoint{Module: "mod", Service: "probe", Name: "connect", Api: standards.CONNECT}
430+
runtime.FastAPI.ConnectEndpoint = connectEndpoint
431+
withConnect, _ := connectOnlyMappings(t, runtime.FastAPI.GRPCEndpoint, connectEndpoint)
432+
req.ProposedNetworkMappings = append(req.ProposedNetworkMappings, withConnect[1])
433+
initialized, err := runtime.Init(ctx, req)
434+
require.NoError(t, err)
435+
require.Equal(t, runtimev0.InitStatus_READY, initialized.GetStatus().GetState(), initialized.GetStatus().GetMessage())
436+
require.NotEmpty(t, runtime.connectAddress)
437+
438+
runtime.FastAPI.ConnectEndpoint = nil
439+
req.ProposedNetworkMappings = req.ProposedNetworkMappings[:1]
440+
initialized, err = runtime.Init(ctx, req)
441+
require.NoError(t, err)
442+
require.Equal(t, runtimev0.InitStatus_READY, initialized.GetStatus().GetState(), initialized.GetStatus().GetMessage())
443+
444+
var asked []string
445+
runtime.answers = func(_ context.Context, target string) error {
446+
asked = append(asked, target)
447+
return nil
448+
}
449+
started, err := runtime.Start(ctx, &runtimev0.StartRequest{})
450+
require.NoError(t, err)
451+
require.Equal(t, runtimev0.StartStatus_STARTED, started.GetStatus().GetState(), started.GetStatus().GetMessage())
452+
t.Cleanup(func() { _, _ = runtime.Stop(context.Background(), &runtimev0.StopRequest{}) })
453+
require.Equal(t, []string{grpcAddress}, asked)
454+
require.Empty(t, processEnv(uvicornProc(t, env), "CODEFLY_CONNECT_PORT"))
455+
}
456+
457+
// TestImageServesNoHTTPWithoutAnHTTPEndpoint: the image of a service with no
458+
// HTTP endpoint binds uvicorn to loopback, never 0.0.0.0:8080.
459+
func TestImageServesNoHTTPWithoutAnHTTPEndpoint(t *testing.T) {
460+
builder, _ := createdBuilder(t)
461+
builder.FastAPI.HTTPEndpoint = nil
462+
outputDir := t.TempDir()
463+
dockerfile := dockerfileOf(t, outputDir, emitRecipe(t, builder, outputDir))
464+
require.Contains(t, dockerfile, `CMD ["uvicorn", "src.main:app", "--host", "127.0.0.1", "--port", "0", "--log-config", "uvicorn-log-config.json"]`)
465+
require.NotContains(t, dockerfile, "0.0.0.0")
466+
require.NotContains(t, dockerfile, "8080")
467+
}

‎main.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,20 @@ func findHTTPEndpoint(ctx context.Context, endpoints []*v0.Endpoint) (*v0.Endpoi
204204
return nil, fmt.Errorf("the app serves one HTTP listener, and the service declares %d HTTP endpoints (%s): keep one with api: rest or api: http", len(found), strings.Join(names, ", "))
205205
}
206206

207+
// unservedError refuses a service nothing would serve: no HTTP endpoint, and
208+
// no gRPC endpoint for a gRPC server to serve. A manifest that lost its
209+
// endpoints would otherwise start a process nobody can reach, and Start would
210+
// have nothing to wait on before it answered STARTED.
211+
func (s *Service) unservedError(ctx context.Context, endpoints []*v0.Endpoint) error {
212+
if s.HTTPEndpoint != nil {
213+
return nil
214+
}
215+
if s.Settings.GRPCServer.Enabled && len(resources.FindEndpointsByAPI(ctx, standards.GRPC, endpoints)) > 0 {
216+
return nil
217+
}
218+
return fmt.Errorf("nothing would serve this service: it declares no HTTP endpoint (api: rest or api: http) and no gRPC endpoint with %s.enabled", GRPCServer)
219+
}
220+
207221
// connectSettingError refuses a Connect face with no gRPC server to transcode
208222
// onto: the face serves the gRPC server's API, never one of its own.
209223
func (s *Settings) connectSettingError() error {

‎runtime.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,9 @@ func (s *Runtime) Load(ctx context.Context, req *runtimev0.LoadRequest) (*runtim
213213
if err != nil {
214214
return s.Base.Runtime.LoadError(err)
215215
}
216+
if err := s.FastAPI.unservedError(ctx, s.Endpoints); err != nil {
217+
return s.Base.Runtime.LoadError(err)
218+
}
216219

217220
if s.FastAPI.Settings.GRPCServer.Enabled {
218221
s.FastAPI.GRPCEndpoint, err = resources.FindGRPCEndpoint(ctx, s.Endpoints)
@@ -499,7 +502,11 @@ func (s *Runtime) Init(ctx context.Context, req *runtimev0.InitRequest) (*runtim
499502
return s.Base.Runtime.InitError(err)
500503
}
501504

505+
// Every listener Init publishes is reset first: a reload that dropped an
506+
// endpoint must not leave Start binding or waiting on the old one.
502507
s.port, s.serveAddress = 0, ""
508+
s.grpcPort, s.grpcAddress = 0, ""
509+
s.connectPort, s.connectAddress = 0, ""
503510
if s.FastAPI.HTTPEndpoint != nil {
504511
net, err := resources.FindNetworkInstanceInNetworkMappings(ctx, s.NetworkMappings, s.FastAPI.HTTPEndpoint, resources.NewNativeNetworkAccess())
505512
if err != nil {

‎templates/builder/Dockerfile.tmpl‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,5 +112,11 @@ WORKDIR /app/code
112112
# writes it. The builder writes both into the project before the build.
113113
COPY --chown=appuser {{.ServicePrefix}}code/uvicorn-log-config.json {{.ServicePrefix}}code/codefly_uvicorn_logging.py ./
114114

115+
{{if .NoHTTPEndpoint -}}
116+
# No HTTP endpoint: the app runs under uvicorn for its lifespan (the gRPC
117+
# server, the Connect face) on a loopback port the system picks.
118+
CMD ["uvicorn", "src.main:app", "--host", "127.0.0.1", "--port", "0", "--log-config", "uvicorn-log-config.json"]
119+
{{- else -}}
115120
# Default REST port (overridden by codefly at runtime).
116121
CMD ["uvicorn", "src.main:app", "--host", "0.0.0.0", "--port", "8080", "--log-config", "uvicorn-log-config.json"]
122+
{{- end}}

0 commit comments

Comments
 (0)