fix(environments): an environment withdraws a whole composed module, its interface exports with it (#1157)

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
 		}

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)
+}

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:

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 "<module>/<service>" 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 "<module>/<service>" 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

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) {

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))