Skip to content

Commit 4e3184c

Browse files
fix(run): deliver a binding outside the run as declared; refuse only the host
Review r53 of 193494a: withholding a binding to a module outside the run dropped what the local authority supports on purpose, an audience the run does not supervise (the default fixture's model stand-in, a playground's bridge to its model provider). The authority decides a binding at call time, so before #1158 such a binding was delivered and served. A binding is now delivered as declared whatever it targets. A binding to a module nothing composes grants nothing, and the authority refuses a call through it by name ("audience X is not composed"). Admission keeps #1158's refusal for exactly one target, the host's own module, which the run declares to no authority; a module selected only through services with no identity workload (a datastore) is no longer refused. The selection is read once per admission for both decisions, and a binding outside the run is reported once per module and binding, not on every start. Tests: a binding to a module outside the run, to an identity-less selected module and to a selected module is delivered unchanged, every selected module with an identity workload is admitted with its own workloads only, and the outside-the-run warning is logged exactly once across two admissions; the host binding is still refused before anything is sent. Sabotage-checked: #1158's broader refusal fails the outside and datastore cases, and dropping the once-guard fails the warning count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 193494a commit 4e3184c

3 files changed

Lines changed: 83 additions & 86 deletions

File tree

‎cmd/run/launcher.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ type launcher struct {
9494
// delivered is the last manifest delivered per module, a withdrawal
9595
// included; protected by delivery rather than the short-lived state lock.
9696
delivered map[string]*solutionhost.ModuleManifest
97+
// outsideTargets are the bindings to a module outside the run that
98+
// admission has reported, so each is said once; protected by delivery.
99+
outsideTargets map[string]bool
97100
// pendingWithdrawals are modules whose withdrawal the host has not applied
98101
// yet: retried on the next composition refresh, even with no gone roots.
99102
pendingWithdrawals map[string]bool
@@ -872,8 +875,7 @@ func (cluster *launcher) deliverReadiness(ctx context.Context, module string, ea
872875

873876
// renderDelivery renders a loaded module's manifest. An early delivery keeps
874877
// the page payload previously published, since deferral removes a page; a
875-
// ready one names the bundle its caller built. A binding to a module outside
876-
// the run is withheld (withholdUnselectedBindings).
878+
// ready one names the bundle its caller built.
877879
func (cluster *launcher) renderDelivery(ctx context.Context, deliverer *gitops.NativeDeliverer, loaded *resources.Module, early bool, bundle *solutionhost.FrontendPayload) (*solutionhost.ModuleManifest, error) {
878880
module := loaded.Name
879881
services := make([]*resources.Service, 0, len(loaded.ServiceReferences))
@@ -900,15 +902,10 @@ func (cluster *launcher) renderDelivery(ctx context.Context, deliverer *gitops.N
900902
if err != nil {
901903
return nil, err
902904
}
903-
manifest, err := deliverer.Render(ctx, &gitops.NativeWorkload{
905+
return deliverer.Render(ctx, &gitops.NativeWorkload{
904906
Module: loaded, Services: services, Declared: cluster.declaredIdentities(module),
905907
Started: started, Carriers: carriers, FrontendBundle: bundle, DeferFrontend: early && bundle == nil,
906908
})
907-
if err != nil {
908-
return nil, err
909-
}
910-
cluster.withholdUnselectedBindings(ctx, manifest)
911-
return manifest, nil
912909
}
913910

914911
// Caller holds delivery across rendering, generation reservation and Applied.

‎cmd/run/launcher_admission.go‎

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,17 @@ func (cluster *launcher) admitSelected(ctx context.Context) error {
8484
return err
8585
}
8686
defer cluster.delivery.Release(1)
87-
modules, _ := cluster.runModules()
87+
// One snapshot of the selection decides both what is admitted and which
88+
// binding targets are part of the run.
89+
modules, selected := map[string]bool{}, map[string]bool{}
90+
for _, unique := range cluster.flow.LocalRuntimeServices() {
91+
if ref, err := resources.ParseServiceWithOptionalModule(unique); err == nil {
92+
selected[ref.Module] = true
93+
}
94+
if identity, ok := cluster.identity(unique); ok && cluster.delivers(identity) {
95+
modules[identity.Module] = true
96+
}
97+
}
8898
names := make([]string, 0, len(modules))
8999
for module := range modules {
90100
names = append(names, module)
@@ -109,15 +119,8 @@ func (cluster *launcher) admitSelected(ctx context.Context) error {
109119
if err != nil {
110120
return err
111121
}
112-
// renderDelivery withheld every binding to a module outside the run, so
113-
// what remains targets the run; one whose target this run declares to
114-
// no authority (the host's own module) could never be served.
115-
if manifest.Authority != nil {
116-
for _, binding := range manifest.Authority.Bindings {
117-
if !modules[binding.Audience] {
118-
return fmt.Errorf("module %s binding %s targets %s, which has no selected local identity workload; authority admission requires its target declaration", module, binding.ID, binding.Audience)
119-
}
120-
}
122+
if err := cluster.checkBindingTargets(ctx, module, manifest, selected); err != nil {
123+
return err
121124
}
122125
manifests = append(manifests, manifest)
123126
}
@@ -129,41 +132,35 @@ func (cluster *launcher) admitSelected(ctx context.Context) error {
129132
return nil
130133
}
131134

132-
// runModules names the modules of the run's selected services (selected) and
133-
// those whose manifests the run delivers (delivered): every selected module
134-
// with an identity workload but the host's own.
135-
func (cluster *launcher) runModules() (delivered, selected map[string]bool) {
136-
delivered, selected = map[string]bool{}, map[string]bool{}
137-
for _, unique := range cluster.flow.LocalRuntimeServices() {
138-
if ref, err := resources.ParseServiceWithOptionalModule(unique); err == nil {
139-
selected[ref.Module] = true
140-
}
141-
if identity, ok := cluster.identity(unique); ok && cluster.delivers(identity) {
142-
delivered[identity.Module] = true
143-
}
144-
}
145-
return delivered, selected
146-
}
147-
148-
// withholdUnselectedBindings drops from a manifest every binding addressed to
149-
// a module this run does not select. A run composes a unit and only the
150-
// modules it consumes, so such a binding is not a fault of the run: the
151-
// delivered document carries no grant through it, and the authority refuses a
152-
// call through it by name. Nothing is granted in its place.
153-
func (cluster *launcher) withholdUnselectedBindings(ctx context.Context, manifest *solutionhost.ModuleManifest) {
154-
if manifest == nil || manifest.Authority == nil {
155-
return
135+
// checkBindingTargets decides, at admission, a module's bindings by where they
136+
// point. A binding to the host's own module could never be served, since the
137+
// run declares that module to no authority, so admission refuses it before
138+
// any manifest is sent. Every other binding is delivered as declared: a run
139+
// composes a unit and only the modules it consumes, and the authority decides
140+
// a binding at call time, serving it when its audience is composed (by this
141+
// run, or by the fixture's stand-in for a process the run does not supervise)
142+
// and refusing a call through it by name when nothing composes it. A binding
143+
// to a module outside the run is said once, since admission repeats per start.
144+
// Caller holds delivery.
145+
func (cluster *launcher) checkBindingTargets(ctx context.Context, module string, manifest *solutionhost.ModuleManifest, selected map[string]bool) error {
146+
if manifest.Authority == nil {
147+
return nil
156148
}
157-
_, selected := cluster.runModules()
158-
kept := make([]solutionhost.AuthorityBinding, 0, len(manifest.Authority.Bindings))
159149
for _, binding := range manifest.Authority.Bindings {
160-
if selected[binding.Audience] {
161-
kept = append(kept, binding)
150+
if binding.Audience == cluster.hostModule {
151+
return fmt.Errorf("module %s binding %s targets %s, the host's own module, which this run declares to no authority; authority admission requires its target declaration", module, binding.ID, binding.Audience)
152+
}
153+
key := module + "/" + binding.ID
154+
if selected[binding.Audience] || cluster.outsideTargets[key] {
162155
continue
163156
}
164-
wool.Get(ctx).In("launcher.deliver").Warn(
165-
fmt.Sprintf("module %s binding %s is withheld from the delivered manifest: %s is not part of this run, so a call through it is refused", manifest.Alias, binding.ID, binding.Audience),
166-
wool.Field("module", manifest.Alias), wool.Field("binding", binding.ID), wool.Field("target", binding.Audience))
157+
if cluster.outsideTargets == nil {
158+
cluster.outsideTargets = map[string]bool{}
159+
}
160+
cluster.outsideTargets[key] = true
161+
wool.Get(ctx).In("launcher.admit").Warn(
162+
fmt.Sprintf("module %s binding %s targets %s, which is not part of this run: it is delivered as declared, and the authority refuses a call through it (audience %q is not composed) unless something else composes %s, such as a fixture's stand-in", module, binding.ID, binding.Audience, binding.Audience, binding.Audience),
163+
wool.Field("module", module), wool.Field("binding", binding.ID), wool.Field("target", binding.Audience))
167164
}
168-
manifest.Authority.Bindings = kept
165+
return nil
169166
}

‎cmd/run/launcher_admission_test.go‎

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net"
99
"os"
1010
"path/filepath"
11+
"strings"
1112
"sync"
1213
"sync/atomic"
1314
"testing"
@@ -25,6 +26,7 @@ import (
2526
basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
2627
"github.com/codefly-dev/core/resources"
2728
"github.com/codefly-dev/core/solutionhost"
29+
"github.com/codefly-dev/core/wool"
2830
"github.com/stretchr/testify/require"
2931
"google.golang.org/grpc"
3032
)
@@ -214,56 +216,57 @@ func writeTargetBinding(t *testing.T, cluster *launcher, audience string) {
214216
require.NoError(t, os.WriteFile(path, []byte(contract), 0600))
215217
}
216218

217-
// A binding to a module the run selects but declares to no authority (the
218-
// host's own) could never be served: admission refuses the whole selection
219+
// A binding to the host's own module could never be served: the run declares
220+
// that module to no authority, so admission refuses the whole selection
219221
// before sending any manifest.
220-
func TestAdmissionRefusesUndeliveredBindingBeforeSending(t *testing.T) {
222+
func TestAdmissionRefusesBindingToHostBeforeSending(t *testing.T) {
221223
cluster, peer := admissionLauncher(t)
222224
writeTargetBinding(t, cluster, "model")
223225
err := cluster.admitSelected(t.Context())
224-
require.ErrorContains(t, err, "binding read-target targets model")
226+
require.ErrorContains(t, err, "binding read-target targets model, the host's own module")
225227
require.Empty(t, peer.snapshots(), "validate the entire selection before applying any manifest")
226228
}
227229

228230
// A run composes a unit and only the modules it consumes: a binding to a
229-
// module outside the run does not stop startup. The delivered manifest
230-
// carries no grant through it, every selected module is still admitted with
231-
// its own workloads only, and the selected target's binding is kept.
232-
func TestAdmissionWithholdsBindingToUnselectedModule(t *testing.T) {
233-
for _, tc := range []struct {
234-
audience string
235-
kept bool
236-
}{{"missing", false}, {"beta", true}} {
237-
t.Run(tc.audience, func(t *testing.T) {
231+
// module outside the run, or to one selected only through services with no
232+
// identity workload, does not stop startup. It is delivered as declared, for
233+
// the authority to decide at call time; every selected module is admitted
234+
// with its own workloads only; a binding outside the run is said once.
235+
func TestAdmissionDeliversBindingsOutsideTheRunAsDeclared(t *testing.T) {
236+
for _, audience := range []string{"missing", "store", "beta"} {
237+
t.Run(audience, func(t *testing.T) {
238238
cluster, peer := admissionLauncher(t)
239-
writeTargetBinding(t, cluster, tc.audience)
240-
require.NoError(t, cluster.admitSelected(t.Context()))
241-
snapshots := peer.snapshots()
242-
require.Len(t, snapshots, 2, "every selected module is admitted")
243-
for _, m := range snapshots {
239+
flow, ok := cluster.flow.(*admissionFlow)
240+
require.True(t, ok)
241+
flow.selected = append(flow.selected, "store/db") // a datastore: selected, no identity workload
242+
writeTargetBinding(t, cluster, audience)
243+
capture := &refreshLogCapture{}
244+
provider := wool.New(context.Background(), &wool.Resource{}).WithLogger(capture)
245+
defer provider.Done()
246+
ctx := provider.Inject(t.Context())
247+
for range 2 { // admission repeats on every start
248+
require.NoError(t, cluster.admitSelected(ctx))
249+
}
250+
admitted := map[string]bool{}
251+
for _, m := range peer.snapshots() {
252+
admitted[m.Alias] = true
244253
require.NotNil(t, m.Authority)
245-
for _, w := range m.Workloads {
246-
require.Equal(t, "api", w.Service, "an identity is declared for a selected workload only")
247-
}
254+
require.Len(t, m.Workloads, 1)
255+
require.Equal(t, "api", m.Workloads[0].Service, "an identity is declared for a selected workload only")
248256
if m.Alias != "alpha" {
249257
continue
250258
}
251-
var audiences []string
252-
for _, binding := range m.Authority.Bindings {
253-
audiences = append(audiences, binding.Audience)
254-
}
255-
if tc.kept {
256-
require.Equal(t, []string{tc.audience}, audiences, "a binding to a selected module is delivered")
257-
} else {
258-
require.Empty(t, audiences, "a binding to a module outside the run carries no grant")
259-
}
259+
require.Len(t, m.Authority.Bindings, 1)
260+
require.Equal(t, "read-target", m.Authority.Bindings[0].ID)
261+
require.Equal(t, audience, m.Authority.Bindings[0].Audience, "the binding is delivered as declared")
262+
}
263+
require.Equal(t, map[string]bool{"alpha": true, "beta": true}, admitted, "only selected modules with an identity workload are admitted")
264+
said := strings.Count(strings.Join(capture.messages, "\n"), "binding read-target targets "+audience+", which is not part of this run")
265+
if audience == "missing" {
266+
require.Equal(t, 1, said, "a binding outside the run is said, once")
267+
} else {
268+
require.Zero(t, said, "a binding to a selected module is part of the run")
260269
}
261-
// The ready delivery renders through the same path and withholds it too.
262-
cluster.running["alpha/api"] = true
263-
require.NoError(t, cluster.deliverReadiness(t.Context(), "alpha", false))
264-
latest := peer.snapshots()[len(peer.snapshots())-1]
265-
require.Equal(t, "alpha", latest.Alias)
266-
require.Equal(t, tc.kept, len(latest.Authority.Bindings) == 1)
267270
})
268271
}
269272
}

0 commit comments

Comments
 (0)