aws-samples / aws-samples/appmod-blueprints

kind-crossplane cannot provision hub into an existing VPC (provider capability parity gap)

Open
#833 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Shell
Stars
105
Forks
62
Avg merge
11h 17m
Merged PRs (30d)
76

Description

## Summary

`kind-crossplane` cannot provision the hub into a **pre-existing (customer-supplied) VPC**. `kind-kro-ack` can. This is a capability gap between two cluster providers that are supposed to be interchangeable.

A customer running in their own account, who already has a VPC governed by their network team, currently cannot use the `kind-crossplane` provider at all. Raised by the `open-agentic-platform` consumer.

## Requirement: providers must be at capability parity

**Every cluster provider must expose an identical capability set. Adding a capability to one provider and not the other is not acceptable.** `cluster-providers/README.md` defines the providers as pluggable behind one contract, and `config.yaml` is a single shared schema. When a capability lands in one provider only:

- `config.local.yaml` silently means different things depending on `clusterProvider`. `hub.network.vpcId` is honoured by `kind-kro-ack` and ignored by `kind-crossplane`, with no error.
- Consumers cannot switch providers without re-validating, which defeats the pluggable design.
- The gap is invisible until provisioning either fails or, worse, silently creates a second VPC.

Any PR adding a provider capability should either implement it for all providers or open a tracking issue like this one **and** make the unimplemented provider fail fast with a clear message rather than ignore the input.

Immediate follow-up: `kind-crossplane` should reject `hub.network.vpcId` with an explicit "not supported by this provider, see #" error instead of silently ignoring it and creating a VPC.

## Current state (verified)

| | existing-VPC import | mechanism |
|---|---|---|
| `kind-kro-ack` | supported | `HUB_VPC_ID`/`HUB_SUBNET_1`/`HUB_SUBNET_2` vars (`Taskfile.yaml:32-37`); `hub:claim` selects kind `EksCluster` + `network` block when `vpcId` set, else `EksclusterWithVpc` + `cidr` block |
| `kind-crossplane` | **missing** | only `hub.vpcCidr` (`Taskfile.yaml:49`, used at `:426`); always creates a VPC; VPC id derived post-hoc from the finished cluster (`:647-654`) |

kro-ack gets this cheaply because KRO exposes two RGD kinds and the Taskfile picks one.

## Why this is not a small change on crossplane

`gitops/abstractions/crossplane/platform-cluster/templates/composition.yaml` is ~1,450 lines / 58KB, `mode: Pipeline` with a **single** step `function-patch-and-transform` (`pt.fn.crossplane.io/v1beta1`) and a **static** `resources:` list.

Importing a VPC requires suppressing 14+ networking resources: `vpc` (:44), `igw` (:65), `eip` (:80), 4 × `Subnet` (:97,147,195,242), `natgw` (:286), 2 × `RouteTable` (:322,357), 2 × `Route` (:339,374), 4 × `RouteTableAssociation` (:395,414,433,452).

**`function-patch-and-transform` cannot conditionally omit a resource.** Confirmed by Crossplane upstream ([blog](https://blog.crossplane.io/building-crossplane-composition-functions-to-empower-your-control-plane/)): *"we needed several compositions to be able to specify different instance counts because native patch and transform does not support conditionally rendering resources."*

Additional constraint: one Composition serves **both** hub and spokes. It is rendered twice, on the Kind bootstrap cluster for the hub (`cluster-providers/kind-crossplane/Taskfile.yaml:412,416`) and on the hub via `gitops/bootstrap/abstractions.yaml`, with spoke claims using the same chart path (`gitops/bootstrap/clusters-crossplane.yaml:56`). Any solution must leave spoke VPC creation untouched.

Also required: the XRD exposes `vpcId` under **`status`** only (`xrd.yaml:132`). New `spec.network.{vpcId,subnetIds}` inputs are needed, and `claim.yaml` currently sets no `compositionRef`/`compositionSelector`.

## Options evaluated

### A. Two Compositions, selected per-claim via `compositionRef`
`platform-cluster` (creates VPC, today's default) and `platform-cluster-existing-vpc` (networking omitted, EKS subnets patched from `spec.network.subnetIds`). Taskfile sets `compositionRef.name` when `hub.network.vpcId` is set.

- Pro: mirrors kro-ack semantics exactly; per-claim choice so spokes provably unaffected; no new dependency.
- Con: two variants of a 1,450-line artifact. Mitigable by rendering both from one Helm template so the shared body is not duplicated.
- This is the documented pre-Functions workaround.

### B. Composition Functions (`function-go-templating` or a custom Go function)
Real conditionals, so one Composition covers both cases.

- Pro: where Crossplane upstream points; removes the limitation rather than working around it; Imagine Learning migrated off patch-and-transform for exactly this reason.
- Con: new Function package must be installed and managed; requires restructuring the composition; larger blast radius since the same Composition serves the whole fleet.

### C. `managementPolicies: ["Observe"]` + `crossplane.io/external-name`
Crossplane's official [Import Existing Resources](https://docs.crossplane.io/latest/guides/import-existing-resources/) pattern: *"Crossplane imports observe only resources but never changes or deletes the resources."*

- Pro: needs **no** conditional rendering for the VPC/subnets. Resource count is unchanged; the MRs become read-only observers and downstream `vpcIdRef`/`subnetIdRefs` still resolve. `policy.fromFieldPath: Optional` patches are naturally skipped when `spec.network.vpcId` is absent, so the default path is untouched.
- Con: **does not solve the whole problem.** IGW / NAT / RouteTables / Associations would still be *created* against a VPC that already has them. Observing those needs IDs we do not collect, and asking for them bloats the input contract. Flagged beta and provider-dependent.
- Best used as a *component* of A or B for the VPC and subnets specifically.

**Recommendation:** A now (parity fastest, no new dependency), with C for the VPC/subnet MRs. Evaluate B as a separate modernisation of the composition, since it is the strategic direction but too large to couple to a parity fix.

## Subnet tagging: a prerequisite, not a detail

The platform relies on subnet role tags that a customer VPC very likely lacks.

`cluster-providers/kind-crossplane/Taskfile.yaml:650-654` derives cluster-secret annotations by tag filter:

```
PRIVATE_SUBNET_IDS: --filters "Name=tag:kubernetes.io/role/internal-elb,Values=1"
PUBLIC_SUBNET_IDS: --filters "Name=tag:kubernetes.io/role/elb,Values=1"
```

Both providers tag only subnets **they create** (kro-ack `rg-vpc.yaml:223,252,280,308`; crossplane `composition.yaml:109,159,206,253`). Neither tags imported subnets. Untagged, these queries return empty, `crossplane-env-config` is malformed, and **ALB/NLB subnet auto-discovery fails for customer workloads too**, not just for platform ingress.

Options: (a) document a hard pre-tag requirement; (b) tag the supplied subnets ourselves — in-repo precedent exists at `workshop/create-config.sh:408`, which tags pre-existing discovered subnets `kubernetes.io/role/internal-elb=1`; (c) fall back to configured `hub.network.subnetIds` when the tag query is empty, splitting public/private by `MapPublicIpOnLaunch`.

(a)+(c) is the read-only combination. (b) has precedent but writes to customer-owned resources.

## Testing and validation strategy

Provider parity is currently unenforced, which is how this gap appeared. Proposed layers:

**1. Static parity test (cheap, CI-able, highest value).** Assert both providers accept the same `config.local.yaml` keys. Enumerate `yq` reads in each Taskfile and diff the key sets; fail on asymmetry. This alone would have caught `hub.network.*` being kro-ack-only. Pairs with the existing `platform/validation/` harness and the drift-test precedent in `platform/validation/pod-identity/test_chart_drift.py`.

**2. Render-level tests (no AWS spend).**
- `helm template` the existing-VPC path and assert **zero** `kind: VPC|InternetGateway|NATGateway|Subnet|RouteTable|Route|RouteTableAssociation|ElasticIP` resources, and that EKS `subnetIds` equal the configured ids.
- `helm template` the default path and assert the output is **byte-identical to `main`** — the regression guard that matters most.
- Assert the spoke render is unchanged when the hub uses the existing-VPC composition.

**3. Integration test (real AWS, nightly or on-demand).**
- Pre-create a "customer" VPC with 2 private + 2 public subnets, NAT, IGW, deliberately **untagged**, to exercise the tagging fallback.
- Provision the hub into it; assert no new VPC was created (`describe-vpcs` count unchanged), EKS lands in the supplied subnets, cluster-secret annotations are populated, and an ALB provisions and serves.
- Repeat on both providers from the same `config.local.yaml`, asserting equivalent outcomes. That comparison is the actual parity gate.
- Tear down; assert the pre-existing VPC **survives** (`deletion-policy`/`Observe` must not delete customer infra). This is the highest-risk failure mode and needs an explicit test.

**4. Negative test.** With `hub.network.vpcId` set on a provider that does not support it, assert a fast, clear failure rather than silent VPC creation.

## Acceptance criteria

- [ ] `kind-crossplane` provisions the hub into `hub.network.{vpcId,subnetIds}` and creates no VPC.
- [ ] Unset `hub.network` behaves exactly as today; default render byte-identical to `main`.
- [ ] Spoke provisioning unaffected.
- [ ] Post-hoc derivation paths (`:647-654`, `:967-968`, `:1004-1014`) resolve for an imported VPC, including untagged subnets.
- [ ] Teardown never deletes customer-owned network resources.
- [ ] Static parity test in CI; both providers accept the same config keys.
- [ ] Until implemented, `kind-crossplane` fails fast on `hub.network.vpcId` instead of ignoring it.

## Notes

Split out of the cluster-provider parity work on `feat/cluster-provider-parity-cloudfront` (async-domain parity and Workshop-Studio decoupling ship there). Deliberately **not** bundled: this is an architectural change to the Composition, not a Taskfile change, and it deserves its own review.

Line references verified against `main` @ `737f081f`.

Contributor guide

Open the contributing guide

Research direction

Start with cluster-providers/kind-crossplane/Taskfile.yaml and gitops/abstractions/crossplane/platform-cluster/templates/composition.yaml, then compare kind-kro-ack's Taskfile inputs and hub:claim path. Read the platform/validation harness and pod-identity drift test before designing render and parity coverage. Done means imported hub networking works without creating or deleting customer resources, default and spoke renders remain unchanged, and unsupported inputs fail clearly.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, helm, kubernetes, shell
Domain
cloud, devops, infrastructure
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.