diff --git a/cmd/generate/permissions.go b/cmd/generate/permissions.go index b3b2bb88..9146282d 100644 --- a/cmd/generate/permissions.go +++ b/cmd/generate/permissions.go @@ -36,8 +36,9 @@ By default each Go service gets its own table in its own Go module: //pkg/gen/permissions, the tree the go-grpc layout generates its bindings into (source-dir is the Go agent's setting, "code" unless the service declares another). A service's table holds the methods its -published contracts serve plus the module contract's foreign methods, which -name no service. What an endpoint serves is what the contract catalog records +published contracts serve plus foreign methods selected for that service. +Unselected foreign methods appear in every Go service's table for compatibility. +What an endpoint serves is what the contract catalog records for it (the declared surface, or every service of its package); a method a published descriptor declares that no endpoint serves, such as a protocol other modules serve, is left out with a note and needs no policy. The TypeScript file @@ -119,8 +120,8 @@ type permissionDestination struct { // permissionDestinations places the module's method policies, and names the // published methods no endpoint serves, which no table holds. An explicit // output holds the whole module's table. Otherwise each Go service gets the -// methods its own published contracts serve, plus the foreign methods the -// module contract declares without naming a service, in the generated tree +// methods its own published contracts serve and foreign methods selected for it, +// plus legacy foreign methods with no service selector, in the generated tree // of its own Go module, so the service imports them with no flag. A Python // service that publishes policies is skipped and named in skipped: nothing in // it reads a table, because the hop in front of it decides every call. diff --git a/cmd/generate/permissions_test.go b/cmd/generate/permissions_test.go index a0604774..9e920496 100644 --- a/cmd/generate/permissions_test.go +++ b/cmd/generate/permissions_test.go @@ -154,6 +154,51 @@ services: return module } +func TestPermissionsForeignServiceSelectorsReachGeneratedTables(t *testing.T) { + module := permissionsModule(t) + path := filepath.Join(module.Dir(), "module.contract.codefly.yaml") + contract, err := os.ReadFile(path) + require.NoError(t, err) + contract = append(contract, []byte(` - method: /foreign.Api/Read + service: api + requires: + - resource_kind: ledger.entry + actions: [read] + - method: /foreign.Admin/Approve + service: admin + requires: + - resource_kind: ledger.entry + actions: [approve] +`)...) + require.NoError(t, os.WriteFile(path, contract, 0o644)) + module, err = resources.LoadModuleFromDir(t.Context(), module.Dir()) + require.NoError(t, err) + require.NoError(t, generatePermissions(t.Context(), module, "", false)) + require.NoError(t, generatePermissions(t.Context(), module, "", true)) + union := t.TempDir() + require.NoError(t, generatePermissions(t.Context(), module, union, false)) + for _, file := range []string{"permissions_gen.go", "permissions_gen.ts"} { + read := func(root string) string { + data, err := os.ReadFile(filepath.Join(root, file)) + require.NoError(t, err) + return string(data) + } + api := read(filepath.Join(module.Dir(), "services/api/code/pkg/gen/permissions")) + admin := read(filepath.Join(module.Dir(), "services/admin/go/pkg/gen/permissions")) + all := read(union) + require.Contains(t, api, "/foreign.Api/Read") + require.NotContains(t, api, "/foreign.Admin/Approve") + require.Contains(t, admin, "/foreign.Admin/Approve") + require.NotContains(t, admin, "/foreign.Api/Read") + for _, table := range []string{api, admin, all} { + require.Contains(t, table, "/otlp.TraceService/Export") + } + for _, method := range []string{"/foreign.Api/Read", "/foreign.Admin/Approve", "/ledger.ApiService/Read", "/ledger.AdminService/Approve", "/ledger.EngineService/Recompute"} { + require.Contains(t, all, method) + } + } +} + func TestPermissionsDefaultLandsInEachGoServiceModule(t *testing.T) { module := permissionsModule(t) require.NoError(t, generatePermissions(t.Context(), module, "", false)) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 45bdc9f4..c1fed1d0 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2867,8 +2867,9 @@ By default each Go service gets its own table in its own Go module: //pkg/gen/permissions, the tree the go-grpc layout generates its bindings into (source-dir is the Go agent's setting, "code" unless the service declares another). A service's table holds the methods its -published contracts serve plus the module contract's foreign methods, which -name no service. What an endpoint serves is what the contract catalog records +published contracts serve plus foreign methods selected for that service. +Unselected foreign methods appear in every Go service's table for compatibility. +What an endpoint serves is what the contract catalog records for it (the declared surface, or every service of its package); a method a published descriptor declares that no endpoint serves, such as a protocol other modules serve, is left out with a note and needs no policy. The TypeScript file diff --git a/docs/commands.md b/docs/commands.md index 42dd9b0c..d67c9f35 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -4645,8 +4645,10 @@ module: `//pkg/gen/permissions`, where `source-dir` is the Go agent's setting (`code` unless the service declares another; its first segment is where `go.mod` lives) and `pkg/gen` is the tree the go-grpc layout generates its bindings into. Each service's table holds the methods its own -published contracts serve, plus the module contract's `foreign_methods`, which -name no service and so appear in every Go service's table. What an endpoint +published contracts serve, plus the module contract's `foreign_methods` selected +for that `service`. Foreign methods without a service selector retain the legacy +behavior of appearing in every Go service's table. Their authority requirements +remain in the module-wide policy union. What an endpoint serves is what the contract catalog (`contracts/api/catalog.codefly.json`) records for it: the module's declared surface (`contracts.codefly.yaml`, see [Generate contracts](#generate-contracts)), or every service of the endpoint's @@ -4684,3 +4686,9 @@ Published package dependencies do not require local source trees. Permission generation is available through the CLI; no separate MCP generator was added. MCP run/test use the same composition preflight as their CLI paths. + +A module with multiple published protobuf services selects the serving service +on each `foreign_methods` entry with `service:`. The hop routes it only to that +service's published gRPC/Connect listeners. An absent selector requires exactly +one such service; unknown or non-protobuf selections are refused. This does not +change a method's authority policy or authorize shadowing a published method. diff --git a/go.mod b/go.mod index 513ccffe..8305e01e 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 - github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a + github.com/codefly-dev/core v0.19.7-0.20261027183450-b71ee8cd6ed0 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 diff --git a/go.sum b/go.sum index 47505b10..888cbe95 100644 --- a/go.sum +++ b/go.sum @@ -117,6 +117,8 @@ github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-00 github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93/go.mod h1:1rmjrVyr0h7zOuwOHS9i+BoeS9dr3XNvtupLZFwY6As= github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a h1:ar8ERpN9n9ZL29LZ63mqBZNLPZ7OpcnQ1+D1g89DbPY= github.com/codefly-dev/core v0.19.7-0.20261026194550-a8599bd3991a/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= +github.com/codefly-dev/core v0.19.7-0.20261027183450-b71ee8cd6ed0 h1:AH7fMtK5YrQV065aUS/9WibFEEpQiYTaxlBF/iKPTsw= +github.com/codefly-dev/core v0.19.7-0.20261027183450-b71ee8cd6ed0/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= diff --git a/pkg/gitops/method_policies.go b/pkg/gitops/method_policies.go index 889a82f3..ab0d8335 100644 --- a/pkg/gitops/method_policies.go +++ b/pkg/gitops/method_policies.go @@ -160,7 +160,8 @@ func (served *ServedMethodPolicies) Table(keep func(service string) bool) ([]*co // ServedMethodPolicies is a module's method policies by where they are // served: each service's published contracts, and the module contract's -// foreign methods, which name no service. +// foreign methods. Explicitly selected foreign methods join ByService; +// unselected entries remain in Foreign for existing consumers. type ServedMethodPolicies struct { // Package is the package's ID, which a refusal names. Package string @@ -198,13 +199,16 @@ type ServedMethodPolicies struct { // package's other services and its imports', so the rest is Unserved. A // package with no catalog falls back to that same default. func ServiceMethodPolicies(dir string) (served ServedMethodPolicies, ok bool, err error) { - foreign, err := foreignMethods(dir) + foreign, selected, err := foreignMethods(dir) if err != nil { return served, false, err } - served.Foreign = foreign served.ByService = map[string][]*compositionv1.MethodPolicyV1{} if _, statErr := os.Stat(filepath.Join(dir, composition.PackageManifestFileName)); os.IsNotExist(statErr) { + if len(selected) > 0 { + return served, false, fmt.Errorf("foreign method service selection requires a package manifest") + } + served.Foreign = foreign return served, len(foreign) > 0, nil } manifest, err := composition.LoadPackageManifest(dir) @@ -267,6 +271,20 @@ func ServiceMethodPolicies(dir string) (served ServedMethodPolicies, ok bool, er return served, false, fmt.Errorf("module package %s declares foreign method %s, which its published contracts already serve", manifest.ID, m.GetMethod()) } servedAnywhere[m.GetMethod()] = true + if service := selected[m.GetMethod()]; service != "" { + if !publishesProtobuf(manifest, service) { + return served, false, fmt.Errorf("foreign method %s selects service %q, which publishes no protobuf contract", m.GetMethod(), service) + } + served.ByService[service] = append(served.ByService[service], m) + if !slices.Contains(served.Services, service) { + served.Services = append(served.Services, service) + } + } else { + served.Foreign = append(served.Foreign, m) + } + } + for _, methods := range served.ByService { + sort.Slice(methods, func(i, j int) bool { return methods[i].GetMethod() < methods[j].GetMethod() }) } // A method one endpoint serves and another's descriptor merely carries is // served. @@ -284,26 +302,48 @@ func declaresNothing(m *compositionv1.MethodPolicyV1) bool { return !m.GetAnyCheckedCaller() && len(m.GetRequires()) == 0 } +// publishesProtobuf reports whether the manifest's service of that name +// publishes a protobuf contract, which is what a foreign method's service +// selector must name: the selector chooses where a method is served, so a +// service serving no protobuf cannot be the answer. +func publishesProtobuf(manifest *composition.PackageManifest, service string) bool { + for i := range manifest.Services { + if manifest.Services[i].Name != service { + continue + } + for _, contract := range manifest.Services[i].APIContracts { + if contract.Kind == composition.APIContractKindProtobuf { + return true + } + } + } + return false +} + // foreignMethods reads the methods the module at dir serves from a protocol // it does not own, from its module contract, each with what a call requires. // A module that publishes no contract declares none. -func foreignMethods(dir string) ([]*compositionv1.MethodPolicyV1, error) { +func foreignMethods(dir string) ([]*compositionv1.MethodPolicyV1, map[string]string, error) { contract, err := modulecontract.Load(dir) if errors.Is(err, os.ErrNotExist) { - return nil, nil + return nil, nil, nil } if err != nil { - return nil, err + return nil, nil, err } var out []*compositionv1.MethodPolicyV1 + selected := map[string]string{} for _, method := range contract.ForeignMethods { m := &compositionv1.MethodPolicyV1{Method: method.Method} + if method.Service != "" { + selected[method.Method] = method.Service + } for _, required := range method.Requires { m.Requires = append(m.Requires, &authorityv1.WorkScopeV1{ResourceKind: required.ResourceKind, Actions: required.Actions}) } out = append(out, m) } - return out, nil + return out, selected, nil } // ScopeCeilings reads the module's own resource kinds with the actions declared diff --git a/pkg/gitops/method_policies_test.go b/pkg/gitops/method_policies_test.go index ff02ae92..a19dd176 100644 --- a/pkg/gitops/method_policies_test.go +++ b/pkg/gitops/method_policies_test.go @@ -469,3 +469,71 @@ func TestAGatewayRouteCarriesThePolicyOfTheProtoMethodItRenders(t *testing.T) { t.Fatalf("a document declaring the proto's own policy: %v", err) } } + +func TestSelectedForeignPolicyOnlyJoinsItsServiceTable(t *testing.T) { + dir := policyPackage(t, false) + raw := []byte(`schema: codefly/module-contract/v1 +principal: console +namespaces: [] +queues: [] +scope_ceilings: [{resource_kind: console.spans, actions: [export]}] +bindings: [] +foreign_methods: + - service: api + method: /foreign.v1.Service/Export + requires: [{resource_kind: console.spans, actions: [export]}] +`) + file := filepath.Join(dir, modulecontract.FileName) + if err := os.WriteFile(file, raw, 0600); err != nil { + t.Fatal(err) + } + selected, ok, err := ServiceMethodPolicies(dir) + if err != nil || !ok { + t.Fatalf("selected: %v %v", ok, err) + } + if len(selected.Foreign) != 0 { + t.Fatal("selected method leaked into every service table") + } + found := false + for _, m := range selected.ByService["api"] { + if m.GetMethod() == "/foreign.v1.Service/Export" { + found = true + } + } + if !found { + t.Fatal("selected service lost foreign method") + } + filtered, err := selected.Table(func(service string) bool { return service != "api" }) + if err != nil { + t.Fatal(err) + } + for _, method := range filtered { + if method.GetMethod() == "/foreign.v1.Service/Export" { + t.Fatal("selected foreign method escaped service filtering") + } + } + legacy := strings.Replace(string(raw), " - service: api\n method:", " - method:", 1) + if err := os.WriteFile(file, []byte(legacy), 0600); err != nil { + t.Fatal(err) + } + previous, ok, err := ServiceMethodPolicies(dir) + if err != nil || !ok { + t.Fatalf("legacy: %v %v", ok, err) + } + selectedAll, err := selected.All() + if err != nil { + t.Fatal(err) + } + previousAll, err := previous.All() + if err != nil { + t.Fatal(err) + } + if len(selectedAll) != len(previousAll) { + t.Fatal("selection changed module policy set") + } + for i, m := range selectedAll { + if !proto.Equal(m, previousAll[i]) { + t.Fatal("selection changed authority") + } + } +} diff --git a/pkg/orchestration/hop_configurations.go b/pkg/orchestration/hop_configurations.go index f33d52c4..79931136 100644 --- a/pkg/orchestration/hop_configurations.go +++ b/pkg/orchestration/hop_configurations.go @@ -245,8 +245,8 @@ func publishedHopEndpoints(ctx context.Context, module *resources.Module) ([]rou // from a protocol the module does not own, so no published contract of its // declares them — to the listeners that serve them, so the routes hold the // same method set as the policy table the authority receives -// (gitops.ServiceMethodPolicies delivers them). A foreign method names no -// service: it is served by the module's one service publishing protobuf over +// (gitops.ServiceMethodPolicies delivers them). A foreign method may select its service. Without a selection it is served +// by the module's one service publishing protobuf over // gRPC or Connect, on each of those listeners, which the hop treats as one // destination. A module where none or several services do is refused by name // rather than guessed, as is a foreign method a published contract already @@ -262,28 +262,39 @@ func routeForeignMethods(endpoints []routing.RouteEndpoint, foreign []modulecont services = append(services, endpoints[i].Service) } } - if len(services) != 1 { - methods := make([]string, 0, len(foreign)) - for _, method := range foreign { - methods = append(methods, method.Method) + + for _, method := range foreign { + service := method.Service + if service == "" { + if len(services) != 1 { + methods := make([]string, 0, len(foreign)) + for _, m := range foreign { + methods = append(methods, m.Method) + } + slices.Sort(services) + return fmt.Errorf("its module contract declares foreign methods %s, which name no service, and %d services publish gRPC or Connect contracts %v: the hop cannot tell which one serves them", + strings.Join(methods, ", "), len(services), services) + } + service = services[0] } - slices.Sort(services) - return fmt.Errorf("its module contract declares foreign methods %s, which name no service, and %d services publish gRPC or Connect contracts %v: the hop cannot tell which one serves them", - strings.Join(methods, ", "), len(services), services) - } - for i := range endpoints { - endpoint := &endpoints[i] - if !rpc(endpoint) { - continue + if !slices.Contains(services, service) { + return fmt.Errorf("foreign method %s selects service %q, which publishes no gRPC or Connect contract", method.Method, service) + } + // A declared service does not authorize shadowing an owned method anywhere. + for i := range endpoints { + if slices.Contains(endpoints[i].Methods, method.Method) { + return fmt.Errorf("foreign method %s is also declared by the contract %s/%s publishes", method.Method, endpoints[i].Service, endpoints[i].Endpoint) + } } - for _, method := range foreign { - if slices.Contains(endpoint.Methods, method.Method) { - return fmt.Errorf("foreign method %s is also declared by the contract %s/%s publishes", method.Method, endpoint.Service, endpoint.Endpoint) + for i := range endpoints { + endpoint := &endpoints[i] + if rpc(endpoint) && endpoint.Service == service { + endpoint.Methods = append(endpoint.Methods, method.Method) + slices.Sort(endpoint.Methods) } - endpoint.Methods = append(endpoint.Methods, method.Method) } - slices.Sort(endpoint.Methods) } + return nil } diff --git a/pkg/orchestration/hop_rpc_routes_test.go b/pkg/orchestration/hop_rpc_routes_test.go index 5d37f15e..9dffc9c1 100644 --- a/pkg/orchestration/hop_rpc_routes_test.go +++ b/pkg/orchestration/hop_rpc_routes_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "fmt" + modulecontract "github.com/codefly-dev/core/contracts/module" + "os" "path/filepath" "strings" "testing" @@ -147,6 +149,45 @@ func TestHopRoutesRefuseForeignMethodsTheyCannotPlace(t *testing.T) { require.NoError(t, err) } +func TestForeignMethodSelectsOnlyItsPublishedService(t *testing.T) { + method := "/foreign.v1.Service/Read" + endpoints := []routing.RouteEndpoint{ + {Service: "chat", Endpoint: "grpc", Transport: "grpc"}, + {Service: "chat", Endpoint: "connect", Transport: "connect"}, + {Service: "worker", Endpoint: "grpc", Transport: "grpc"}, + {Service: "web", Endpoint: "http", Transport: "http"}, + } + foreign := []modulecontract.ForeignMethod{{Service: "chat", Method: method}} + require.NoError(t, routeForeignMethods(endpoints, foreign)) + require.Equal(t, []string{method}, endpoints[0].Methods) + require.Equal(t, []string{method}, endpoints[1].Methods) + require.Empty(t, endpoints[2].Methods) + require.ErrorContains(t, routeForeignMethods(endpoints, foreign), "also declared") + foreign[0].Service = "missing" + require.ErrorContains(t, routeForeignMethods(endpoints, foreign), "publishes no gRPC or Connect contract") + foreign[0].Service = "web" + require.ErrorContains(t, routeForeignMethods(endpoints, foreign), "publishes no gRPC or Connect contract") +} + +func TestHopRoutesReadForeignServiceSelectionsFromContract(t *testing.T) { + t.Setenv(resources.CodeflyHomeEnv, filepath.Join(t.TempDir(), "home")) + workspace := rpcHopWorkspace(t, []string{"chat", "worker"}, []string{"/foreign.v1.Service/Read", "/foreign.v1.Service/Execute"}) + file := filepath.Join(workspace.Dir(), "modules", "owner", modulecontract.FileName) + raw, err := os.ReadFile(file) + require.NoError(t, err) + body := strings.Replace(string(raw), " - method: /foreign.v1.Service/Read", " - service: chat\n method: /foreign.v1.Service/Read", 1) + body = strings.Replace(body, " - method: /foreign.v1.Service/Execute", " - service: worker\n method: /foreign.v1.Service/Execute", 1) + require.NoError(t, os.WriteFile(file, []byte(body), 0600)) + routes, err := hopRoutesOf(t, workspace) + require.NoError(t, err) + for method, service := range map[string]string{"/foreign.v1.Service/Read": "chat", "/foreign.v1.Service/Execute": "worker"} { + destination, _, err := routes.Resolve("owner", "grpc", method) + require.NoError(t, err) + require.Equal(t, service, destination.Service) + } + +} + // A host that stands a hop on some compositions and not others declares // hop-routes always: saas's gateway is the hop on a cell and stands none where // the composition names no module for it. Core's reader raises "no