From fce07655ac171f6dbae4b03615a215d583aab43d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonasz=20Ma=C5=82ecki?= Date: Wed, 21 Oct 2026 17:30:23 +0200 Subject: [PATCH 1/2] feat(simulate): add Development runtime for diff servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Provider manifest passed with --providers can now carry simulate.crossplane.io/runtime: Development, like render's per-function runtime annotation: the CLI connects to a diff server the user already runs, from source or under a debugger, at the gRPC target in runtime-development-target (default localhost:9099, upjet's default --address) instead of fetching the package and starting a container. Nothing is fetched for such a provider, and the plan protocol has no RPC that reports what a server serves, so the manifest declares the API groups in runtime-development-api-groups. Resources in those groups route to it unless a package-run server ships a CRD for their exact kind. Docker is only pinged when some provider still needs a container. Fixes #442 Signed-off-by: Jonasz Małecki --- cmd/crossplane/simulate/simulate.go | 162 +++++++++++++---- cmd/crossplane/simulate/simulate_test.go | 214 ++++++++++++++++++----- internal/diffserver/manager.go | 175 ++++++++++++++---- internal/diffserver/manager_test.go | 111 ++++++++++++ 4 files changed, 551 insertions(+), 111 deletions(-) diff --git a/cmd/crossplane/simulate/simulate.go b/cmd/crossplane/simulate/simulate.go index bfdea6a3..35a4a63e 100644 --- a/cmd/crossplane/simulate/simulate.go +++ b/cmd/crossplane/simulate/simulate.go @@ -69,8 +69,14 @@ import ( var helpDetail string // Per-provider runtime annotations on Provider manifests passed via -// --providers, mirroring render's per-function runtime-docker annotations. +// --providers, mirroring render's per-function runtime annotations. const ( + // AnnotationKeyRuntime selects how a provider's diff server is run: + // Docker (the default) runs the package image as a local container, + // Development connects to a diff server already running outside Docker, + // for example from source or under a debugger. + AnnotationKeyRuntime = "simulate.crossplane.io/runtime" + // AnnotationKeyDockerPublishAddress sets the address Docker publishes // this provider's diff server port on. Defaults to loopback only. AnnotationKeyDockerPublishAddress = "simulate.crossplane.io/runtime-docker-publish-address" @@ -84,6 +90,21 @@ const ( // tears it down, Orphan leaves it running so later runs reuse it without // paying startup. AnnotationKeyDockerCleanup = "simulate.crossplane.io/runtime-docker-cleanup" + + // AnnotationKeyDevelopmentTarget is the gRPC target of a Development + // provider's diff server. Defaults to localhost:9099, upjet's default + // for `internal diff-server --address`. + AnnotationKeyDevelopmentTarget = "simulate.crossplane.io/runtime-development-target" + + // AnnotationKeyDevelopmentAPIGroups lists, comma-separated, the API + // groups a Development provider's diff server serves. No package is + // fetched for a Development provider, so nothing else says which + // resources route to it. Legacy and namespaced (.m.) groups are distinct + // and must both be listed. + AnnotationKeyDevelopmentAPIGroups = "simulate.crossplane.io/runtime-development-api-groups" + + // defaultDevelopmentTarget is upjet's default diff server address. + defaultDevelopmentTarget = "localhost:9099" ) // Cmd previews what applying managed resources would do. @@ -140,8 +161,19 @@ func (c *Cmd) Run(ctx context.Context, k *kong.Context, logger logging.Logger, s return err } - if err := docker.Check(ctx); err != nil { - return errors.Wrap(err, "cannot connect to Docker, which simulate needs to run provider diff servers") + var inputs []diffserver.Input + if c.Providers != "" { + if inputs, err = loadProviderInputs(c.Providers); err != nil { + return errors.Wrapf(err, "cannot load provider manifests from %q", c.Providers) + } + } + // Only a provider whose package image runs as a container needs Docker; + // a Development provider's server is already running. Discovery from the + // cluster always yields the former. + if c.Providers == "" || needsDocker(inputs) { + if err := docker.Check(ctx); err != nil { + return errors.Wrap(err, "cannot connect to Docker, which simulate needs to run provider diff servers") + } } kubeClient, rmapper, defaultNamespace, err := c.setupKubeClient(logger) @@ -154,12 +186,7 @@ func (c *Cmd) Run(ctx context.Context, k *kong.Context, logger logging.Logger, s return err } - var inputs []diffserver.Input - if c.Providers != "" { - if inputs, err = loadProviderInputs(c.Providers); err != nil { - return errors.Wrapf(err, "cannot load provider manifests from %q", c.Providers) - } - } else { + if c.Providers == "" { // With local ImageConfigs the rewrites apply to original package // references, so discovery must hand those out; without them the // cluster's resolved reference is the exact artifact it runs. @@ -243,9 +270,22 @@ func (c *Cmd) imageConfigs() ([]pkgv1beta1.ImageConfig, error) { return proj.Spec.ImageConfigs, nil } +// needsDocker reports whether any input runs its diff server as a container. +func needsDocker(inputs []diffserver.Input) bool { + for _, in := range inputs { + if in.Runtime != diffserver.RuntimeDevelopment { + return true + } + } + return false +} + // loadProviderInputs reads Provider manifests and turns them into diff server -// inputs: the package image from spec.package, and per-provider Docker -// overrides from the simulate.crossplane.io runtime annotations. +// inputs per their simulate.crossplane.io runtime annotations: the package +// image from spec.package with per-provider Docker overrides, or, for the +// Development runtime, the target of an already running server and the API +// groups it serves. A group can route to only one development server, so +// manifests that declare the same group are rejected up front. func loadProviderInputs(path string) ([]diffserver.Input, error) { loader, err := load.NewLoader(path) if err != nil { @@ -257,38 +297,98 @@ func loadProviderInputs(path string) ([]diffserver.Input, error) { } inputs := make([]diffserver.Input, 0, len(objs)) + declared := map[string]string{} // API group -> the Development provider declaring it for _, o := range objs { if o.GetKind() != "Provider" { return nil, errors.Errorf("expected only Provider manifests, found %s", o.GetKind()) } - in := diffserver.Input{} - if in.Image, _, _ = unstructured.NestedString(o.Object, "spec", "package"); in.Image == "" { - return nil, errors.Errorf("provider %q has no spec.package", o.GetName()) + in, err := providerInput(o) + if err != nil { + return nil, err } - annotations := o.GetAnnotations() - if v := annotations[AnnotationKeyDockerPublishAddress]; v != "" { - if in.PublishAddress, err = netip.ParseAddr(v); err != nil { - return nil, errors.Wrapf(err, "provider %q has an invalid %s annotation", o.GetName(), AnnotationKeyDockerPublishAddress) + for _, g := range in.APIGroups { + if other, ok := declared[g]; ok { + return nil, errors.Errorf("providers %q and %q both declare the API group %q in their %s annotation: a group can route to only one development diff server", + other, o.GetName(), g, AnnotationKeyDevelopmentAPIGroups) } - } - in.Target = annotations[AnnotationKeyDockerTarget] - switch cleanup := diffserver.Cleanup(annotations[AnnotationKeyDockerCleanup]); cleanup { - case diffserver.CleanupRemove, diffserver.CleanupOrphan, "": - in.Cleanup = cleanup - case containerruntime.CleanupStop: - // Render's Stop policy is deliberately unsupported: a stopped - // diff server serves nothing, and restarting one costs nearly as - // much as starting it. - fallthrough - default: - return nil, errors.Errorf("provider %q has an invalid %s annotation %q: must be %s or %s", - o.GetName(), AnnotationKeyDockerCleanup, cleanup, diffserver.CleanupRemove, diffserver.CleanupOrphan) + declared[g] = o.GetName() } inputs = append(inputs, in) } return inputs, nil } +// providerInput turns one Provider manifest into a diff server input per its +// runtime annotation. +func providerInput(o *unstructured.Unstructured) (diffserver.Input, error) { + switch r := diffserver.Runtime(o.GetAnnotations()[AnnotationKeyRuntime]); r { + case diffserver.RuntimeDocker, "": + return dockerInput(o) + case diffserver.RuntimeDevelopment: + return developmentInput(o) + default: + return diffserver.Input{}, errors.Errorf("provider %q has an unsupported %s annotation value %q: must be %s or %s", + o.GetName(), AnnotationKeyRuntime, r, diffserver.RuntimeDocker, diffserver.RuntimeDevelopment) + } +} + +// dockerInput reads a provider whose package image runs as a container: the +// image from spec.package and the Docker overrides from its annotations. +func dockerInput(o *unstructured.Unstructured) (diffserver.Input, error) { + in := diffserver.Input{} + if in.Image, _, _ = unstructured.NestedString(o.Object, "spec", "package"); in.Image == "" { + return diffserver.Input{}, errors.Errorf("provider %q has no spec.package", o.GetName()) + } + annotations := o.GetAnnotations() + if v := annotations[AnnotationKeyDockerPublishAddress]; v != "" { + addr, err := netip.ParseAddr(v) + if err != nil { + return diffserver.Input{}, errors.Wrapf(err, "provider %q has an invalid %s annotation", o.GetName(), AnnotationKeyDockerPublishAddress) + } + in.PublishAddress = addr + } + in.Target = annotations[AnnotationKeyDockerTarget] + switch cleanup := diffserver.Cleanup(annotations[AnnotationKeyDockerCleanup]); cleanup { + case diffserver.CleanupRemove, diffserver.CleanupOrphan, "": + in.Cleanup = cleanup + case containerruntime.CleanupStop: + // Render's Stop policy is deliberately unsupported: a stopped + // diff server serves nothing, and restarting one costs nearly as + // much as starting it. + fallthrough + default: + return diffserver.Input{}, errors.Errorf("provider %q has an invalid %s annotation %q: must be %s or %s", + o.GetName(), AnnotationKeyDockerCleanup, cleanup, diffserver.CleanupRemove, diffserver.CleanupOrphan) + } + return in, nil +} + +// developmentInput reads a provider whose diff server the user already runs. +// Nothing is fetched and nothing starts for it, so spec.package is not +// needed and the Docker annotations are ignored, the way render ignores them +// on a Development function. The served API groups must be declared instead. +func developmentInput(o *unstructured.Unstructured) (diffserver.Input, error) { + annotations := o.GetAnnotations() + in := diffserver.Input{ + Runtime: diffserver.RuntimeDevelopment, + Name: o.GetName(), + DevelopmentTarget: defaultDevelopmentTarget, + } + if t := annotations[AnnotationKeyDevelopmentTarget]; t != "" { + in.DevelopmentTarget = t + } + for g := range strings.SplitSeq(annotations[AnnotationKeyDevelopmentAPIGroups], ",") { + if g = strings.TrimSpace(g); g != "" { + in.APIGroups = append(in.APIGroups, g) + } + } + if len(in.APIGroups) == 0 { + return diffserver.Input{}, errors.Errorf("provider %q uses the Development runtime but declares no API groups: set its %s annotation to the comma-separated groups its diff server serves, for example ec2.aws.upbound.io,ec2.aws.m.upbound.io", + o.GetName(), AnnotationKeyDevelopmentAPIGroups) + } + return in, nil +} + // loadImageConfigs reads ImageConfig manifests from a YAML file. func loadImageConfigs(path string) ([]pkgv1beta1.ImageConfig, error) { loader, err := load.NewLoader(path) diff --git a/cmd/crossplane/simulate/simulate_test.go b/cmd/crossplane/simulate/simulate_test.go index b5f9f7f0..acc8535d 100644 --- a/cmd/crossplane/simulate/simulate_test.go +++ b/cmd/crossplane/simulate/simulate_test.go @@ -98,9 +98,17 @@ func notSupportedErr(t *testing.T) error { } func TestLoadProviderInputs(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "providers.yaml") - manifest := `--- + const ec2 = "xpkg.upbound.io/upbound/provider-aws-ec2:v2.9.0" + + cases := map[string]struct { + reason string + manifest string + want []diffserver.Input + wantErr string // A substring of the error; empty means no error. + }{ + "DockerAnnotations": { + reason: "The Docker runtime is the default; its annotations override the publish and target addresses per provider.", + manifest: `--- apiVersion: pkg.crossplane.io/v1 kind: Provider metadata: @@ -109,7 +117,7 @@ metadata: simulate.crossplane.io/runtime-docker-publish-address: 0.0.0.0 simulate.crossplane.io/runtime-docker-target: 192.168.64.2 spec: - package: xpkg.upbound.io/upbound/provider-aws-ec2:v2.9.0 + package: ` + ec2 + ` --- apiVersion: pkg.crossplane.io/v1 kind: Provider @@ -117,53 +125,175 @@ metadata: name: provider-aws-s3 spec: package: xpkg.upbound.io/upbound/provider-aws-s3:v2.9.0 -` - if err := os.WriteFile(path, []byte(manifest), 0o600); err != nil { - t.Fatalf("cannot write manifest: %v", err) - } - - got, err := loadProviderInputs(path) - if err != nil { - t.Fatalf("loadProviderInputs(...): unexpected error: %v", err) - } - want := []diffserver.Input{ - { - Image: "xpkg.upbound.io/upbound/provider-aws-ec2:v2.9.0", - PublishAddress: netip.MustParseAddr("0.0.0.0"), - Target: "192.168.64.2", +`, + want: []diffserver.Input{ + {Image: ec2, PublishAddress: netip.MustParseAddr("0.0.0.0"), Target: "192.168.64.2"}, + {Image: "xpkg.upbound.io/upbound/provider-aws-s3:v2.9.0"}, + }, }, - {Image: "xpkg.upbound.io/upbound/provider-aws-s3:v2.9.0"}, - } - if diff := cmp.Diff(want, got, cmp.Comparer(func(a, b netip.Addr) bool { return a == b })); diff != "" { - t.Errorf("loadProviderInputs(...): -want, +got:\n%s", diff) - } - - notAProvider := filepath.Join(dir, "other.yaml") - if err := os.WriteFile(notAProvider, []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: x\n"), 0o600); err != nil { - t.Fatalf("cannot write manifest: %v", err) - } - if _, err := loadProviderInputs(notAProvider); err == nil { - t.Errorf("loadProviderInputs(...): want an error for non-Provider manifests") - } - - // Render's Stop cleanup policy is deliberately unsupported for diff - // servers, so the annotation that would request it is rejected. - stopCleanup := filepath.Join(dir, "stop.yaml") - manifest = `--- + "NotAProvider": { + reason: "Only Provider manifests can be run as diff servers.", + manifest: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: x\n", + wantErr: "expected only Provider manifests", + }, + "StopCleanupRejected": { + reason: "Render's Stop cleanup policy is deliberately unsupported for diff servers.", + manifest: `--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-aws-ec2 + annotations: + simulate.crossplane.io/runtime-docker-cleanup: Stop +spec: + package: ` + ec2 + ` +`, + wantErr: "invalid", + }, + "DevelopmentDefaults": { + reason: "A Development provider defaults to upjet's diff server address and needs no spec.package.", + manifest: `--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-aws-ec2 + annotations: + simulate.crossplane.io/runtime: Development + simulate.crossplane.io/runtime-development-api-groups: ec2.aws.upbound.io, ec2.aws.m.upbound.io +`, + want: []diffserver.Input{{ + Runtime: diffserver.RuntimeDevelopment, + Name: "provider-aws-ec2", + DevelopmentTarget: "localhost:9099", + APIGroups: []string{"ec2.aws.upbound.io", "ec2.aws.m.upbound.io"}, + }}, + }, + "DevelopmentTargetIgnoresDockerAnnotations": { + reason: "A Development provider takes its target from the annotation; spec.package and the Docker annotations do not apply to it, as in render.", + manifest: `--- apiVersion: pkg.crossplane.io/v1 kind: Provider metadata: name: provider-aws-ec2 annotations: + simulate.crossplane.io/runtime: Development + simulate.crossplane.io/runtime-development-target: localhost:9444 + simulate.crossplane.io/runtime-development-api-groups: ec2.aws.upbound.io + simulate.crossplane.io/runtime-docker-publish-address: 0.0.0.0 simulate.crossplane.io/runtime-docker-cleanup: Stop spec: - package: xpkg.upbound.io/upbound/provider-aws-ec2:v2.9.0 -` - if err := os.WriteFile(stopCleanup, []byte(manifest), 0o600); err != nil { - t.Fatalf("cannot write manifest: %v", err) + package: ` + ec2 + ` +`, + want: []diffserver.Input{{ + Runtime: diffserver.RuntimeDevelopment, + Name: "provider-aws-ec2", + DevelopmentTarget: "localhost:9444", + APIGroups: []string{"ec2.aws.upbound.io"}, + }}, + }, + "DevelopmentWithoutAPIGroups": { + reason: "Nothing is fetched for a Development provider, so it must declare the API groups it serves.", + manifest: `--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-aws-ec2 + annotations: + simulate.crossplane.io/runtime: Development + simulate.crossplane.io/runtime-development-api-groups: " , " +`, + wantErr: AnnotationKeyDevelopmentAPIGroups, + }, + "DevelopmentGroupsOverlap": { + reason: "An API group can route to only one development diff server.", + manifest: `--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-aws-ec2 + annotations: + simulate.crossplane.io/runtime: Development + simulate.crossplane.io/runtime-development-api-groups: ec2.aws.upbound.io +--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-aws-ec2-fork + annotations: + simulate.crossplane.io/runtime: Development + simulate.crossplane.io/runtime-development-target: localhost:9100 + simulate.crossplane.io/runtime-development-api-groups: ec2.aws.upbound.io +`, + wantErr: "both declare the API group", + }, + "UnknownRuntime": { + reason: "Runtime values other than Docker and Development are rejected, as in render.", + manifest: `--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-aws-ec2 + annotations: + simulate.crossplane.io/runtime: Podman +spec: + package: ` + ec2 + ` +`, + wantErr: "unsupported", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "providers.yaml") + if err := os.WriteFile(path, []byte(tc.manifest), 0o600); err != nil { + t.Fatalf("cannot write manifest: %v", err) + } + + got, err := loadProviderInputs(path) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("%s\nloadProviderInputs(...): want an error containing %q, got %v", tc.reason, tc.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("%s\nloadProviderInputs(...): unexpected error: %v", tc.reason, err) + } + if diff := cmp.Diff(tc.want, got, cmp.Comparer(func(a, b netip.Addr) bool { return a == b })); diff != "" { + t.Errorf("%s\nloadProviderInputs(...): -want, +got:\n%s", tc.reason, diff) + } + }) } - if _, err := loadProviderInputs(stopCleanup); err == nil || !strings.Contains(err.Error(), "invalid") { - t.Errorf("loadProviderInputs(...): want an invalid-annotation error for the Stop cleanup policy, got %v", err) +} + +func TestNeedsDocker(t *testing.T) { + cases := map[string]struct { + reason string + inputs []diffserver.Input + want bool + }{ + "NoInputs": { + reason: "Nothing to run means nothing to run in Docker.", + want: false, + }, + "OnlyDevelopment": { + reason: "Development providers connect to servers the user runs, so Docker is not needed.", + inputs: []diffserver.Input{{Runtime: diffserver.RuntimeDevelopment, Name: "dev"}}, + want: false, + }, + "MixedRuntimes": { + reason: "One provider whose image runs as a container is enough to need Docker.", + inputs: []diffserver.Input{{Runtime: diffserver.RuntimeDevelopment, Name: "dev"}, {Image: "xpkg.example.org/provider:v1"}}, + want: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + if got := needsDocker(tc.inputs); got != tc.want { + t.Errorf("%s\nneedsDocker(...): want %t, got %t", tc.reason, tc.want, got) + } + }) } } diff --git a/internal/diffserver/manager.go b/internal/diffserver/manager.go index a65923b0..418e4462 100644 --- a/internal/diffserver/manager.go +++ b/internal/diffserver/manager.go @@ -19,6 +19,9 @@ limitations under the License. // function packages. Whether a package can serve diffs is declared in its // metadata (the DiffServer capability), and which resources it serves comes // from the CRDs it ships, so everything is known before a container starts. +// A provider can instead point at a diff server already running outside +// Docker (the Development runtime, like render's); nothing is fetched for it, +// so the provider declares the API groups its server serves. package diffserver import ( @@ -81,6 +84,21 @@ const ( CleanupOrphan = containerruntime.CleanupOrphan ) +// Runtime is how a provider's diff server is run, following render's +// per-function runtime annotation. +type Runtime string + +const ( + // RuntimeDocker runs the provider's package image as a local container. + // It is the default. + RuntimeDocker Runtime = "Docker" + + // RuntimeDevelopment connects to a diff server the user runs themselves, + // from source or under a debugger. No package is fetched and no + // container starts, so the provider declares the API groups it serves. + RuntimeDevelopment Runtime = "Development" +) + // labelDomain prefixes the labels simulate applies to the Docker containers // it creates. const labelDomain = "simulate.crossplane.io" @@ -106,40 +124,62 @@ const ( LabelKeyCleanup = labelDomain + "/cleanup" ) -// Server is a running diff server for one provider package. +// Server is a running diff server for one provider. type Server struct { - Image string - Address string + // Image identifies the provider: its package image, or the manifest + // name of a Development provider, which has no image to run. + Image string + Address string + // ContainerID is empty for a Development provider: its server runs + // outside Docker. ContainerID string - GroupKinds []schema.GroupKind - - conn *grpc.ClientConn - client diffv1alpha1.PlanServiceClient - running *containerruntime.Running - cleanup Cleanup + // GroupKinds are the kinds the provider's package ships CRDs for. + GroupKinds []schema.GroupKind + // APIGroups are the groups a Development provider declared it serves. + APIGroups []string + + conn *grpc.ClientConn + client diffv1alpha1.PlanServiceClient + // stop tears down whatever Start created. Nil when nothing was created. + stop func(context.Context) error } -// Skipped records a provider image that does not run a diff server this run, -// and why. Resources it would have served fall back to the approximate -// client-side diff. +// Skipped records a provider that does not run a diff server this run, and +// why. Resources it would have served fall back to the approximate +// client-side diff. Image identifies the provider the way Server.Image does. type Skipped struct { Image string Reason string } -// Input describes one provider package to run as a diff server. The zero -// values mean the manager's defaults apply; set them to override Docker +// Input describes one provider to run, or connect to, as a diff server. The +// zero values mean the manager's defaults apply; set them to override Docker // behavior for this provider only, the way render's per-function runtime // annotations do. type Input struct { + // Image is the provider package to run. A Development provider needs + // none: its server is already running. Image string PublishAddress netip.Addr Target string Cleanup Cleanup + + // Runtime selects how the diff server is run. Empty means Docker. + Runtime Runtime + // Name identifies the provider in progress and skip messages when there + // is no image to name it by, e.g. the Provider manifest's name. + Name string + // DevelopmentTarget is the gRPC target of a Development provider's + // running diff server. + DevelopmentTarget string + // APIGroups are the API groups a Development provider serves. Resources + // in these groups route to it unless a package-run server ships a CRD + // for their exact group and kind. + APIGroups []string } -// Manager starts diff server containers for provider packages and routes -// resources to them. +// Manager starts diff server containers for provider packages, connects to +// diff servers run outside Docker, and routes resources to them. type Manager struct { pkgs runtimexpkg.Client cli containerruntime.Client @@ -150,6 +190,7 @@ type Manager struct { mu sync.Mutex routes map[schema.GroupKind]*Server + groups map[string]*Server servers []*Server } @@ -184,6 +225,7 @@ func NewManager(pkgs runtimexpkg.Client, log logging.Logger, opts ...Option) *Ma readyTimeout: defaultReadyTimeout, publishAddr: netip.MustParseAddr("127.0.0.1"), routes: map[schema.GroupKind]*Server{}, + groups: map[string]*Server{}, } for _, opt := range opts { opt(m) @@ -192,12 +234,14 @@ func NewManager(pkgs runtimexpkg.Client, log logging.Logger, opts ...Option) *Ma } // Start inspects each provider image and runs a diff server container for -// every package that declares the DiffServer capability, in parallel and best -// effort: an image that cannot serve (no capability, fetch or startup -// failure) is returned as skipped rather than failing the run, since its -// resources degrade to the approximate client-side diff. Progress is reported -// per image on events (nil means no reporting), with the package pull and the -// server start as separate steps because the pull dominates a cold start. +// every package that declares the DiffServer capability, and connects to the +// already running server of every Development provider, in parallel and best +// effort: a provider that cannot serve (no capability, fetch or startup +// failure, nothing answering at a development target) is returned as skipped +// rather than failing the run, since its resources degrade to the approximate +// client-side diff. Progress is reported per provider on events (nil means no +// reporting), with the package pull and the server start as separate steps +// because the pull dominates a cold start. func (m *Manager) Start(ctx context.Context, inputs []Input, events async.EventChannel) ([]Skipped, error) { var ( wg sync.WaitGroup @@ -244,9 +288,21 @@ func (m *Manager) register(s *Server) { } m.routes[gk] = s } + for _, g := range s.APIGroups { + if other, ok := m.groups[g]; ok { + m.log.Info("Multiple development diff servers declare the same API group; keeping the first", + "group", g, "kept", other.Image, "ignored", s.Image) + continue + } + m.groups[g] = s + } } func (m *Manager) startOne(ctx context.Context, in Input, events async.EventChannel) (*Server, *Skipped) { + if in.Runtime == RuntimeDevelopment { + return m.connectDevelopment(ctx, in, events) + } + image := in.Image // The package pull is its own reported step: on a cold cache it downloads @@ -316,7 +372,7 @@ func (m *Manager) startOne(ctx context.Context, in Input, events async.EventChan return nil, &Skipped{Image: image, Reason: fmt.Sprintf("cannot start diff server container: %s", err)} } - server, err := m.connect(ctx, running, image) + server, err := m.connect(ctx, running, image, cleanup) if err != nil { events.SendEvent(start, async.EventStatusFailure) // An orphaned container is left behind even on failure, for debugging. @@ -330,20 +386,41 @@ func (m *Manager) startOne(ctx context.Context, in Input, events async.EventChan events.SendEvent(start, async.EventStatusSuccess) server.GroupKinds = gks - server.cleanup = cleanup m.log.Debug("Diff server ready", "image", image, "address", server.Address, "reused", running.Reused, "cleanup", cleanup) return server, nil } -func (m *Manager) connect(ctx context.Context, running *containerruntime.Running, image string) (*Server, error) { - conn, err := grpc.NewClient(running.Addr, grpc.WithTransportCredentials(insecure.NewCredentials())) +// connectDevelopment connects to a diff server the user already runs, from +// source or under a debugger. Nothing is fetched and nothing starts, so the +// routes come from the API groups the provider declared. +func (m *Manager) connectDevelopment(ctx context.Context, in Input, events async.EventChannel) (*Server, *Skipped) { + name := in.Name + if name == "" { + name = in.Image + } + + step := fmt.Sprintf("Connecting to diff server for %s at %s", name, in.DevelopmentTarget) + events.SendEvent(step, async.EventStatusStarted) + conn, client, err := m.dial(ctx, in.DevelopmentTarget) if err != nil { - return nil, errors.Wrap(err, "cannot create gRPC client") + events.SendEvent(step, async.EventStatusFailure) + return nil, &Skipped{Image: name, Reason: fmt.Sprintf("no diff server answered at %s; start the provider's internal diff-server command before simulating: %s", in.DevelopmentTarget, err)} } + events.SendEvent(step, async.EventStatusSuccess) - client := diffv1alpha1.NewPlanServiceClient(conn) - if err := waitReady(ctx, client, m.readyTimeout); err != nil { - _ = conn.Close() + m.log.Debug("Development diff server ready", "provider", name, "target", in.DevelopmentTarget, "apiGroups", in.APIGroups) + return &Server{ + Image: name, + Address: in.DevelopmentTarget, + APIGroups: in.APIGroups, + conn: conn, + client: client, + }, nil +} + +func (m *Manager) connect(ctx context.Context, running *containerruntime.Running, image string, cleanup Cleanup) (*Server, error) { + conn, client, err := m.dial(ctx, running.Addr) + if err != nil { return nil, err } @@ -353,10 +430,25 @@ func (m *Manager) connect(ctx context.Context, running *containerruntime.Running ContainerID: running.ID, conn: conn, client: client, - running: running, + stop: func(ctx context.Context) error { return running.Stop(ctx, cleanup) }, }, nil } +// dial connects to the diff server at target and waits for it to answer. +func (m *Manager) dial(ctx context.Context, target string) (*grpc.ClientConn, diffv1alpha1.PlanServiceClient, error) { + conn, err := grpc.NewClient(target, grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + return nil, nil, errors.Wrap(err, "cannot create gRPC client") + } + + client := diffv1alpha1.NewPlanServiceClient(conn) + if err := waitReady(ctx, client, m.readyTimeout); err != nil { + _ = conn.Close() + return nil, nil, err + } + return conn, client, nil +} + // waitReady polls the server with an empty Plan request until it answers. // Docker publishes the host port before the server inside the container binds // its own, so a successful TCP connect proves nothing; any gRPC status from @@ -386,15 +478,18 @@ func waitReady(ctx context.Context, c diffv1alpha1.PlanServiceClient, timeout ti } // Route returns the diff server client that serves the given group and kind, -// if any. +// if any. A package-run server that ships a CRD for the exact group and kind +// wins over a development server that declared the group. func (m *Manager) Route(gk schema.GroupKind) (diffv1alpha1.PlanServiceClient, bool) { m.mu.Lock() defer m.mu.Unlock() - s, ok := m.routes[gk] - if !ok { - return nil, false + if s, ok := m.routes[gk]; ok { + return s.client, true + } + if s, ok := m.groups[gk.Group]; ok { + return s.client, true } - return s.client, true + return nil, false } // Servers returns the running diff servers. @@ -406,7 +501,8 @@ func (m *Manager) Servers() []*Server { // Stop closes connections and tears down containers according to each // server's cleanup policy: Remove tears the container down, Orphan leaves it -// running so the next run finds it by name and skips startup. +// running so the next run finds it by name and skips startup. A development +// server is the user's to stop, so only its connection is closed. func (m *Manager) Stop(ctx context.Context) error { m.mu.Lock() defer m.mu.Unlock() @@ -416,7 +512,10 @@ func (m *Manager) Stop(ctx context.Context) error { if s.conn != nil { _ = s.conn.Close() } - if err := s.running.Stop(ctx, s.cleanup); err != nil { + if s.stop == nil { + continue + } + if err := s.stop(ctx); err != nil { errs = append(errs, errors.Wrapf(err, "cannot stop diff server container %s", s.ContainerID)) } } diff --git a/internal/diffserver/manager_test.go b/internal/diffserver/manager_test.go index 75a36c1d..c690f97f 100644 --- a/internal/diffserver/manager_test.go +++ b/internal/diffserver/manager_test.go @@ -368,4 +368,115 @@ func TestManagerStart(t *testing.T) { t.Errorf("Start(...): want the unready container cleaned up, got removes %v", fd.removed) } }) + + t.Run("DevelopmentRoutesDeclaredGroupsWithoutDocker", func(t *testing.T) { + // An empty package client and a Docker fake with nothing published: + // a fetch or a container start would surface as a skip. + fd := newFakeDocker("", nil) + m := NewManager(&fakePkgClient{}, log, WithDockerClient(fd.client())) + + in := Input{ + Runtime: RuntimeDevelopment, + Name: "provider-aws-ec2", + DevelopmentTarget: servePlanService(t), + APIGroups: []string{"ec2.aws.upbound.io", "ec2.aws.m.upbound.io"}, + } + skipped, err := m.Start(ctx, []Input{in}, nil) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + if len(skipped) != 0 { + t.Fatalf("Start(...): unexpected skips: %+v", skipped) + } + + for _, gk := range []schema.GroupKind{ + {Group: "ec2.aws.upbound.io", Kind: "VPC"}, + {Group: "ec2.aws.m.upbound.io", Kind: "Subnet"}, + } { + if _, ok := m.Route(gk); !ok { + t.Errorf("Route(%s): want a route from the declared API groups", gk) + } + } + if _, ok := m.Route(schema.GroupKind{Group: "s3.aws.upbound.io", Kind: "Bucket"}); ok { + t.Errorf("Route(Bucket.s3): want no route for a group the provider did not declare") + } + if len(fd.created) != 0 { + t.Errorf("Start(...): a development diff server must not start a container, got %v", fd.created) + } + + if err := m.Stop(ctx); err != nil { + t.Fatalf("Stop(...): unexpected error: %v", err) + } + if len(fd.stopped) != 0 || len(fd.removed) != 0 { + t.Errorf("Stop(...): nothing to clean up for a development diff server, got stops %v removes %v", fd.stopped, fd.removed) + } + }) + + t.Run("PackageRoutesWinOverDeclaredGroups", func(t *testing.T) { + fd := newFakeDocker(servePlanService(t), nil) + m := NewManager(pkgs, log, WithDockerClient(fd.client())) + + const pkgImage, devName = "xpkg.example.org/provider-aws-ec2:v2.9.0", "provider-aws-ec2-dev" + dev := Input{ + Runtime: RuntimeDevelopment, + Name: devName, + DevelopmentTarget: servePlanService(t), + APIGroups: []string{"ec2.aws.upbound.io"}, + } + skipped, err := m.Start(ctx, []Input{{Image: pkgImage}, dev}, nil) + if err != nil || len(skipped) != 0 { + t.Fatalf("Start(...): unexpected error or skips: %v %+v", err, skipped) + } + + clients := map[string]diffv1alpha1.PlanServiceClient{} + for _, s := range m.Servers() { + clients[s.Image] = s.client + } + + // The package ships a CRD for VPC, so its server serves VPC even + // though the development server declared the whole group. Subnet + // ships in no package, so the declared group routes it. + if c, ok := m.Route(schema.GroupKind{Group: "ec2.aws.upbound.io", Kind: "VPC"}); !ok || c != clients[pkgImage] { + t.Errorf("Route(VPC.ec2): want the package-run server, got %v (ok %t)", c, ok) + } + if c, ok := m.Route(schema.GroupKind{Group: "ec2.aws.upbound.io", Kind: "Subnet"}); !ok || c != clients[devName] { + t.Errorf("Route(Subnet.ec2): want the development server, got %v (ok %t)", c, ok) + } + + if err := m.Stop(ctx); err != nil { + t.Fatalf("Stop(...): unexpected error: %v", err) + } + if len(fd.removed) != 1 { + t.Errorf("Stop(...): want only the package-run container removed, got %v", fd.removed) + } + }) + + t.Run("DevelopmentTargetNotReadySkips", func(t *testing.T) { + // A listener that accepts no gRPC: readiness never succeeds. No + // Docker client at all: a development provider never needs one. + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("cannot listen: %v", err) + } + t.Cleanup(func() { _ = lis.Close() }) + + m := NewManager(&fakePkgClient{}, log, WithReadyTimeout(700*time.Millisecond)) + + target := lis.Addr().String() + skipped, err := m.Start(ctx, []Input{{ + Runtime: RuntimeDevelopment, + Name: "provider-aws-ec2", + DevelopmentTarget: target, + APIGroups: []string{"ec2.aws.upbound.io"}, + }}, nil) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + if len(skipped) != 1 || !strings.Contains(skipped[0].Reason, target) { + t.Errorf("Start(...): want a skip naming the unreachable target %s, got %+v", target, skipped) + } + if _, ok := m.Route(schema.GroupKind{Group: "ec2.aws.upbound.io", Kind: "VPC"}); ok { + t.Errorf("Route(VPC.ec2): want no route from a skipped development server") + } + }) } From b177e5a20a3e962b15be86c8484ff537e51b6838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonasz=20Ma=C5=82ecki?= Date: Wed, 21 Oct 2026 17:30:23 +0200 Subject: [PATCH 2/2] docs(simulate): document the Development runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Describe the Development runtime annotations, the API groups a Development provider must declare, and the from-source iteration loop in the command help and the design one-pager. Signed-off-by: Jonasz Małecki --- cmd/crossplane/simulate/help/simulate.md | 24 ++++++++++++++++++++++++ design/one-pager-cli-simulate.md | 7 ++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/cmd/crossplane/simulate/help/simulate.md b/cmd/crossplane/simulate/help/simulate.md index c0d5a91d..324dbccf 100644 --- a/cmd/crossplane/simulate/help/simulate.md +++ b/cmd/crossplane/simulate/help/simulate.md @@ -22,6 +22,20 @@ annotations (`simulate.crossplane.io/runtime-docker-publish-address` and `simulate.crossplane.io/runtime-docker-target`) for Docker setups where the published port isn't reachable on the address the daemon reports. +A provider manifest can also opt out of Docker with +`simulate.crossplane.io/runtime: Development`, like render's per-function +runtime annotation: the CLI connects to a diff server you already run, from +source or under a debugger, at the gRPC target in +`simulate.crossplane.io/runtime-development-target` (default `localhost:9099`, +upjet's default `--address`). No package is fetched for such a provider, so +its manifest declares the API groups its server serves in +`simulate.crossplane.io/runtime-development-api-groups`, comma-separated; +legacy and namespaced (`.m.`) groups are distinct and both need listing. +Resources in those groups route to it unless another provider's package ships +a CRD for their exact kind. `spec.package` and the `runtime-docker-*` +annotations are ignored on a Development provider, and Docker isn't needed +at all when every provider in the file is one. + Not every provider serves diffs. A provider package declares the capability in its metadata, and a resource whose provider serves no diff server (or whose diff server can't compute a diff for it) still appears in the preview @@ -62,6 +76,16 @@ annotated with `simulate.crossplane.io/runtime-docker-cleanup: Orphan`: crossplane resource simulate -f resources.yaml --providers providers.yaml ``` +Iterate on a provider's diff server from source, with no package to build or +push: run `go run ./cmd/provider internal diff-server` in the provider +repository, then simulate with a Provider manifest annotated with +`simulate.crossplane.io/runtime: Development` and +`simulate.crossplane.io/runtime-development-api-groups: ec2.aws.upbound.io,ec2.aws.m.upbound.io`: + +```shell +crossplane resource simulate -f resources.yaml --providers providers.yaml +``` + Emit machine-readable results for CI gating: ```shell diff --git a/design/one-pager-cli-simulate.md b/design/one-pager-cli-simulate.md index d19a8acb..97ad8aa1 100644 --- a/design/one-pager-cli-simulate.md +++ b/design/one-pager-cli-simulate.md @@ -131,7 +131,12 @@ Manifests rather than bare image references, so the override can be checked in and shared, and so per-provider runtime configuration has a carrier: the `simulate.crossplane.io/runtime-docker-*` annotations mirror render's per-function runtime annotations for the Docker setups where the -published port is not reachable on the address the daemon reports. +published port is not reachable on the address the daemon reports, and +`simulate.crossplane.io/runtime: Development` mirrors render's Development +runtime for a diff server the developer runs outside Docker. Nothing is +fetched for such a provider, so its manifest declares the API groups the +server serves and those groups route to it unless a package ships a CRD for +the exact kind. ### Previewing a Provider Upgrade