From 4d6bb7b8b9f3fa88bf57c8ebd7a66e7ce947358b Mon Sep 17 00:00:00 2001 From: Jon Tucci Date: Wed, 18 Mar 2026 10:09:24 -0700 Subject: [PATCH 1/2] Support CROSSPLANE_DIFF_DOCKER_NETWORK env var for container networking When crossplane-diff runs inside a Docker container (e.g. a GitHub Actions container job), function containers created via the Docker socket land on a different Docker network and are unreachable. This adds support for the CROSSPLANE_DIFF_DOCKER_NETWORK environment variable. When set, the value is applied as the render.crossplane.io/runtime-docker-network annotation on all function resources before they are passed to render.Render(). This tells the Crossplane render runtime to connect function containers to the specified Docker network, making them reachable from the caller. Usage in a GitHub Actions workflow: env: CROSSPLANE_DIFF_DOCKER_NETWORK: ${{ job.container.network }} Depends on: crossplane/crossplane#7216 Signed-off-by: Jon Tucci --- cmd/diff/diffprocessor/function_provider.go | 34 ++++++++++ .../diffprocessor/function_provider_test.go | 68 +++++++++++++++++++ go.mod | 2 +- go.sum | 4 +- 4 files changed, 105 insertions(+), 3 deletions(-) diff --git a/cmd/diff/diffprocessor/function_provider.go b/cmd/diff/diffprocessor/function_provider.go index 7f3dd150..487dee7f 100644 --- a/cmd/diff/diffprocessor/function_provider.go +++ b/cmd/diff/diffprocessor/function_provider.go @@ -22,6 +22,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "os" "strings" "time" @@ -48,6 +49,35 @@ type FunctionProvider interface { Cleanup(ctx context.Context) error } +// EnvDockerNetwork is the environment variable that specifies which Docker +// network function containers should join. This is needed when crossplane-diff +// runs inside a Docker container (e.g. a GitHub Actions container job) so that +// function containers are on the same network and reachable via container IP. +const EnvDockerNetwork = "CROSSPLANE_DIFF_DOCKER_NETWORK" + +// annotationRuntimeDockerNetwork is the render annotation that configures the +// Docker network for function containers. +const annotationRuntimeDockerNetwork = "render.crossplane.io/runtime-docker-network" + +// applyDockerNetworkAnnotation sets the Docker network annotation on functions +// if the CROSSPLANE_DIFF_DOCKER_NETWORK environment variable is set. +func applyDockerNetworkAnnotation(fns []pkgv1.Function, log logging.Logger) { + network := os.Getenv(EnvDockerNetwork) + if network == "" { + return + } + + log.Debug("Setting Docker network annotation on functions", "network", network) + + for i := range fns { + if fns[i].Annotations == nil { + fns[i].Annotations = make(map[string]string) + } + + fns[i].Annotations[annotationRuntimeDockerNetwork] = network + } +} + // DefaultFunctionProvider fetches functions from the cluster on each call. // This is appropriate for the xr command where each XR is processed independently. type DefaultFunctionProvider struct { @@ -74,6 +104,8 @@ func (p *DefaultFunctionProvider) GetFunctionsForComposition(comp *apiextensions p.logger.Debug("Fetched functions from pipeline", "composition", comp.GetName(), "count", len(fns)) + applyDockerNetworkAnnotation(fns, p.logger) + return fns, nil } @@ -168,6 +200,8 @@ func (p *CachedFunctionProvider) GetFunctionsForComposition(comp *apiextensionsv // Cache for future calls p.cache[compName] = fns + applyDockerNetworkAnnotation(fns, p.logger) + return fns, nil } diff --git a/cmd/diff/diffprocessor/function_provider_test.go b/cmd/diff/diffprocessor/function_provider_test.go index bc142960..94c32ee7 100644 --- a/cmd/diff/diffprocessor/function_provider_test.go +++ b/cmd/diff/diffprocessor/function_provider_test.go @@ -17,6 +17,7 @@ limitations under the License. package diffprocessor import ( + "os" "strings" "testing" @@ -671,3 +672,70 @@ func TestGenerateContainerName(t *testing.T) { }) } } + +func TestApplyDockerNetworkAnnotation(t *testing.T) { + tests := map[string]struct { + envValue string + fns []pkgv1.Function + wantNetwork string + }{ + "EnvSet": { + envValue: "github_network_abc123", + fns: []pkgv1.Function{ + {ObjectMeta: metav1.ObjectMeta{Name: "function-1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "function-2"}}, + }, + wantNetwork: "github_network_abc123", + }, + "EnvNotSet": { + envValue: "", + fns: []pkgv1.Function{ + {ObjectMeta: metav1.ObjectMeta{Name: "function-1"}}, + }, + wantNetwork: "", + }, + "ExistingAnnotations": { + envValue: "my-network", + fns: []pkgv1.Function{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "function-1", + Annotations: map[string]string{ + "existing-key": "existing-value", + }, + }, + }, + }, + wantNetwork: "my-network", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + if tt.envValue != "" { + t.Setenv(EnvDockerNetwork, tt.envValue) + } else { + os.Unsetenv(EnvDockerNetwork) + } + + logger := tu.TestLogger(t, false) + applyDockerNetworkAnnotation(tt.fns, logger) + + for _, fn := range tt.fns { + got := fn.Annotations[annotationRuntimeDockerNetwork] + if got != tt.wantNetwork { + t.Errorf("function %q: network annotation = %q, want %q", fn.Name, got, tt.wantNetwork) + } + } + + // Verify existing annotations are preserved when env is set + if tt.wantNetwork != "" { + for _, fn := range tt.fns { + if v, ok := fn.Annotations["existing-key"]; ok && v != "existing-value" { + t.Errorf("existing annotation was modified: got %q, want %q", v, "existing-value") + } + } + } + }) + } +} diff --git a/go.mod b/go.mod index 73b0cb72..3d246e10 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( dario.cat/mergo v1.0.2 github.com/Masterminds/semver v1.5.0 github.com/alecthomas/kong v1.15.0 - github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260615182009-ba59fbfac34b + github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260617170926-a416505fb016 github.com/crossplane/crossplane-runtime/v2 v2.4.0-rc.0 github.com/crossplane/crossplane/apis/v2 v2.4.0-rc.0 github.com/crossplane/crossplane/v2 v2.3.2 diff --git a/go.sum b/go.sum index 77dadda5..ac7a63d9 100644 --- a/go.sum +++ b/go.sum @@ -47,8 +47,8 @@ github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260615182009-ba59fbfac34b h1:Ol4CZMZvcS/m3IYRz4YceZGj6pCF1TLyN+nTpgMgDPg= -github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260615182009-ba59fbfac34b/go.mod h1:TVfHZdpkSlrfkE6V8VLlTOS/DT4Qsxc+ybMcUnZz3ko= +github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260617170926-a416505fb016 h1:UY6gCUfSLuoG4g8WUy9+tYOCXXCPFy9ootsQ+OL2LBk= +github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260617170926-a416505fb016/go.mod h1:TVfHZdpkSlrfkE6V8VLlTOS/DT4Qsxc+ybMcUnZz3ko= github.com/crossplane/crossplane-runtime/v2 v2.4.0-rc.0 h1:Zgiq+hrh9lbjWtv8ECCLd1A0I9knt3c8ZUELExw6M1w= github.com/crossplane/crossplane-runtime/v2 v2.4.0-rc.0/go.mod h1:PAo3zIfmMzrS18HGyHJLXCeXIp0nFW2Md2Fn9gocMaU= github.com/crossplane/crossplane/apis/v2 v2.4.0-rc.0 h1:4PBahj+tnK9RwSZm1bYGvOkHOU+1CSHjJF2PoPzBMD0= From 128b59272fd0c2c3939b2c94ba24e4361dd188bb Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Wed, 17 Jun 2026 17:04:25 -0400 Subject: [PATCH 2/2] fix(function-provider): apply docker-network annotation on cache hits Address review feedback on PR #255: - CachedFunctionProvider.GetFunctionsForComposition now applies the Docker network annotation on every call, including cache hits, so the annotation always reflects the current value of CROSSPLANE_DIFF_DOCKER_NETWORK (rather than whatever was in effect when the entry was first cached). Reorder the cache-miss path to annotate before storing, so cache state is correct independent of slice aliasing. - Tighten TestApplyDockerNetworkAnnotation: use t.Setenv even for the empty case so prior env state is restored on cleanup, and assert the pre-existing annotation key is both present and unchanged (the prior check only failed on a wrong value, not on accidental removal). - Add TestCachedFunctionProvider_DockerNetworkAnnotation_CacheHit as a regression test for the cache-hit path: prime the cache with the env var unset, then set it and request the same composition; the returned cached functions must carry the annotation. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan Ogilvie --- cmd/diff/diffprocessor/function_provider.go | 10 ++- .../diffprocessor/function_provider_test.go | 83 +++++++++++++++---- 2 files changed, 76 insertions(+), 17 deletions(-) diff --git a/cmd/diff/diffprocessor/function_provider.go b/cmd/diff/diffprocessor/function_provider.go index 487dee7f..c3047cc3 100644 --- a/cmd/diff/diffprocessor/function_provider.go +++ b/cmd/diff/diffprocessor/function_provider.go @@ -153,9 +153,13 @@ func generateInstanceID() string { func (p *CachedFunctionProvider) GetFunctionsForComposition(comp *apiextensionsv1.Composition) ([]pkgv1.Function, error) { compName := comp.GetName() - // Check cache first + // Check cache first. Re-apply the Docker network annotation on every cache + // hit so the annotation reflects the current value of EnvDockerNetwork + // (rather than whatever was in effect when the entry was first cached). if cached, ok := p.cache[compName]; ok { p.logger.Debug("Using cached functions", "composition", compName, "count", len(cached)) + applyDockerNetworkAnnotation(cached, p.logger) + return cached, nil } @@ -197,11 +201,11 @@ func (p *CachedFunctionProvider) GetFunctionsForComposition(comp *apiextensionsv p.containerNames = append(p.containerNames, containerName) } + applyDockerNetworkAnnotation(fns, p.logger) + // Cache for future calls p.cache[compName] = fns - applyDockerNetworkAnnotation(fns, p.logger) - return fns, nil } diff --git a/cmd/diff/diffprocessor/function_provider_test.go b/cmd/diff/diffprocessor/function_provider_test.go index 94c32ee7..e9bc5153 100644 --- a/cmd/diff/diffprocessor/function_provider_test.go +++ b/cmd/diff/diffprocessor/function_provider_test.go @@ -17,7 +17,6 @@ limitations under the License. package diffprocessor import ( - "os" "strings" "testing" @@ -243,6 +242,56 @@ func TestCachedFunctionProvider_GetFunctionsForComposition_CacheHit(t *testing.T } } +// TestCachedFunctionProvider_DockerNetworkAnnotation_CacheHit verifies that +// the Docker network annotation reflects the *current* value of +// CROSSPLANE_DIFF_DOCKER_NETWORK on every call, including when a cached entry +// is returned. Without this guarantee, a cache populated before the env var +// was set would keep returning unannotated functions for the rest of the +// process, defeating the feature. +func TestCachedFunctionProvider_DockerNetworkAnnotation_CacheHit(t *testing.T) { + functions := []pkgv1.Function{ + {ObjectMeta: metav1.ObjectMeta{Name: "function-test"}}, + } + + fnClient := tu.NewMockFunctionClient(). + WithFunctionsFetchCallback(func() ([]pkgv1.Function, error) { + return functions, nil + }). + Build() + + logger := tu.TestLogger(t, false) + provider := NewCachedFunctionProvider(fnClient, logger) + + comp := &apiextensionsv1.Composition{ + ObjectMeta: metav1.ObjectMeta{Name: "test-composition"}, + } + + // Prime the cache with the env var unset so the cached entry has no + // network annotation written during the miss path. + t.Setenv(EnvDockerNetwork, "") + + if _, err := provider.GetFunctionsForComposition(comp); err != nil { + t.Fatalf("priming GetFunctionsForComposition() error = %v", err) + } + + // Now set the env var and request the same composition again. The + // returned (cached) functions must carry the annotation. + t.Setenv(EnvDockerNetwork, "ci-network") + + fns, err := provider.GetFunctionsForComposition(comp) + if err != nil { + t.Fatalf("cache-hit GetFunctionsForComposition() error = %v", err) + } + + if len(fns) != 1 { + t.Fatalf("got %d functions, want 1", len(fns)) + } + + if got := fns[0].Annotations[annotationRuntimeDockerNetwork]; got != "ci-network" { + t.Errorf("cache-hit network annotation = %q, want %q", got, "ci-network") + } +} + func TestCachedFunctionProvider_GetFunctionsForComposition_Error(t *testing.T) { fnClient := tu.NewMockFunctionClient(). WithFailedFunctionsFetch("fetch error"). @@ -675,9 +724,10 @@ func TestGenerateContainerName(t *testing.T) { func TestApplyDockerNetworkAnnotation(t *testing.T) { tests := map[string]struct { - envValue string - fns []pkgv1.Function - wantNetwork string + envValue string + fns []pkgv1.Function + wantNetwork string + checkExistingPreserved bool }{ "EnvSet": { envValue: "github_network_abc123", @@ -706,17 +756,17 @@ func TestApplyDockerNetworkAnnotation(t *testing.T) { }, }, }, - wantNetwork: "my-network", + wantNetwork: "my-network", + checkExistingPreserved: true, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - if tt.envValue != "" { - t.Setenv(EnvDockerNetwork, tt.envValue) - } else { - os.Unsetenv(EnvDockerNetwork) - } + // t.Setenv (even with an empty value) snapshots the prior value + // and restores it on subtest cleanup, avoiding leaks into other + // tests in this package or into the developer's shell. + t.Setenv(EnvDockerNetwork, tt.envValue) logger := tu.TestLogger(t, false) applyDockerNetworkAnnotation(tt.fns, logger) @@ -728,11 +778,16 @@ func TestApplyDockerNetworkAnnotation(t *testing.T) { } } - // Verify existing annotations are preserved when env is set - if tt.wantNetwork != "" { + if tt.checkExistingPreserved { for _, fn := range tt.fns { - if v, ok := fn.Annotations["existing-key"]; ok && v != "existing-value" { - t.Errorf("existing annotation was modified: got %q, want %q", v, "existing-value") + v, ok := fn.Annotations["existing-key"] + if !ok { + t.Errorf("function %q: existing annotation %q was removed", fn.Name, "existing-key") + continue + } + + if v != "existing-value" { + t.Errorf("function %q: existing annotation = %q, want %q", fn.Name, v, "existing-value") } } }