From 287afe6b671a5c57d7649be9ecf84ea921d1e2c6 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:41:29 -0400 Subject: [PATCH] fix(run): compose modules behind the hop on a Connect-faced host Five gaps kept a platform-core composition (saas host, authority hop in front of annotations, model, robin, tracing) from running in enforce mode: - Delivery: the composition interface is reached on a Connect endpoint too. A Connect server answers gRPC on the same listener, the rule the hop's routing holds since authority#31, so host.delivery can name saas accounts' Connect face and every manifest carries its module's method policies. Before, the run refused that endpoint ("serves connect, not gRPC") and the host was delivered through the legacy REST route, which carries none: accounts answered "annotations's delivery declares no method policy" to every call. - Hop routing library re-pinned to authority 3afad722 (#26, #31): one service's gRPC and Connect endpoints are one destination, as the hop itself now routes them. - Foreign methods are routed: a module's contract-declared foreign methods (robin's TargetAuthorityService, tracing's OTLP Export) join the routes of its one gRPC/Connect service, so the hop holds the same method set as the policy table the authority receives. A module with several such services is refused by name. - A service's reference to its own endpoint resolves to the address its Init proposes, so --temporary-ports no longer refuses the hop's ENDPOINT. - A workspace-scope generated secret for a group the workspace inherits from a composed workspace is refused unless the group is declared under configuration-extensions: the generated file was an own group replacing the inherited one whole, dropping the hop's MODULES and every route. Co-Authored-By: Claude Opus 5.5 --- cmd/run/generated_secrets.go | 72 +++++++++++ cmd/run/generated_secrets_test.go | 80 ++++++++++++ docs/commands.md | 7 +- docs/module-manifests.md | 8 +- go.mod | 2 +- go.sum | 2 + pkg/orchestration/grpc_endpoint.go | 19 ++- pkg/orchestration/grpc_endpoint_test.go | 51 +++++++- pkg/orchestration/hop_configurations.go | 49 ++++++++ pkg/orchestration/hop_rpc_routes_test.go | 148 +++++++++++++++++++++++ pkg/orchestration/runner.go | 16 ++- pkg/orchestration/self_reference_test.go | 54 +++++++++ 12 files changed, 492 insertions(+), 16 deletions(-) create mode 100644 pkg/orchestration/hop_rpc_routes_test.go create mode 100644 pkg/orchestration/self_reference_test.go diff --git a/cmd/run/generated_secrets.go b/cmd/run/generated_secrets.go index 2c583e22..d494792b 100644 --- a/cmd/run/generated_secrets.go +++ b/cmd/run/generated_secrets.go @@ -3,8 +3,11 @@ package run import ( "context" "fmt" + "slices" "strings" + "github.com/codefly-dev/core/configurations" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" "github.com/codefly-dev/core/resources" "github.com/codefly-dev/cli/cmd/config" @@ -28,9 +31,22 @@ func provisionGeneratedSecrets(ctx context.Context, workspace *resources.Workspa if err != nil { return nil, err } + resource, err := env.Resource() + if err != nil { + return nil, err + } + profiles, err := resource.ConfigurationProfileNames() + if err != nil { + return nil, err + } var written []string for index := range secrets.Generate { generator := &secrets.Generate[index] + if generator.Scope == environments.SecretGeneratorScopeWorkspace { + if err := refuseShadowingAnInheritedGroup(ctx, workspace, profiles, generator); err != nil { + return nil, fmt.Errorf("service-secrets generate[%d]: %w", index, err) + } + } scopes, err := generatorScopes(ctx, workspace, generator) if err != nil { return nil, fmt.Errorf("service-secrets generate[%d]: %w", index, err) @@ -50,6 +66,62 @@ func provisionGeneratedSecrets(ctx context.Context, workspace *resources.Workspa return written, nil } +// refuseShadowingAnInheritedGroup refuses a workspace-scope generator whose +// group this workspace inherits from a workspace it composes and does not +// author itself. The generated file is an own group of that name, and an own +// group replaces the inherited one whole (core's workspace composition) — so +// writing it, or reading one an earlier run wrote, silently drops every key +// the composed workspace supplies: a workspace importing platform-core lost +// the hop's MODULES that way, and with it every hop route. Declaring the group +// in `configuration-extensions` makes the generated keys extend the inherited +// group instead, which is then the composition's own answer. A group the +// workspace authors (an own file holding a key the generator does not write) +// is its author's to replace. +func refuseShadowingAnInheritedGroup(ctx context.Context, workspace *resources.Workspace, profiles []string, generator *environments.EnvironmentSecretGenerator) error { + name := generator.Configuration + if _, extends := workspace.ConfigurationExtensions[name]; extends { + return nil + } + inheritedFrom, err := composedWorkspaceDeclaring(ctx, workspace, profiles, name) + if err != nil || inheritedFrom == "" { + return err + } + own, err := configurations.LoadProfileConfigurations(ctx, workspace.Dir(), "configurations", profiles) + if err != nil { + return err + } + for _, info := range own.Infos { + if info.GetName() != name { + continue + } + for _, value := range info.GetConfigurationValues() { + if !slices.ContainsFunc(generator.Keys, func(key string) bool { return resources.Match(strings.TrimSpace(key), value.GetKey()) }) { + return nil + } + } + } + return fmt.Errorf("workspace %s inherits configuration %q from workspace %s and generates %s into it; a generated file is an own %q group, which replaces the inherited one whole and drops every key %s supplies: declare it under configuration-extensions in %s (`configuration-extensions: {%s: {}}`) so the generated keys extend the inherited group", + workspace.Name, name, inheritedFrom, strings.Join(generator.Keys, ", "), name, inheritedFrom, resources.WorkspaceConfigurationName, name) +} + +// composedWorkspaceDeclaring names the composed workspace, at any depth, whose +// own configurations declare the group, or "" when none does. +func composedWorkspaceDeclaring(ctx context.Context, workspace *resources.Workspace, profiles []string, name string) (string, error) { + for _, child := range workspace.ComposedWorkspaces() { + provided, err := configurations.LoadProfileConfigurations(ctx, child.Dir(), "configurations", profiles) + if err != nil { + return "", err + } + if slices.ContainsFunc(provided.Infos, func(info *basev0.ConfigurationInformation) bool { return info.GetName() == name }) { + return child.Name, nil + } + if found, err := composedWorkspaceDeclaring(ctx, child, profiles, name); err != nil || found != "" { + return found, err + } + } + return "", nil +} + // generatorScopes is the directory each of the generator's configurations // lives under: the workspace's, or each named service's. func generatorScopes(ctx context.Context, workspace *resources.Workspace, generator *environments.EnvironmentSecretGenerator) ([]string, error) { diff --git a/cmd/run/generated_secrets_test.go b/cmd/run/generated_secrets_test.go index 7a8d5eb7..8b01dac2 100644 --- a/cmd/run/generated_secrets_test.go +++ b/cmd/run/generated_secrets_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/codefly-dev/core/configurations" "github.com/codefly-dev/core/resources" "github.com/stretchr/testify/require" @@ -86,3 +87,82 @@ func TestALocalRunWritesNothingForAStoreAndGuessesNoService(t *testing.T) { require.Error(t, err) require.True(t, strings.Contains(err.Error(), "only for the services the generator names"), err.Error()) } + +// importingWorkspace is a workspace composing `base` by path, whose own +// configurations declare the hop group (MODULES, ENDPOINT) the way +// platform-core does; extension and authored add the importing workspace's own +// declarations. +func importingWorkspace(t *testing.T, extension, authored bool) *resources.Workspace { + t.Helper() + root := t.TempDir() + declaration := "name: product\nlayout: modules\nworkspaces:\n - name: base\n path: ../base\nenvironments:\n - name: local\n" + if extension { + declaration += "configuration-extensions:\n hop: {}\n" + } + files := map[string]string{ + "base/workspace.codefly.yaml": "name: base\nlayout: modules\nenvironments:\n - name: local\n", + "base/configurations/local/hop.env": "MODULES=annotations,robin\nENDPOINT=hop:1\n", + "product/.gitignore": "**/configurations/local/*.secret.env\n", + "product/workspace.codefly.yaml": declaration, + } + if authored { + files["product/configurations/local/hop.env"] = "MODULES=robin\n" + } + for name, content := range files { + path := filepath.Join(root, name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + } + cmd := exec.Command("git", "init", "-q") + cmd.Dir = filepath.Join(root, "product") + out, err := cmd.CombinedOutput() + require.NoErrorf(t, err, "git init: %s", out) + workspace, err := resources.LoadWorkspaceFromDir(context.Background(), filepath.Join(root, "product")) + require.NoError(t, err) + return workspace +} + +// A generated key for a group the workspace inherits from a composed +// workspace would make an own group that replaces the inherited one whole — +// the hop would lose MODULES and every route with it — so the run refuses, +// naming the declaration that makes the key extend the inherited group, and +// writes nothing. Declared, the generated key joins the inherited group. A +// group the workspace authors itself is its own to replace. +func TestAGeneratedSecretNeverReplacesAnInheritedGroup(t *testing.T) { + ctx := context.Background() + env := &environments.Environment{Name: "local", ServiceSecrets: &environments.EnvironmentServiceSecrets{ + Generate: []environments.EnvironmentSecretGenerator{{Scope: environments.SecretGeneratorScopeWorkspace, Configuration: "hop", Keys: []string{"PROOF"}}}, + }} + read := func(t *testing.T, workspace *resources.Workspace) map[string]bool { + t.Helper() + provided, err := configurations.ReadWorkspaceConfigurations(ctx, workspace, &resources.Environment{Name: "local"}) + require.NoError(t, err) + keys := map[string]bool{} + for _, info := range provided.Infos { + if info.GetName() == "hop" { + for _, value := range info.GetConfigurationValues() { + keys[value.GetKey()] = true + } + } + } + return keys + } + + refused := importingWorkspace(t, false, false) + _, err := provisionGeneratedSecrets(ctx, refused, env) + require.ErrorContains(t, err, `workspace product inherits configuration "hop" from workspace base and generates PROOF into it`) + require.ErrorContains(t, err, "configuration-extensions: {hop: {}}") + require.NoFileExists(t, filepath.Join(refused.Dir(), "configurations", "local", "hop.secret.env")) + require.Equal(t, map[string]bool{"MODULES": true, "ENDPOINT": true}, read(t, refused), "the inherited group is intact") + + extended := importingWorkspace(t, true, false) + written, err := provisionGeneratedSecrets(ctx, extended, env) + require.NoError(t, err) + require.Equal(t, []string{"hop/PROOF"}, written) + require.Equal(t, map[string]bool{"MODULES": true, "ENDPOINT": true, "PROOF": true}, read(t, extended), "the generated key extends the inherited group") + + authored := importingWorkspace(t, false, true) + written, err = provisionGeneratedSecrets(ctx, authored, env) + require.NoError(t, err) + require.Equal(t, []string{"hop/PROOF"}, written) +} diff --git a/docs/commands.md b/docs/commands.md index 175843f1..d335e7cf 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -323,7 +323,12 @@ configuration does not hold yet to a value minted by the same generator, into the file `codefly config generate` writes (`configurations//.secret.env`, 0600, refused when git would track it), and says which keys it generated, never their values. A held value is never rotated. A `service` generator names -the services it covers; a local run guesses none. An environment whose +the services it covers; a local run guesses none. A `workspace` generator for a +group the workspace inherits from a composed workspace, and does not author, is +refused unless the group is declared under `configuration-extensions`: the +generated file is an own group, which would replace the inherited one whole +(the hop's `MODULES` lost, and every route with it); declared, the generated +keys extend it. An environment whose `service-secrets` names a store keeps its secrets there, and a run writes nothing for it; a render refuses a secret in an environment that names no store, since it carries secrets only as references into one. So a playground diff --git a/docs/module-manifests.md b/docs/module-manifests.md index 246eea3f..56f8f04e 100644 --- a/docs/module-manifests.md +++ b/docs/module-manifests.md @@ -1127,9 +1127,11 @@ run, the CLI is: for its routed endpoints, signs it with the developer key and carries it, with a native credential for account `delivery` in the module's namespace, to `ApplyManifest` of the `codefly.dev/composition` interface the - `host.delivery` endpoint declares in its module's interface endpoints. A - host whose delivery endpoint declares no such interface — the saas host on - main — receives it through the legacy HTTP route instead, POSTed to + `host.delivery` endpoint declares in its module's interface endpoints — + a `grpc` endpoint, or a `connect` one, which answers the gRPC protocol on + the same listener (the saas host's Connect face). A + host whose delivery endpoint declares no such interface (a saas host + delivered at its `rest` endpoint) receives it through the legacy HTTP route instead, POSTed to `/platform/_delivery/manifest`, with the envelope written through its gateway (below); the run logs that reason on every delivery that takes it. A host whose module does not load receives nothing, with the reason; the diff --git a/go.mod b/go.mod index 8c9a7a33..c6db6a2c 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/codefly-dev/authority/libraries/authority-client/go v0.0.0-20261026143932-67b85b14a942 github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261026143932-67b85b14a942 github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 - github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261025214748-c9cef7470d20 + github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 github.com/codefly-dev/core v0.19.7-0.20261026150808-7c0407679119 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 diff --git a/go.sum b/go.sum index ce9402e0..979528e2 100644 --- a/go.sum +++ b/go.sum @@ -111,6 +111,8 @@ github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000 github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93/go.mod h1:XsOFnO2MQdr37vM3ULa0Uwi8ZWptyVx7hRSUyIdaZ1Q= github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261025214748-c9cef7470d20 h1:zokkjvY+C+FhXq9Tuo93wJBVwLxcX4RfHy8U8KeAAW8= github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261025214748-c9cef7470d20/go.mod h1:021MSgdhW9pSlRLcHEryPDXfI/rGY49rJAhe4RbU7BQ= +github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 h1:XmPltOHirB97RopD8WIJe9bwt1wkHAzngKoj8k2XI8U= +github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855/go.mod h1:rfn41uiORQ36RrOeFEjXHPLC9NUi0j14AoYJtcovsn4= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93 h1:4EygH7/J0E6aM7hUPdbQGfit1rsLUNCtWJ8kMh2b9PE= 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.20261026150808-7c0407679119 h1:+Gkwdvx1g0xla5oyHe0OXBtT0NA2E2BqjfGX7r6uhy8= diff --git a/pkg/orchestration/grpc_endpoint.go b/pkg/orchestration/grpc_endpoint.go index 7eb3f0da..f191d947 100644 --- a/pkg/orchestration/grpc_endpoint.go +++ b/pkg/orchestration/grpc_endpoint.go @@ -40,8 +40,11 @@ func grpcTransport(secure bool) credentials.TransportCredentials { // GRPCEndpointTarget resolves a running service's gRPC endpoint as this run // recorded it now — the host-native instance first, as GetAddressForEndpoint -// answers — with the transport its declaration asks for. An endpoint that -// serves another API is refused by name. +// answers — with the transport its declaration asks for. A Connect endpoint is +// one too: a Connect server answers the gRPC protocol on the same listener +// (h2c in plaintext), which is the rule the hop's routing already holds — it +// sends a service's gRPC calls to its Connect endpoint (authority#31). An +// endpoint that serves another API is refused by name. func (flow *Flow) GRPCEndpointTarget(ctx context.Context, module, service, endpoint string) (GRPCTarget, error) { if flow == nil || flow.SharedState == nil { return GRPCTarget{}, fmt.Errorf("cannot resolve %s/%s/%s: the run records no network state", module, service, endpoint) @@ -56,8 +59,14 @@ func (flow *Flow) GRPCEndpointTarget(ctx context.Context, module, service, endpo if declared.GetName() != endpoint { continue } - if declared.GetApi() != standards.GRPC { - return GRPCTarget{}, fmt.Errorf("%s/%s serves %q, not gRPC", unique, endpoint, declared.GetApi()) + var secure bool + switch declared.GetApi() { + case standards.GRPC: + secure = resources.IsGRPC(ctx, declared).GetSecured() + case standards.CONNECT: + secure = declared.GetApiDetails().GetHttp().GetSecured() + default: + return GRPCTarget{}, fmt.Errorf("%s/%s serves %q, which speaks neither gRPC nor Connect", unique, endpoint, declared.GetApi()) } instance := reachableNetworkInstance(mapping.GetInstances()) if instance == nil { @@ -67,7 +76,7 @@ func (flow *Flow) GRPCEndpointTarget(ctx context.Context, module, service, endpo if address == "" { return GRPCTarget{}, fmt.Errorf("%s/%s has no address recorded", unique, endpoint) } - return GRPCTarget{Address: address, Secure: resources.IsGRPC(ctx, declared).GetSecured()}, nil + return GRPCTarget{Address: address, Secure: secure}, nil } return GRPCTarget{}, fmt.Errorf("%s declares no endpoint %s in this run", unique, endpoint) } diff --git a/pkg/orchestration/grpc_endpoint_test.go b/pkg/orchestration/grpc_endpoint_test.go index c5f1ff05..50b96cea 100644 --- a/pkg/orchestration/grpc_endpoint_test.go +++ b/pkg/orchestration/grpc_endpoint_test.go @@ -2,8 +2,14 @@ package orchestration import ( "context" + "errors" + "net/http" + "net/http/httptest" "testing" + "connectrpc.com/connect" + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" "github.com/codefly-dev/core/resources" "github.com/codefly-dev/core/standards" @@ -24,6 +30,8 @@ func TestGRPCEndpointTargetIsTheNativeInstanceWithItsDeclaredTransport(t *testin }}, {Endpoint: grpcEndpoint("secured", true), Instances: []*basev0.NetworkInstance{networkInstance(resources.NetworkAccessNative, "https://localhost:7002")}}, {Endpoint: &basev0.Endpoint{Name: "rest", Api: standards.REST}, Instances: []*basev0.NetworkInstance{networkInstance(resources.NetworkAccessNative, "http://localhost:7003")}}, + {Endpoint: &basev0.Endpoint{Name: "connect", Api: standards.CONNECT, ApiDetails: resources.ToHTTPAPI(&basev0.HttpAPI{})}, Instances: []*basev0.NetworkInstance{networkInstance(resources.NetworkAccessNative, "http://localhost:7004")}}, + {Endpoint: &basev0.Endpoint{Name: "connect-tls", Api: standards.CONNECT, ApiDetails: resources.ToHTTPAPI(&basev0.HttpAPI{Secured: true})}, Instances: []*basev0.NetworkInstance{networkInstance(resources.NetworkAccessNative, "https://localhost:7005")}}, }) ctx := context.Background() target, err := flow.GRPCEndpointTarget(ctx, "platform", "host", "delivery") @@ -32,8 +40,16 @@ func TestGRPCEndpointTargetIsTheNativeInstanceWithItsDeclaredTransport(t *testin target, err = flow.GRPCEndpointTarget(ctx, "platform", "host", "secured") require.NoError(t, err) require.Equal(t, GRPCTarget{Address: "localhost:7002", Secure: true}, target) + // A Connect listener answers gRPC on the same port: a host whose + // composition interface is its Connect face is reached there. + connectTarget, err := flow.GRPCEndpointTarget(ctx, "platform", "host", "connect") + require.NoError(t, err) + require.Equal(t, GRPCTarget{Address: "localhost:7004"}, connectTarget) + connectTarget, err = flow.GRPCEndpointTarget(ctx, "platform", "host", "connect-tls") + require.NoError(t, err) + require.Equal(t, GRPCTarget{Address: "localhost:7005", Secure: true}, connectTarget) _, err = flow.GRPCEndpointTarget(ctx, "platform", "host", "rest") - require.ErrorContains(t, err, "not gRPC") + require.ErrorContains(t, err, "speaks neither gRPC nor Connect") _, err = flow.GRPCEndpointTarget(ctx, "platform", "host", "missing") require.ErrorContains(t, err, "declares no endpoint missing") _, err = flow.GRPCEndpointTarget(ctx, "platform", "other", "delivery") @@ -42,3 +58,36 @@ func TestGRPCEndpointTargetIsTheNativeInstanceWithItsDeclaredTransport(t *testin require.NoError(t, err) require.NoError(t, conn.Close()) } + +// The claim the Connect case rests on, on the wire: the gRPC client the run +// delivers manifests with reaches a Connect handler of the composition +// interface on a plaintext listener, as a host serving its Connect face over +// h2c answers it. +func TestAConnectListenerAnswersTheCompositionClientOverGRPC(t *testing.T) { + handler := connect.NewUnaryHandler(compositionv1.CompositionService_ApplyManifest_FullMethodName, + func(_ context.Context, request *connect.Request[compositionv1.ApplyManifestRequest]) (*connect.Response[compositionv1.ApplyManifestResponse], error) { + if request.Msg.GetCredential() != "delivery-token" { + return nil, connect.NewError(connect.CodeUnauthenticated, errors.New("unexpected credential")) + } + return connect.NewResponse(&compositionv1.ApplyManifestResponse{Generation: 7}), nil + }) + server := httptest.NewUnstartedServer(handler) + server.Config.Protocols = new(http.Protocols) + server.Config.Protocols.SetHTTP1(true) + server.Config.Protocols.SetUnencryptedHTTP2(true) + server.Start() + t.Cleanup(server.Close) + + flow := flowWithNetworkMappings("platform/host", []*basev0.NetworkMapping{ + {Endpoint: &basev0.Endpoint{Name: "connect", Api: standards.CONNECT, ApiDetails: resources.ToHTTPAPI(&basev0.HttpAPI{})}, + Instances: []*basev0.NetworkInstance{networkInstance(resources.NetworkAccessNative, server.URL)}}, + }) + target, err := flow.GRPCEndpointTarget(t.Context(), "platform", "host", "connect") + require.NoError(t, err) + conn, err := target.Dial() + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + out, err := compositionv1.NewCompositionServiceClient(conn).ApplyManifest(t.Context(), &compositionv1.ApplyManifestRequest{Credential: "delivery-token"}) + require.NoError(t, err) + require.Equal(t, uint64(7), out.GetGeneration()) +} diff --git a/pkg/orchestration/hop_configurations.go b/pkg/orchestration/hop_configurations.go index 281bed6f..36a9e6b1 100644 --- a/pkg/orchestration/hop_configurations.go +++ b/pkg/orchestration/hop_configurations.go @@ -209,12 +209,61 @@ func publishedHopEndpoints(ctx context.Context, module *resources.Module) ([]rou slices.Sort(entry.Methods) out = append(out, entry) } + if err := routeForeignMethods(out, contract.ForeignMethods); err != nil { + return nil, err + } slices.SortFunc(out, func(a, b routing.RouteEndpoint) int { return strings.Compare(a.Service+"/"+a.Endpoint, b.Service+"/"+b.Endpoint) }) return out, nil } +// routeForeignMethods adds the module contract's foreign methods — served +// 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 +// 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 +// declares. +func routeForeignMethods(endpoints []routing.RouteEndpoint, foreign []modulecontract.ForeignMethod) error { + if len(foreign) == 0 { + return nil + } + rpc := func(e *routing.RouteEndpoint) bool { return e.Transport == "grpc" || e.Transport == "connect" } + var services []string + for i := range endpoints { + if rpc(&endpoints[i]) && !slices.Contains(services, endpoints[i].Service) { + 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) + } + 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 + } + 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) + } + endpoint.Methods = append(endpoint.Methods, method.Method) + } + slices.Sort(endpoint.Methods) + } + return nil +} + // publishedContract is the package manifest's contract for a catalog // listener, under the same digest: the bytes the authority's table is read from. func publishedContract(manifest *composition.PackageManifest, endpoint *composition.APIContractEndpoint) (*composition.ProvidedAPIContract, error) { diff --git a/pkg/orchestration/hop_rpc_routes_test.go b/pkg/orchestration/hop_rpc_routes_test.go new file mode 100644 index 00000000..87e25281 --- /dev/null +++ b/pkg/orchestration/hop_rpc_routes_test.go @@ -0,0 +1,148 @@ +package orchestration + +import ( + "context" + "encoding/json" + "fmt" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/authority/libraries/hop/go/routing" + "github.com/codefly-dev/core/composition" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" +) + +// rpcHopWorkspace composes module `owner`, each of whose services publishes +// one protobuf contract (package .v1, service Service, method Call) +// on a grpc and a connect endpoint, as robin, tracing, annotations and model +// do, with the foreign methods its module contract declares; and a hop service +// that reads the generated routes. +func rpcHopWorkspace(t *testing.T, services []string, foreign []string) *resources.Workspace { + t.Helper() + contract := "schema: codefly/module-contract/v1\nprincipal: owner\nnamespaces: [owner]\nqueues: []\nscope_ceilings: [{resource_kind: owner.things, actions: [read]}]\nbindings: []\n" + if len(foreign) > 0 { + contract += "foreign_methods:\n" + for _, method := range foreign { + contract += " - method: " + method + "\n requires: [{resource_kind: owner.things, actions: [read]}]\n" + } + } + files := map[string]string{ + "workspace.codefly.yaml": "name: routes\nlayout: modules\nmodules:\n - name: owner\n - name: authority\n", + "modules/owner/module.codefly.yaml": "name: owner\nservices:\n" + bulletList(services), + "modules/owner/module.contract.codefly.yaml": contract, + "modules/authority/module.codefly.yaml": "name: authority\nservices:\n - name: hop\n", + "modules/authority/services/hop/service.codefly.yaml": "name: hop\nversion: 0.0.0\nagent: {kind: runtime::service, name: go-grpc, version: 0.0.16, publisher: codefly.ai}\nworkspace-configuration-dependencies: [hop-routes]\n", + } + catalog := composition.APIContractCatalog{Schema: composition.APIContractCatalogSchema, Package: "owner/owner", Version: "1.0.0"} + manifest := "schema: codefly/module-package/v2\nkind: module-package\nid: owner/owner\nversion: 1.0.0\nminimum-codefly-version: '>=0.3.32 <1.0.0'\nartifact-roots: [contracts, services]\ncontracts: {composition: '>=2.0.0 <3.0.0'}\nservices:\n" + for _, service := range services { + pkg := service + ".v1" + set := &descriptorpb.FileDescriptorSet{File: []*descriptorpb.FileDescriptorProto{{ + Name: proto.String(service + ".proto"), Package: proto.String(pkg), Syntax: proto.String("proto3"), + MessageType: []*descriptorpb.DescriptorProto{{Name: proto.String("Message")}}, + Service: []*descriptorpb.ServiceDescriptorProto{{Name: proto.String("Service"), Method: []*descriptorpb.MethodDescriptorProto{{ + Name: proto.String("Call"), InputType: proto.String("." + pkg + ".Message"), OutputType: proto.String("." + pkg + ".Message"), + }}}}, + }}} + raw, err := proto.Marshal(set) + require.NoError(t, err) + path := "contracts/api/" + service + ".binpb" + digest := composition.APIContractDigest(raw) + files["modules/owner/"+path] = string(raw) + files["modules/owner/services/"+service+"/service.codefly.yaml"] = "name: " + service + "\nversion: 0.0.0\nagent: {kind: runtime::service, name: go-grpc, version: 0.0.16, publisher: codefly.ai}\nendpoints:\n - name: grpc\n api: grpc\n - name: connect\n api: connect\n" + manifest += fmt.Sprintf(" - name: %s\n endpoints: [connect, grpc]\n api-contracts:\n", service) + for _, endpoint := range []string{"connect", "grpc"} { + manifest += fmt.Sprintf(" - endpoint: %s\n kind: protobuf\n package: %s\n path: %s\n digest: %s\n", endpoint, pkg, path, digest) + catalog.Endpoints = append(catalog.Endpoints, composition.APIContractEndpoint{Service: service, Endpoint: endpoint, API: endpoint, Kind: composition.APIContractKindProtobuf, + Package: pkg, Path: path, Digest: digest, Services: composition.ProtobufServices(set, pkg)}) + } + } + raw, err := json.Marshal(catalog) + require.NoError(t, err) + files["modules/owner/"+composition.APIContractCatalogFileName] = string(raw) + files["modules/owner/module.package.codefly.yaml"] = manifest + return writeTempWorkspace(t, files) +} + +func bulletList(names []string) string { + var out strings.Builder + for _, name := range names { + out.WriteString(" - name: " + name + "\n") + } + return out.String() +} + +func hopRoutesOf(t *testing.T, workspace *resources.Workspace) (routing.Routing, error) { + t.Helper() + hop := &basev0.Configuration{Origin: resources.ConfigurationWorkspace, Infos: []*basev0.ConfigurationInformation{{Name: "hop", ConfigurationValues: []*basev0.ConfigurationValue{{Key: "MODULES", Value: "owner"}}}}} + group, err := generatedHopRoutes(context.Background(), workspace, []*basev0.Configuration{hop}) + if err != nil { + return routing.Routing{}, err + } + raw := hopRoutingSnapshot(group) + require.NotEmpty(t, raw) + routes, err := routing.DecodeRouting(raw) + require.NoError(t, err) + return *routes, nil +} + +// A service publishing the same contract on a grpc and a connect endpoint is +// one destination since authority#31: the run plans its routes instead of +// refusing them as ambiguous, and a gRPC call resolves to the Connect +// listener, which serves gRPC too. The hop routing library before #31 refused +// this composition at plan time. +func TestHopRoutesPlanOneServicePublishedOnGRPCAndConnect(t *testing.T) { + t.Setenv(resources.CodeflyHomeEnv, filepath.Join(t.TempDir(), "home")) + routes, err := hopRoutesOf(t, rpcHopWorkspace(t, []string{"chat"}, nil)) + require.NoError(t, err) + require.Len(t, routes.Endpoints, 2) + destination, _, err := routes.Resolve("owner", "grpc", "/chat.v1.Service/Call") + require.NoError(t, err) + require.Equal(t, "connect", destination.Endpoint) + require.Equal(t, "${endpoint:owner/chat/connect}", destination.Address) +} + +// A module's foreign methods — robin's side of annotations' owner protocol, +// tracing's OTLP Export — are routed to its one gRPC/Connect service, so the +// hop has a destination for every method the authority holds a policy for. +func TestHopRoutesCarryTheForeignMethodsOfTheOneRPCService(t *testing.T) { + t.Setenv(resources.CodeflyHomeEnv, filepath.Join(t.TempDir(), "home")) + for name, foreign := range map[string][]string{ + "robin": {"/annotations.v1.TargetAuthorityService/ListKinds", "/annotations.v1.TargetAuthorityService/CheckPinned", "/annotations.v1.TargetAuthorityService/Reach"}, + "tracing": {"/opentelemetry.proto.collector.trace.v1.TraceService/Export", "/opentelemetry.proto.collector.logs.v1.LogsService/Export"}, + } { + t.Run(name, func(t *testing.T) { + routes, err := hopRoutesOf(t, rpcHopWorkspace(t, []string{"chat"}, foreign)) + require.NoError(t, err) + for _, endpoint := range routes.Endpoints { + require.Subset(t, endpoint.Methods, foreign, "%s carries the foreign methods", endpoint.Endpoint) + require.Contains(t, endpoint.Methods, "/chat.v1.Service/Call") + } + for _, method := range foreign { + destination, _, err := routes.Resolve("owner", "grpc", method) + require.NoError(t, err, method) + require.Equal(t, "chat", destination.Service) + require.Equal(t, "connect", destination.Endpoint) + } + }) + } +} + +// A foreign method names no service, so a module with two gRPC/Connect +// services is refused by name rather than routed to a guess; one already +// published by a contract is refused as well. +func TestHopRoutesRefuseForeignMethodsTheyCannotPlace(t *testing.T) { + t.Setenv(resources.CodeflyHomeEnv, filepath.Join(t.TempDir(), "home")) + _, err := hopRoutesOf(t, rpcHopWorkspace(t, []string{"chat", "host"}, []string{"/annotations.v1.TargetAuthorityService/ListKinds"})) + require.ErrorContains(t, err, "hop routes for owner: its module contract declares foreign methods /annotations.v1.TargetAuthorityService/ListKinds, which name no service, and 2 services publish gRPC or Connect contracts [chat host]") + _, err = hopRoutesOf(t, rpcHopWorkspace(t, []string{"chat"}, []string{"/chat.v1.Service/Call"})) + require.ErrorContains(t, err, "foreign method /chat.v1.Service/Call is also declared by the contract chat/") + // Without foreign methods, two services each on both listeners plan. + _, err = hopRoutesOf(t, rpcHopWorkspace(t, []string{"chat", "host"}, nil)) + require.NoError(t, err) +} diff --git a/pkg/orchestration/runner.go b/pkg/orchestration/runner.go index 70b06411..5cdda09f 100644 --- a/pkg/orchestration/runner.go +++ b/pkg/orchestration/runner.go @@ -2103,7 +2103,17 @@ func (runner *Runner) gatherInitInputs(ctx, cfgCtx context.Context, w *wool.Wool if err != nil { return nil, w.Wrapf(err, "cannot create runtime context: <%s>", runner.runtimeContext) } - out.workspaceConfigurations, err = runner.workspaceConfigurations(cfgCtx, out.dependenciesNetworkMappings, out.runtimeContext) + // The addresses this Init proposes come first: a service whose own groups + // reference its own endpoint (the hop's ENDPOINT names the hop) resolves + // that reference to the address it is about to serve. Deriving it again as + // a producer's would allocate a second, different port under + // --temporary-ports, where a derivation is refused for that reason. + out.networkMappings, err = runner.world.LocalNetworkManager.GenerateNetworkMappings(ctx, runner.world.Env.Runtime(), runner.world.Workspace, runner.instance.Identity, runner.endpoints, out.runtimeContext) + if err != nil { + return nil, w.Wrapf(err, "cannot generate network mappings for service endpoints") + } + resolvable := append(slices.Clone(out.dependenciesNetworkMappings), out.networkMappings...) + out.workspaceConfigurations, err = runner.workspaceConfigurations(cfgCtx, resolvable, out.runtimeContext) if err != nil { return nil, runner.initReadFailure(cfgCtx, w, err, "workspace dependencies configurations") } @@ -2111,10 +2121,6 @@ func (runner *Runner) gatherInitInputs(ctx, cfgCtx context.Context, w *wool.Wool if err != nil { return nil, runner.initReadFailure(cfgCtx, w, err, "dependencies configurations") } - out.networkMappings, err = runner.world.LocalNetworkManager.GenerateNetworkMappings(ctx, runner.world.Env.Runtime(), runner.world.Workspace, runner.instance.Identity, runner.endpoints, out.runtimeContext) - if err != nil { - return nil, w.Wrapf(err, "cannot generate network mappings for service endpoints") - } return out, nil } diff --git a/pkg/orchestration/self_reference_test.go b/pkg/orchestration/self_reference_test.go new file mode 100644 index 00000000..3163f207 --- /dev/null +++ b/pkg/orchestration/self_reference_test.go @@ -0,0 +1,54 @@ +package orchestration + +import ( + "testing" + + "github.com/codefly-dev/core/resources" + coreservices "github.com/codefly-dev/core/services" + "github.com/codefly-dev/core/wool" + "github.com/stretchr/testify/require" +) + +// A service whose own group references its own endpoint — the hop's ENDPOINT +// names the hop — reads the address its Init proposes, under +// --temporary-ports as well: that address is allocated by this Init, so it is +// known here, where deriving it as another producer's would allocate a second +// port and is refused. +func TestASelfReferenceResolvesToTheAddressThisInitProposes(t *testing.T) { + for _, temporary := range []bool{false, true} { + t.Run(map[bool]string{false: "named-ports", true: "temporary-ports"}[temporary], func(t *testing.T) { + workspace := writeTempWorkspace(t, map[string]string{ + "workspace.codefly.yaml": "name: bare\nlayout: modules\nmodules:\n - name: payments\n", + "modules/payments/module.codefly.yaml": "kind: module\nname: payments\nservices:\n - name: worker\n", + "configurations/local/hop.env": "ENDPOINT=${endpoint:payments/worker/grpc}\n", + "modules/payments/services/worker/service.codefly.yaml": "kind: service\nname: worker\nversion: 0.0.0\nmodule: payments\n" + + "agent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.16\n publisher: codefly.ai\n" + + "endpoints:\n - name: grpc\n api: grpc\n" + + "workspace-configuration-dependencies:\n - hop\n", + }) + world, service := referenceValidityWorld(t, workspace, func(world *World) { world.Mode = RunMode }) + if temporary { + world.temporaryPorts = true + world.LocalNetworkManager.WithTemporaryPorts() + } + identity, err := service.Identity() + require.NoError(t, err) + endpoints, err := service.LoadEndpoints(t.Context()) + require.NoError(t, err) + require.NoError(t, world.SharedState.RecordEndpoints(t.Context(), identity, endpoints)) + runner := &Runner{ + world: world, + instance: &coreservices.Instance{Service: service, Identity: identity}, + runtimeContext: resources.RuntimeContextNative, + endpoints: endpoints, + } + inputs, err := runner.gatherInitInputs(t.Context(), t.Context(), wool.Get(t.Context())) + require.NoError(t, err) + value, found := groupValue(inputs.workspaceConfigurations, "hop", "ENDPOINT") + require.True(t, found) + proposed, err := resources.FindNetworkInstanceInNetworkMappings(t.Context(), inputs.networkMappings, endpoints[0], resources.NewNativeNetworkAccess()) + require.NoError(t, err) + require.Equal(t, proposed.GetAddress(), value, "the reference reads the address this Init proposes") + }) + } +}