diff --git a/cmd/deploy/gitops.go b/cmd/deploy/gitops.go index 23708fc6..bb235a05 100644 --- a/cmd/deploy/gitops.go +++ b/cmd/deploy/gitops.go @@ -14,6 +14,7 @@ import ( "github.com/codefly-dev/cli/pkg/cli" "github.com/codefly-dev/cli/pkg/cli/models" "github.com/codefly-dev/cli/pkg/control" + "github.com/codefly-dev/cli/pkg/environments" "github.com/codefly-dev/cli/pkg/gitops" "github.com/codefly-dev/cli/pkg/orchestration" "github.com/codefly-dev/core/resources" @@ -693,8 +694,35 @@ func printCell(cell *gitops.CellResult) { var gitOpsRenderAll bool +// compositionRenderOrder is the order renderComposition renders modules in — +// the host's module first, then the rest by name — less every module the +// environment withdraws whole, which it returns by name: nothing of such a +// module renders in this environment. +func compositionRenderOrder(env *environments.Environment, modules []*resources.Module) (rendered []*resources.Module, withdrawn []string) { + host := "" + if env.Host != nil { + host, _, _ = env.Host.DeliveryEndpoint() + } + for _, module := range modules { + if env.WithdrawsModule(module) { + withdrawn = append(withdrawn, module.Name) + continue + } + rendered = append(rendered, module) + } + sort.SliceStable(rendered, func(i, j int) bool { + if (rendered[i].Name == host) != (rendered[j].Name == host) { + return rendered[i].Name == host + } + return rendered[i].Name < rendered[j].Name + }) + sort.Strings(withdrawn) + return rendered, withdrawn +} + // renderComposition renders every module the workspace composes for the -// selected environment — the host module first, since every other module's +// selected environment — a module the environment withdraws whole is not one +// of them — the host module first, since every other module's // tree is read against it, then the rest in name order — and derives the // environment's cell file once, from all of them. One command for one // composition: a stale sibling is never a reason to run it again. @@ -718,17 +746,11 @@ func renderComposition(ctx context.Context) error { if err != nil { return err } - host := "" - if env.Host != nil { - host, _, _ = env.Host.DeliveryEndpoint() + rendered, withdrawn := compositionRenderOrder(env, modules) + for _, name := range withdrawn { + cli.Info("Skipped %s: environment %s withdraws every service of it (exclude-services)", name, env.Name) } - sort.SliceStable(modules, func(i, j int) bool { - if (modules[i].Name == host) != (modules[j].Name == host) { - return modules[i].Name == host - } - return modules[i].Name < modules[j].Name - }) - for _, module := range modules { + for _, module := range rendered { if err = requireWorkspaceReady(ctx, "render", gitOpsEnv, module.Name); err != nil { return err } diff --git a/cmd/deploy/render_order_test.go b/cmd/deploy/render_order_test.go new file mode 100644 index 00000000..2c424c61 --- /dev/null +++ b/cmd/deploy/render_order_test.go @@ -0,0 +1,45 @@ +package deploy + +import ( + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +// A composition renders the host's module first, then the rest by name, and +// leaves out every module the environment withdraws whole: a module with one +// service still composed renders as before. +func TestCompositionRenderOrderLeavesOutAWithdrawnModule(t *testing.T) { + module := func(name string, services ...string) *resources.Module { + m := &resources.Module{Name: name} + for _, service := range services { + m.ServiceReferences = append(m.ServiceReferences, &resources.ServiceReference{Name: service}) + } + return m + } + env := &environments.Environment{ + Name: "staging", + Host: &environments.EnvironmentHost{Delivery: "saas/accounts/connect"}, + ExcludeServices: []environments.EnvironmentExcludedService{ + {Service: "authority/local"}, {Service: "authority/hop"}, + {Service: "runtime/witness"}, + }, + } + modules := []*resources.Module{ + module("runtime", "tasks", "witness"), + module("authority", "local", "hop"), + module("documents", "documents"), + module("saas", "accounts"), + } + + rendered, withdrawn := compositionRenderOrder(env, modules) + + var names []string + for _, m := range rendered { + names = append(names, m.Name) + } + require.Equal(t, []string{"saas", "documents", "runtime"}, names) + require.Equal(t, []string{"authority"}, withdrawn) +} diff --git a/docs/configuration-contract.md b/docs/configuration-contract.md index a99edb4f..d0bb05fc 100644 --- a/docs/configuration-contract.md +++ b/docs/configuration-contract.md @@ -175,6 +175,27 @@ environments: withdrawn-from: [saas/accounts] ``` +Nothing may still reach what is withdrawn: a service that declares a +dependency on it is refused unless `withdrawn-from` names it, and so are a +solution consuming one of its endpoints, an environment declaration naming it +and a configuration reference to one of its endpoints. A service the module's +`interface:` exports, or names its `service-entry`, is withdrawn only with the +whole module: an environment that names every service of a module leaves the +module out, and its exports leave with it. Withdrawing some of its services is +refused, because the module the environment still composes would promise an +endpoint that is not there. + +```yaml +environments: + - name: staging # the host is another module's service here + exclude-services: + - service: authority/local + - service: authority/hop + withdrawn-from: [documents/documents] + - service: authority/edge + - service: authority/store +``` + A product whose hosted environment is imported can keep this decision in an **authored** `delivery.environments.yaml` and pass it alongside the producer's contract: diff --git a/pkg/environments/excluded_services.go b/pkg/environments/excluded_services.go index d38ec550..87c0a744 100644 --- a/pkg/environments/excluded_services.go +++ b/pkg/environments/excluded_services.go @@ -27,6 +27,12 @@ import ( // service-secrets, managed-services) is refused, and so is a workspace // configuration reference to one of its endpoints, at plan time: such a value // would resolve to nothing. +// +// A service the module's interface exports, or names its service-entry, is +// withdrawn only with the whole module: an environment that excludes every +// service of a module leaves the module out, its exports with it, while one +// that excludes only some of them is refused, because the module it still +// composes would promise an endpoint that is not there. type EnvironmentExcludedService struct { // Service is the "/" withdrawn; never a bare name, so one // entry can only ever withdraw one service. @@ -77,6 +83,22 @@ func (env *Environment) ComposedServices(module *resources.Module) []*resources. return composed } +// WithdrawsModule reports whether this environment excludes every service of +// the module: the module is then not composed here at all, so neither its +// interface exports nor its service-entry are offered to the composition, and +// nothing of it renders, builds or deploys here. +func (env *Environment) WithdrawsModule(module *resources.Module) bool { + if env == nil || module == nil || len(module.ServiceReferences) == 0 { + return false + } + for _, reference := range module.ServiceReferences { + if reference != nil && !env.Excludes(module.Name, reference.Name) { + return false + } + } + return true +} + // ExcludedServiceUniques returns the "/" this environment // withdraws, sorted: the form architecture.ExcludeServices keys on. func (env *Environment) ExcludedServiceUniques() []string { @@ -249,24 +271,8 @@ func (env *Environment) validateExclusionsAgainst(graph *exclusionGraph) error { } } } - // A module interface exporting one of its endpoints, or naming it the - // module's entry, promises the composition what this environment does - // not have; so does a solution consuming one of its endpoints. - for _, module := range modules { - if module == nil { - continue - } - if module.ServiceEntry != "" && excluded[resources.ServiceUnique(module.Name, module.ServiceEntry)] { - return fmt.Errorf("module %s names %s/%s its service-entry, which exclude-services withdraws from this environment", module.Name, module.Name, module.ServiceEntry) - } - if module.Interface == nil { - continue - } - for _, exported := range module.Interface.Endpoints { - if exported != nil && excluded[resources.ServiceUnique(module.Name, exported.Service)] { - return fmt.Errorf("module %s exports %s/%s/%s through its interface, which exclude-services withdraws from this environment", module.Name, module.Name, exported.Service, exported.Endpoint) - } - } + if err := env.validateModulePromises(modules, excluded); err != nil { + return err } for index := range graph.consumed { consumption := &graph.consumed[index] @@ -290,6 +296,33 @@ func (env *Environment) validateExclusionsAgainst(graph *exclusionGraph) error { return nil } +// validateModulePromises refuses withdrawing a service a module still promises +// the composition: one its interface exports, or names its service-entry. A +// module interface exporting one of its endpoints, or naming it the module's +// entry, promises the composition what this environment does not have. A +// module this environment withdraws whole makes no promise here: its exports +// and its entry leave with it, and whatever still reached them is a consumer +// validateExclusionsAgainst refuses by name. +func (env *Environment) validateModulePromises(modules []*resources.Module, excluded map[string]bool) error { + for _, module := range modules { + if module == nil || env.WithdrawsModule(module) { + continue + } + if module.ServiceEntry != "" && excluded[resources.ServiceUnique(module.Name, module.ServiceEntry)] { + return fmt.Errorf("module %s names %s/%s its service-entry, which exclude-services withdraws from this environment: exclude every service of module %s to leave the module out, or stop excluding %s/%s", module.Name, module.Name, module.ServiceEntry, module.Name, module.Name, module.ServiceEntry) + } + if module.Interface == nil { + continue + } + for _, exported := range module.Interface.Endpoints { + if exported != nil && excluded[resources.ServiceUnique(module.Name, exported.Service)] { + return fmt.Errorf("module %s exports %s/%s/%s through its interface, which exclude-services withdraws from this environment: exclude every service of module %s to leave the module out, or stop excluding %s/%s", module.Name, module.Name, exported.Service, exported.Endpoint, module.Name, module.Name, exported.Service) + } + } + } + return nil +} + type namedService struct{ block, name string } // serviceDeclarations lists every service the environment's own declarations diff --git a/pkg/environments/excluded_services_test.go b/pkg/environments/excluded_services_test.go index 00b7e1d6..48bf62ea 100644 --- a/pkg/environments/excluded_services_test.go +++ b/pkg/environments/excluded_services_test.go @@ -157,6 +157,73 @@ func TestExcludeServicesRefusesAnInterfaceExportOfIt(t *testing.T) { require.ErrorContains(t, err, "module shop exports shop/cache/tcp through its interface") } +// An environment that excludes every service of a module leaves the module +// out: its interface exports and its service-entry leave with it, so the +// refusals above do not hold, while a consumer outside the module still does. +func TestExcludeServicesWithdrawsAWholeModuleWithItsExports(t *testing.T) { + const wholeShop = ` exclude-services: + - service: shop/api + - service: shop/vault + - service: shop/cache +` + exporting := "service-entry: cache\ninterface:\n endpoints:\n - service: cache\n endpoint: tcp\n visibility: internal\n" + workspace := writeExclusionWorkspaceExporting(t, wholeShop, exporting) + require.NoError(t, environments.ValidateWorkspace(context.Background(), workspace)) + + staging, err := environments.Select(workspace, "staging") + require.NoError(t, err) + module, err := workspace.LoadModuleFromName(context.Background(), "shop") + require.NoError(t, err) + require.Empty(t, staging.ComposedServices(module)) + require.True(t, staging.WithdrawsModule(module)) + local, err := environments.Select(workspace, "local") + require.NoError(t, err) + require.Len(t, local.ComposedServices(module), 3, "another environment still composes the module") + require.False(t, local.WithdrawsModule(module)) + + // Leaving one service of the module composed keeps its promise in force. + partial := writeExclusionWorkspaceExporting(t, ` exclude-services: + - service: shop/vault + withdrawn-from: [shop/api] + - service: shop/cache +`, exporting) + err = environments.ValidateWorkspace(context.Background(), partial) + require.ErrorContains(t, err, "exclude every service of module shop to leave the module out") + + // A consumer in another module is still a consumer of what left. + consumed := writeExclusionWorkspaceExporting(t, wholeShop, exporting) + root := consumed.Dir() + data, err := os.ReadFile(filepath.Join(root, resources.WorkspaceConfigurationName)) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(root, resources.WorkspaceConfigurationName), + []byte(strings.Replace(string(data), " - name: shop\n", " - name: shop\n - name: portal\n", 1)), 0o644)) + for rel, content := range map[string]string{ + "modules/portal/module.codefly.yaml": "kind: module\nname: portal\nservices:\n - name: web\n", + "modules/portal/services/web/service.codefly.yaml": `kind: service +name: web +version: 0.0.0 +agent: + kind: runtime::service + name: go-grpc + version: 0.0.1 + publisher: codefly.ai +service-dependencies: + - name: cache + module: shop + endpoints: + - name: tcp +`, + } { + full := filepath.Join(root, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(content), 0o644)) + } + reloaded, err := resources.LoadWorkspaceFromDir(context.Background(), root) + require.NoError(t, err) + err = environments.ValidateWorkspace(context.Background(), reloaded) + require.ErrorContains(t, err, `exclude-services withdraws "shop/cache", but portal/web declares a dependency on it`) +} + // A module naming the excluded service its entry routes the host to a service // that is not there. func TestExcludeServicesRefusesTheModuleServiceEntry(t *testing.T) { diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index a202e820..3e6b09a0 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -161,7 +161,7 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro if len(file.Namespaces) == 0 { return cellfile.File{}, CellResult{}, fmt.Errorf("no module is rendered for environment %s under %s", env.Name, modulesDir) } - if err = refuseUnrenderedModules(workspace, env, result.Modules); err != nil { + if err = refuseUnrenderedModules(ctx, workspace, env, result.Modules); err != nil { return cellfile.File{}, CellResult{}, err } sort.Slice(file.Namespaces, func(i, j int) bool { return file.Namespaces[i].Name < file.Namespaces[j].Name }) @@ -188,12 +188,18 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro // claim about a composition that may no longer exist, and the file it was // read from is as likely to have been cleaned as the records were. No digest // is invented either: the render of the module is what knows it. -func refuseUnrenderedModules(workspace *resources.Workspace, env *environments.Environment, rendered []string) error { +func refuseUnrenderedModules(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, rendered []string) error { var missing []string for _, reference := range workspace.Modules { - if reference != nil && !slices.Contains(rendered, reference.Name) { - missing = append(missing, reference.Name) + if reference == nil || slices.Contains(rendered, reference.Name) { + continue + } + // A module the environment withdraws whole has no render here and + // owes none (render refuses it): it delivers nothing to the cell. + if module, err := workspace.LoadModuleFromName(ctx, reference.Name); err == nil && env.WithdrawsModule(module) { + continue } + missing = append(missing, reference.Name) } if len(missing) == 0 { return nil @@ -643,7 +649,9 @@ func endpointConsumers(ctx context.Context, workspace *resources.Workspace, env for index := range consumed { consumption := &consumed[index] module, loadErr := workspace.LoadModuleFromName(ctx, consumption.Solution) - if loadErr != nil || module.ServiceEntry == "" { + // A solution the environment withdraws whole has no entry here to + // consume anything. + if loadErr != nil || module.ServiceEntry == "" || env.WithdrawsModule(module) { continue } add(consumption.Module+"/"+consumption.Service+"/"+consumption.Endpoint, resources.ServiceUnique(module.Name, module.ServiceEntry)) diff --git a/pkg/gitops/excluded_services_test.go b/pkg/gitops/excluded_services_test.go index 44ee1dd2..f0faec6e 100644 --- a/pkg/gitops/excluded_services_test.go +++ b/pkg/gitops/excluded_services_test.go @@ -62,6 +62,33 @@ func TestRenderModuleLeavesAnExcludedServiceOutOfTheTreeAndTheInventory(t *testi `rendered unit "store" is a service environment "staging" excludes`) } +// A module the environment withdraws whole renders nothing there: the render +// refuses it by name instead of producing an empty tree. +func TestRenderModuleRefusesAModuleTheEnvironmentWithdrawsWhole(t *testing.T) { + installFakeAgents(t) + root := t.TempDir() + require.NoError(t, copyTree(filepath.Join("testdata", "container-ports"), root)) + path := filepath.Join(root, resources.WorkspaceConfigurationName) + data, err := os.ReadFile(path) + require.NoError(t, err) + data = append(data, []byte(` exclude-services: + - service: shop/api + - service: shop/store +`)...) + require.NoError(t, os.WriteFile(path, data, 0o644)) + ctx := context.Background() + workspace, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + module, err := workspace.LoadModuleFromName(ctx, "shop") + require.NoError(t, err) + env, err := orchestration.SelectEnvironment(workspace, "staging") + require.NoError(t, err) + require.True(t, env.WithdrawsModule(module)) + + _, err = RenderModule(ctx, workspace, module, env, "acme-staging", nil) + require.ErrorContains(t, err, "environment staging withdraws every service of module shop (exclude-services)") +} + // withdrawnCellWorkspace is the cell fixture with staging excluding shop/api // and withdrawing billing/ledger's edge to it — and, as the exclusion // requires, with no ingress, egress or cell declaration naming shop/api left. @@ -87,6 +114,24 @@ func TestAWithdrawnEdgeIsNoCellConsumer(t *testing.T) { require.Empty(t, consumers["shop/api/grpc"]) } +// A module the environment withdraws whole owes the cell file no render: the +// file is derived from the other modules' trees and describes nothing of it. +func TestCellFileLeavesOutAModuleTheEnvironmentWithdrawsWhole(t *testing.T) { + workspace := withdrawnCellWorkspace(t) + env := selectedEnvironment(t, workspace, "staging") + shop, err := workspace.LoadModuleFromName(context.Background(), "shop") + require.NoError(t, err) + require.True(t, env.WithdrawsModule(shop), "the fixture withdraws every service of shop") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + + result, err := RenderCell(context.Background(), workspace, env) + require.NoError(t, err) + require.Equal(t, []string{"billing"}, result.Modules) + file, err := os.ReadFile(result.Path) + require.NoError(t, err) + require.NotContains(t, string(file), "acme-shop") +} + // A cell file is never derived from a tree that still delivers an excluded // service: the render of that module is owed first. func TestCellFileRefusesATreeThatStillDeliversAnExcludedService(t *testing.T) { diff --git a/pkg/gitops/orchestrate.go b/pkg/gitops/orchestrate.go index 5e135478..45ae2375 100644 --- a/pkg/gitops/orchestrate.go +++ b/pkg/gitops/orchestrate.go @@ -49,6 +49,9 @@ func renderModuleTree( if err := environments.ValidateWorkspace(ctx, workspace); err != nil { return RenderResult{}, err } + if env.WithdrawsModule(module) { + return RenderResult{}, fmt.Errorf("environment %s withdraws every service of module %s (exclude-services), so nothing of it renders there", env.Name, module.Name) + } if err := CheckCompositionPermissions(ctx, workspace, env, true, module.Name); err != nil { return RenderResult{}, err }