Rewrite functions and tests in Python

[ytsarev]

Rewrites the three composition functions and all twelve test modules from KCL to Python, on crossplane-function-sdk-python 0.15.1, in the SDK layout up function generate produces. Function directory names are unchanged, so the published packages keep their paths.

Behaviour is unchanged, and that is checked, not assumed

Test assertions are partial, so a green suite alone doesn't show a rewrite preserved behaviour. Two stronger checks:

• Functions: before the rewrite I saved the full rendered output of every composition test from the KCL version, then diffed the Python version against it, resource by resource and field by field. Across 20 tests, two resources differ, both on purpose:
  • the ControlPlane manifest no longer carries a foo annotation holding KCL's own rendering of the whole spec, a debug leftover with no Python equivalent
  • a Secrets Manager secret with no secretsManagerSecret block no longer sets recoveryWindowInDays: null, a KCL None != Undefined bug
• Tests: each Python test module generates a CompositionTest/E2ETest manifest identical to the one its KCL module generated. That includes values KCL's typed models were materialising without the source writing them — Object defaults, XRD defaults on inline XRs, spec.parameters: {} — which are assertions the suite really made, so they are now spelled out.

Reaching parity meant reproducing KCL behaviour Python lacks:

• materialised defaults
• the composition keys KCL actually produced where a metadata merge had replaced the intended one. A changed key would make Crossplane replace the resource.

Intentional changes beyond the port

Each is its own commit, on top of the parity-verified port.

The kubeconfig Secrets are real YAML

The kubeconfig Secrets held KCL's str() rendering of a dict: single quotes, True/False, unquoted strings in lists. It only worked because that text happens to parse as a YAML flow mapping, and the port first reproduced it byte for byte through a KCL-format serializer.

• Now: they're written with yaml.safe_dump, and that serializer is gone.
• What changed: the kubeconfig is the same; only its bytes change. In the full render of every test, the kubeconfig data in the three kubeconfig Secret Objects is the only difference.
• Proven live: the e2e passes on it. provider-kubernetes reaches the environment's group and control plane through these kubeconfigs, so the Environment could not reach Ready otherwise.

Repository external names

Repositories carried the misspelled crosslane.io/external-name, which Crossplane ignores. The port first reproduced it for parity, and a separate commit corrects it:

• Before: provider-upbound looks a Repository up by its external name. With none set, Crossplane defaulted it to the generated metadata.name, so the first Observe missed and the provider only recovered by running Create, an upsert that then rewrote the external name.
• After: the external name is the repository name from the start, so an existing repository is observed directly.
• Existing resources: already-reconciled Repositories have the correct external name, so nothing changes for them.

Commits

• test: cover teamWithRobot — teamWithRobot had no test. Written and passing against the KCL functions, so it is part of the baseline.
• refactor: rewrite composition functions in Python
• ci: build functions one at a time — sets UP_MAX_CONCURRENCY=1 in ci.yaml and composition-tests.yaml. ci(e2e): build functions one at a time #39 made the same change to e2e.yaml on main.
• test: rewrite tests in Python
• fix: set Repository external names under the right annotation key
• refactor: restore module boundaries and share common helpers — environments' resources are a resources/ package again, one module per area; helpers duplicated across functions live once in common/, symlinked into each function. A pure move: every test's full render is unchanged.
• refactor: write the kubeconfig Secrets as YAML
• build: pin PyYAML and grpcio in the functions — PyYAML now decides the kubeconfig Secrets' bytes, so it's pinned to the version the tests use.
• docs: explain name-keyed resources precisely; describe each function
• fix: address review findings, and lint Python in CI — an empty externalSecret data/template.data means "not specified" again; a malformed bootstrap kubeconfig leaves the Environment uninitialised instead of failing; a pinned ruff job in CI.

Why builds are serialised

Every Python function build mounts one shared pip-cache Docker volume, up-python-sdk-pip-cache. On a fresh runner, concurrent builds fail with mkdir ...: file exists:

• Where it comes from: up's Python builder (internal/xpkg/functions/python_sdk.go) mounts the volume without NoCopy, over a 38 MB pip cache pre-seeded in the build image.
• How often: it failed 6 of 6 attempts on CI; UP_MAX_CONCURRENCY=1 fixes it.
• What isn't known: raw Docker didn't reproduce it, so the exact trigger isn't pinned down.

This belongs upstream in up. The setting can go once it's fixed there.

Testing

• Composition tests: 22/22, locally and in CI.
• E2E: passed on the rebased branch, the first live run of the Python functions against AWS and Upbound Spaces.
  • The Ready assertion kept polling until the environment was actually up.
  • Teardown left the e2e group empty.
• Latest commit: ruff, yamllint, composition tests and e2e all pass on 498ee8e.

[kaessert]

The port kept the logic split into separate builder functions, but the environments function went from 12 KCL modules (argo/, aws/, utils/, teamRobot.k, pKubernetesHelper.k, bootstrapSecretSync.k, …) to basically two files. resources.py (408 lines) now holds every area behind # --- comment headers. We'd like to bring the module boundaries back before merging, e.g.:

functions/environments/function/
  fn.py              # orchestration (as today)
  resources/
    kubernetes.py    # k8s_object, kubeconfig, provider configs, observe_secret
    argo.py
    team_robot.py
    secret_sync.py
    aws.py           # provider config, crossplane_role, TRUST_POLICY
    util.py          # management_policies, pc_ref, naming/hash helpers

This is purely a move, so the existing composition tests should pass unchanged.

Related: ORPHAN, IAM_NAME_MAX, _simple_hash, truncate_iam_name and _dig are duplicated between environments/function/resources.py and sharedawssecret/function/fn.py. If each function has to be packaged on its own, a test asserting the copies stay in sync would help. Otherwise a shared module would be better.

[ytsarev]

Both done in 74e3cbd.

• resources.py is now the resources/ package you proposed. It's a pure move: all 20 tests pass, and every test's full rendered output is identical to before.
• For the duplicates we can have a real shared module. up packages each function from its own directory but follows symlinks when it does, so the helpers now live once in common/ at the repo root, and each function has a function/common symlink to it. With a single copy there's nothing to keep in sync. I also moved two duplicates you didn't list: upboundreposet's ORPHAN, and the Object defaults kept for KCL parity.

CI is green on 74e3cbd, e2e included, so the symlinked package builds on a clean checkout too.

[jboero]

Reviewed the whole PR against the KCL it replaces — all three functions, the shared common/ package, and the original modules on main.

Review assisted by Claude (Claude Code). I read and verified the findings below myself before posting.

Verdict

High-quality port. The riskiest thing about a KCL→Python rewrite of a composition is silently changing a composition resource key, because Crossplane then deletes and recreates the resource. I checked every key independently and they are all correct. Two minor findings below, the rest are nits. Nothing I'd consider blocking.

What I verified

KCL's utils._metadata(name) carries the key in an annotation, and whether a later metadata merge keeps or drops it depends on the entry operator in the right operand (= override vs : union), not on | itself:

Resource	KCL right-hand annotations op	Key KCL produced	Python key
upboundreposet Repository	annotations: (union)	{org}-{repo}	f"{org}-{repo}" ✅
upboundreposet ProviderConfig	annotations:	providerConfigUpbound	"providerConfigUpbound" ✅
environments AWS ProviderConfig	annotations = (override)	resource name	env_name ✅
teamRobot ProviderConfig	annotations =	resource name	f"{group}-upbound" ✅
pKubernetesHelper ProviderConfig	annotations =	resource name	config_name ✅
crossplaneRole OIDC provider	annotations =	resource name	oidc_name ✅
sharedawssecret SM Secret	annotations =	resource name	f"{aws_secret_name}-secretsmanager-secret" ✅
teamRobot Team	annotations:	envTeam — both with and without teamExternalName	"envTeam" ✅

The KCL-era tests confirm this independently in three places (test-upboundreposet/main.k:23,48, test-environment/main.k:200), including the case where the same-looking block in two different functions yields different keys. That's an easy thing to get wrong and it's right here.

The two deliberate behaviour changes also check out: the crosslane.io typo was real (upboundreposet/main.k:40), and the recoveryWindowInDays: null really was KCL's None != Undefined quirk leaking through.

Minor findings

1. (minor) sharedawssecret/function/fn.py:94-95 — presence vs. truthiness, inconsistent with the KCL and with its sibling function

secret_template_data = dig(raw_ext, "spec", "target", "template", "data")
secret_data = dig(raw_ext, "spec", "data")

Both are gated on is not None later (lines 260, 267). The KCL used ... or Undefined, so an explicitly empty data: [] fell through to the dataFrom.extract branch. Here [] takes the data branch and emits data: [], and the SharedExternalSecret syncs nothing. Same for template.data: {}.

Worth noting environments/function/fn.py::_external_secret_spec uses plain truthiness (if spec.data:, if template.data:) for the same fields, so the two functions in this PR disagree with each other. Only reachable for a hand-written SharedAWSSecret XR, so the blast radius is small — or None on both lines would restore parity and internal consistency.

2. (minor) pyyaml is unpinned in the functions' pyproject.toml (pyyaml>=6.0), and the kubeconfig Secret's bytes now come from yaml.safe_dump

The tests pin pyyaml==6.0.2 exactly; the functions don't. Since c8772e7, a PyYAML emitter change would alter the base64 payload of the three kubeconfig Secrets on the next function rebuild — a live diff with no source change. That argument didn't exist before this commit; it does now. Suggest pinning pyyaml (and probably grpcio) to match the tests.

Nits

• The = vs : explanation is slightly imprecise. resources/__init__.py says the key was lost "because merging metadata had replaced the annotation". It's specifically the = override operator on the annotations entry. Worth stating precisely, because it's the only thing that explains why upboundreposet keys its ProviderConfig providerConfigUpbound while environments keys the identical-looking one by name — a future reader will otherwise read that as a bug.
• No Python linting in CI. ~1,500 lines of new Python, and # noqa:FBT001 in the scaffolding implies ruff ran locally, but only yamllint (scoped to apis/) runs in CI. A ruff check step would be cheap insurance.
• environments/function/fn.py:47 parse_bootstrap_kubeconfig raises TypeError if clusters[0].cluster.server is absent (re.sub on None), where KCL's ?. degraded to Undefined. Narrow, but it turns a malformed kubeconfig from "environment doesn't initialise" into "function errors".
• main.py swallows startup failures (except Exception: click.echo(...), exit 0). That's up function generate scaffolding and identical in all three, so probably leave it — just noting that a broken function container exits cleanly instead of crash-looping.
• UP_MAX_CONCURRENCY=1 is well justified and correctly flagged as temporary, but there's no link to an up issue. A tracking link in the comment would make it removable later rather than permanent.
• functions/*/README.md are still the generator boilerplate ("You may fill in details…").
• The function/common symlinks work in CI, but will break on a Windows checkout without core.symlinks. Probably fine for this repo.

[ytsarev]

Thanks, this was a careful review. Addressed in e3505c8, 10b0e8b and 498ee8e:

1. Empty data / template.data: fixed. Confirmed with a failing test first: data: [] replaced the default dataFrom extract, so the external secret synced nothing. Both now use or None, matching the KCL and environments. Covered by test-sharedawssecret-empty-external-secret-data.

2. PyYAML pin: done. It's pinned to 6.0.2, the version the tests build their expected kubeconfig with, since the two have to agree. The kubeconfig byte assertions still pass, so the pin didn't move the bytes. grpcio is pinned to 1.84.0, which SDK 0.15.1 already required exactly.

Nits:

• parse_bootstrap_kubeconfig: reproduced the TypeError. A kubeconfig without server or the Spaces extension now yields no coordinates, so the Environment stays uninitialised. Covered by test-environment-malformed-bootstrap-kubeconfig.
• = vs :: the resources/ docstring now names the operators precisely, and uses the two ProviderConfigs as the example.
• ruff: there's a CI job now, with ruff pinned and a repo ruff.toml. It found an unused variable, dict() calls and unsorted imports, all fixed.
• main.py: left as generated, per your suggestion; ruff exempts just those two rules there.
• Function READMEs: replaced with real ones.
• Windows symlinks: noted in the README.
• UP_MAX_CONCURRENCY link: I'll add it once the upstream up issue is filed.

[kaessert]

Dry-run against production inputs: Python matches KCL.

I rendered every XR that runs in production through both main's KCL functions (4bdfb81) and this PR's Python functions (498ee8e) and compared the outputs. It was fully offline: a read-only snapshot, with secrets redacted and converted to the v2 XR shape, rendered with crossplane render / up composition render.

Kind	XRs	Composed resources	Result
Environment	5	81	intended diffs only
SharedAWSSecret	2 (+1 nested in an Environment)	15 (+8)	identical
UpboundRepoSet	2	560	intended diffs only

• Both sides produce the same composition keys, so nothing would be replaced. The desired XR status and conditions are equal, and neither side emits any results.
• The only differences are the intended ones:
  • kubeconfig Secrets are now written with yaml.safe_dump (c8772e7). The bytes differ, but they're equal once parsed.
  • crosslane.io/external-name is now crossplane.io/external-name on Repositories (3968b2d). The values are unchanged.
  • KCL's debug annotation foo: str(oxrSpec) on the ControlPlane is gone.
  • KCL's subjects[0].name: null in one admin binding is gone.
• Not exercised by production data: the empty external secret data fix and the malformed-kubeconfig handling. The new tests cover those.

From a behaviour point of view, I see nothing blocking this PR. The same run turned up v1→v2 migration issues that are already on main, independent of this PR. I'll raise those separately.

[kaessert]

Great job mate! LGTM ✅