Skip to content

feat: enable ManagementPolicies by default - #41

Merged
haarchri merged 1 commit into
mainfrom
enable-management-policies-by-default
Sep 24, 2026
Merged

feat: enable ManagementPolicies by default#41
haarchri merged 1 commit into
mainfrom
enable-management-policies-by-default

Conversation

@ytsarev

@ytsarev ytsarev commented Sep 23, 2026

Copy link
Copy Markdown
Member

Enables ManagementPolicies by default.

Why

ManagementPolicies has been stable in Crossplane for a long time, and both provider-aws and provider-kubernetes already ship with it on — verified on a live control plane, with no args set on either deployment. This provider still defaulted the flag to false and still described the feature as alpha.

The consequence is that any resource with a non-default managementPolicies fails to reconcile:

Synced=False  ReconcileError
`spec.managementPolicies` is set to a non-default value but the feature is not enabled:
[Create Observe Update LateInitialize]

Under Crossplane v2 this stops being an edge case. Namespaced managed resources have no deletionPolicy field at all, so managementPolicies is the only way to express "do not delete the external resource". Orphaning a Repository or a Team is simply not possible without this — and platform-ref-upbound orphans repositories by default, precisely so that deleting an XR does not destroy published packages.

It was also awkward to turn on. Upbound Spaces' admission webhook rejects arbitrary args on a package runtime:

deploymentruntimeconfig must not set deploymentTemplate.spec properties beyond
--debug (or -d) arg, --poll=<duration>, 0 or 1 replicas, environment variables
for the package-runtime container, and template.metadata.annotations[...]

so the only route on a managed control plane is the ENABLE_MANAGEMENT_POLICIES env var via a DeploymentRuntimeConfig — not obvious from the flag name, and easy to get wrong by reaching for args first.

What changed

  • --enable-management-policies now defaults to true; --enable-management-policies=false remains as an opt-out.
  • Help text says so.
  • Internal constant renamed EnableAlphaManagementPolicies → EnableManagementPolicies (12 controllers), and the startup log no longer claims "Alpha feature enabled". internal/ only, no API change.

Verification

go build ./..., go vet ./... and go test ./... all pass.

Behaviour was confirmed against a live Upbound control plane (Crossplane 2.4.1-up.1) before this change, by injecting the env var through a DeploymentRuntimeConfig: a composed Repository carrying ["Create","Observe","Update","LateInitialize"] moved from the feature-not-enabled error to a normal auth failure against a deliberately invalid token, with the policy accepted as stored. This PR makes that the default rather than something each operator has to discover.

🤖 Generated with Claude Code

@ytsarev @claude
feat: enable ManagementPolicies by default
1695490 
ManagementPolicies has been stable in Crossplane for a long time, and
provider-aws and provider-kubernetes already run with it on. This provider
still gated it behind a flag defaulting to false, and still described it as
alpha.

The practical consequence is that any resource with a non-default
managementPolicies silently fails to reconcile:

  `spec.managementPolicies` is set to a non-default value but the feature
  is not enabled: [Create Observe Update LateInitialize]

That is not an edge case under Crossplane v2. Namespaced managed resources
have no deletionPolicy field at all, so managementPolicies is the only way
to express "do not delete the external resource on delete". Orphaning a
Repository or a Team - which platform-ref-upbound does by default - is
impossible without it.

It was also awkward to turn on. Upbound Spaces' admission webhook rejects
arbitrary args on a package runtime, so the only route was the
ENABLE_MANAGEMENT_POLICIES env var via a DeploymentRuntimeConfig, which is
not obvious from the flag name alone.

Flips the default to true, keeping --enable-management-policies=false as an
opt-out, and renames the internal flag constant to drop "Alpha" now that it
is on by default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@haarchri
haarchri merged commit 242e775 into main Sep 24, 2026
6 checks passed
ytsarev added a commit to upbound/platform-ref-upbound that referenced this pull request Sep 24, 2026
@ytsarev @claude
docs: mark the ManagementPolicies workaround as temporary
dfe2238 
upbound/provider-upbound#41 flips --enable-management-policies to true by
default and is merged, but no release contains it yet - v1.1.1 is still the
latest and is what upbound.yaml pins, so the DeploymentRuntimeConfig is still
required today.

Records the removal condition in both the README and the e2e suite, so the
workaround does not outlive the fix: bump provider-upbound once a release
ships with it, then delete step 5a and the enable-management-policies runtime
config.

Also notes explicitly that step 5b is NOT in the same category. Server-side
apply is the correct default for provider-kubernetes; the Spaces API gateway
is the exception, so disabling it stays a permanent part of this setup rather
than something awaiting an upstream fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ytsarev ytsarev mentioned this pull request Sep 24, 2026
Merged
ytsarev added a commit to upbound/platform-ref-upbound that referenced this pull request Sep 25, 2026
@ytsarev @claude
feat!: migrate to Crossplane v2 namespaced APIs and the v2 provider l…
1e9b473 
…ine (#37)

* feat(deps)!: move project to v2alpha1 and bump providers to the v2 line

Adopts the meta.dev.upbound.io/v2alpha1 project format and raises the
Crossplane constraint to >=v2.0.0-0.

Also fixes a dependency deadlock. The old constraints pinned
provider-kubernetes to v0 and provider-upbound to v0.9.1. On a control
plane where newer versions are already installed, Crossplane's Lock
resolver (with --enable-dependency-version-upgrades) prunes them as
outdated, cannot find an upgrade that satisfies the constraint, and
aborts. Because the resolver only processes implied[0] per reconcile, it
never reaches the embedded function packages, which are then reported as
"missing dependencies" - a symptom that hides the real cause.

  provider-family-aws          -> v2.8.1  (now pinned explicitly)
  provider-aws-iam             -> v2.8.1
  provider-aws-secretsmanager  -> v2.8.1
  provider-kubernetes    v0    -> v1.3.3
  provider-upbound       v0.9.1 -> v1.1.1
  function-auto-ready  >=v0.0.0 -> v0.7.0

These versions ship the modern .m. API groups alongside the legacy ones,
so this commit stands alone: the existing v1 compositions and tests are
unaffected and stay green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* feat!: migrate to Crossplane v2 namespaced APIs and drop the X prefix

XRDs move to apiextensions.crossplane.io/v2 with scope: Namespaced, and
the X prefix is dropped now that v2 has no claims:

  XEnvironment     -> Environment
  XSharedAWSSecret -> SharedAWSSecret
  XUpboundRepoSet  -> UpboundRepoSet

Compositions stay on apiextensions.crossplane.io/v1 - there is no v2
Composition. Function directories are renamed to match, which changes the
published OCI paths to platform-ref-upbound_{environments,sharedawssecret,
upboundreposet}.

Composed resources move to the modern .m. provider groups. Note the
kubernetes Object version moves *backwards*, v1alpha2 -> v1alpha1, because
the modern group only publishes v1alpha1:

  kubernetes.crossplane.io/v1alpha2   -> kubernetes.m.crossplane.io/v1alpha1
  iam.aws.upbound.io/v1beta1          -> iam.aws.m.upbound.io/v1beta1
  secretsmanager.aws.upbound.io/..    -> secretsmanager.aws.m.upbound.io/..
  aws.upbound.io/v1beta1              -> aws.m.upbound.io/v1beta1
  iam.upbound.io/v1alpha1             -> iam.m.upbound.io/v1alpha1
  repository.upbound.io/v1alpha1      -> repository.m.upbound.io/v1alpha1
  upbound.io/v1alpha1                 -> m.upbound.io/v1alpha1
  apiextensions.crossplane.io Usage   -> protection.crossplane.io/v1beta1

Namespaced managed resources have no deletionPolicy field, so the XR-level
deletionPolicy parameter is retained and translated per resource via a new
utils._managementPolicies helper: Delete -> ["*"], Orphan -> ["Create",
"Observe","Update","LateInitialize"]. providerConfigRef now requires kind;
every reference points at a namespaced ProviderConfig this project itself
composes. writeConnectionSecretToRef loses its namespace field, so the IAM
AccessKey connection secret now lands in the XR's namespace - the
references.patchesFrom block that copies it follows it rather than reading
a hardcoded "default".

Two v2 behaviours worth recording, both of which fail silently:

  - A namespaced XR carries spec.crossplane, whose resourceRefs Crossplane
    fills with plain dicts. Spreading the whole observed spec into the
    typed XR schema then fails to type-check, so the functions take only
    spec.parameters.
  - An observed composed resource must carry metadata.namespace matching
    the XR, or crossplane render does not associate it and ocds lookups
    return nothing, silently disabling observed-state branches.

Also fixes a pre-existing failure in the no-cloudprovider suite, which
injected an XR with an empty spec into extraResources/observedResources;
up test run now sources the observed composite from there, wiping the spec
the xrPath example provides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: document the v2 namespaced API and the ManagementPolicies prerequisite

Records the breaking changes for operators: the kinds lose their X prefix,
XRs are namespaced (kubectl get environments -n <ns>), and the bootstrap
ProviderConfig created by hand is now the namespaced
kubernetes.m.crossplane.io/v1alpha1 kind in the XR's namespace.

Adds a required step to enable ManagementPolicies on provider-upbound.
Orphaning is expressed with managementPolicies in v2, but provider-upbound
gates that behind an alpha feature that defaults to off, so without this
the composed Repository and Team never reconcile:

  `spec.managementPolicies` is set to a non-default value but the feature
  is not enabled

It must be set as an environment variable rather than a container arg:
Upbound Spaces' admission webhook rejects arbitrary args on a package
runtime but permits env vars, and the provider reads
ENABLE_MANAGEMENT_POLICIES as an alias for its --enable-management-policies
flag. provider-aws and provider-kubernetes already have it on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: emit base64 `data` instead of `stringData` in composed Secrets

provider-kubernetes v1.3.3 applies server-side and records field ownership
for exactly what it wrote. Kubernetes converts `stringData` to `data` on
write, so the owned field is never present on the stored object and every
subsequent observe fails:

  failed calling ExtractInto for unstructured: unable to convert managed
  fields for provider-kubernetes/<name> to unstructured, expected map,
  got <nil>

The Objects then never reconcile, the ProviderConfigs built from those
kubeconfig Secrets never become usable, and the Objects cannot even be
deleted, because finalization also needs an observe.

Found on a live control plane; composition tests passed throughout, since a
render cannot show what the API server does to stringData on write. Verified
by before/after event counts on the same cluster: 27
CannotObserveExternalResource events before, zero across 11 minutes after,
with the stored Secrets now carrying data.kubeconfig and no stringData.

Only surfaced now because this is the first release on provider-kubernetes
v1; the v0 line did not apply server-side.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: harden optional-field guards against None

Two pipeline-aborting crashes seen on a live control plane, both from the
same trap: an absent optional field yields None, not Undefined, so a guard
written against Undefined passes and the following attribute access fails.

  main.k:76  invalid value 'NoneType' to load attribute 'org'
  sharedawssecret/main.k  invalid value 'UndefinedType' to load attribute 'create'

_initReady now uses `?.` on every hop rather than testing the parent once.
The first reconcile of any Environment hit this, before status.upbound was
populated; it self-recovered, so it only showed up as a one-shot error event.

secretsManagerSecret is now treated as opt-out: only an explicit `false`
disables creation. The first attempt at this fix tested `== Undefined`,
which was worse than the crash - it silently skipped creating the secret
altogether. The new test caught that, which is the whole reason it exists.

Adds test-sharedawssecret-no-secretsmanager-block. All four existing
sharedawssecret cases set secretsManagerSecret, so nothing covered the
omission - which is exactly the shape Environment produces when sharedSecret
is given without one. Confirmed the test bites: reverting the fix reproduces
"invalid value 'UndefinedType' to load attribute 'create'" while the other
four stay green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: require disabling server-side apply on provider-kubernetes

provider-kubernetes v1 defaults --enable-server-side-apply to true. The
Upbound Spaces API rejects apply patches:

  Unsupported patch format. Only merge and json patch are supported.

so every object this configuration creates through a Spaces-backed
ProviderConfig fails - the environment group Namespace, the ControlPlane,
the SharedSecretStore and the SharedExternalSecret. Objects targeting a
normal control-plane API are unaffected, which is what makes the split so
easy to misread as a composition bug.

Setting ENABLE_SERVER_SIDE_APPLY=false selects the provider's
PatchingResourceSyncer, which uses a merge patch that Spaces accepts. As
with ManagementPolicies it has to go in as an environment variable, since
the Spaces admission webhook rejects arbitrary args on a package runtime.

Verified on a live control plane: with this set, the group Namespace and a
real ControlPlane (Ready=True, Healthy=True, ControlPlaneProvisioned=True)
were created in the Space, along with the SharedSecretStore and
SharedExternalSecret. 10 of 11 composed Objects reconcile; the remaining one
needs an argocd namespace on the target control plane and is environmental.

Step 6 now covers both provider runtime prerequisites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: clarify that it is the Spaces API gateway, not control planes, that rejects SSA

Spaces control planes are ordinary Kubernetes API servers and accept
server-side apply; verified with `kubectl apply --server-side
--dry-run=server` against a control plane endpoint (serverside-applied) and
against the gateway (BadRequest). The gateway at https://<spaceHost>, which
serves groups and spaces.upbound.io resources, is what rejects apply
patches - "invalid patch type" for core kinds, "Unsupported patch format.
Only merge and json patch are supported." for spaces.upbound.io kinds.

That distinction is what makes the failure split by ProviderConfig: the
<env>-space and <group>-group configs target the gateway, the
control-plane-scoped one does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: guard _initReady by truthiness, not `!= Undefined`

My previous attempt at this was wrong. Switching to `?.` on every hop stopped
the crash at the guard itself but let an empty status through, so the
pipeline died four lines later instead:

  main.k:137  envGroupName = "{}-{}".format(_oxrStatusUpbound.bootstrapGroup, ...)
              invalid value 'NoneType' to load attribute 'bootstrapGroup'

In KCL both `None != Undefined` and `None?.field != Undefined` evaluate to
true, so no Undefined-based test can distinguish "status absent" from
"status present". These fields are all non-empty strings once set, so a
plain truthy test is the only form correct for None, Undefined and "" alike.
This is the same trap as the secretsManagerSecret guard, which is why that
one is written as an explicit `== False` comparison.

Adds test-environment-uninitialized, which reproduces the live shape by
setting `status = {upbound = None}` rather than omitting status. That
distinction is the whole point: with status merely absent the test passes
against the broken guard too, because a render supplies Undefined where a
real API server gives None. With None it fails on the original code with the
exact live error, and passes on this one.

The crash predates the v2 migration; it was only ever a one-shot error on a
fresh XR that self-recovered on the next reconcile, so it never surfaced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: restructure the README as a followable guide

The setup steps had grown to nine, several of them interleaved with
explanation, and the reference material repeated itself across an overview
list, a "what does X do" section and the per-API sections.

Reorganised into How it works / Prerequisites / Getting started / API
reference / Development, with parameter tables per API instead of prose
bullet lists. Net 552 -> 457 lines despite adding material.

The important addition is an architecture section explaining the two-control-
plane model, which was not documented anywhere. Everything - the XRs and all
composed resources - lives on the bootstrap control plane; the environment
control plane it creates starts empty by design, because nothing here deploys
into it. Argo CD does, via the cluster-registration secret written back to
the bootstrap plane. Without that, an empty `up alpha query managed` against
a new environment reads as a failure when it is the expected result.

Also now documented, all learned from a live run:
  - Environment derives spaceHost/org/bootstrapGroup/bootstrapCtp by parsing
    the bootstrap kubeconfig, which is why status.upbound is briefly empty
  - web identity as the preferred AWS path, with the exact OIDC subject
  - the OIDC provider being an account-wide singleton, and why Orphan matters
  - SharedAWSSecret issuing a long-lived IAM access key, and why
  - createArgoSecret needing Argo CD on the bootstrap plane, with a pointer
    to addon-argocd-core
  - which Spaces endpoint rejects server-side apply, and which does not

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: never delete an adopted OIDC provider, and add the e2e suite

An OIDC provider supplied via aws.providerRole.oidcProviderArn is adopted,
not created - and AWS allows exactly one provider per URL per account, so
proidc.upbound.io is shared by every Upbound integration in that account.
It was inheriting the XR-level deletionPolicy, meaning a single
`deletionPolicy: Delete` teardown would have deleted it and broken all of
them. Adoption now implies orphaning; a provider the composition actually
creates still follows deletionPolicy.

No new XRD field: "do not delete what you did not create" is the correct
behaviour rather than something a user should have to opt into.

Adds test-environment-adopted-oidc-is-orphaned, which pins both halves -
the adopted provider orphaned while the Role beside it still honours Delete.
Verified it bites: reverting the fix fails it with
`spec.managementPolicies: Invalid value: ["*"]`.

Adds tests/e2etest-environment, which provisions a real group, control plane,
IAM role and Secrets Manager secret, then asserts Ready. Notes:

  - Secrets reach the test through file.read_env on UP_-prefixed variables;
    manifest generation runs in a container and nothing else crosses. Each
    read asserts rather than silently emitting an empty credential.
  - AWS uses web identity, so no AWS credential exists anywhere.
  - The bootstrap ProviderConfig uses InjectedIdentity, because the test's
    own control plane name is not knowable when the manifest is generated.
    The kubeconfig Secret beside it only has to parse - the composition reads
    it solely to derive spaceHost/org/bootstrapGroup/bootstrapCtp.
  - Both providers are declared in initResources with their
    DeploymentRuntimeConfigs already bound, since neither default works
    against Spaces.
  - deletionPolicy: Delete so CI cleans up after itself, which is only safe
    because of the adopted-OIDC fix above.

Requires a new UP_E2E_SPACE_HOST repo secret; the other UP_E2E_* already exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: mark the ManagementPolicies workaround as temporary

upbound/provider-upbound#41 flips --enable-management-policies to true by
default and is merged, but no release contains it yet - v1.1.1 is still the
latest and is what upbound.yaml pins, so the DeploymentRuntimeConfig is still
required today.

Records the removal condition in both the README and the e2e suite, so the
workaround does not outlive the fix: bump provider-upbound once a release
ships with it, then delete step 5a and the enable-management-policies runtime
config.

Also notes explicitly that step 5b is NOT in the same category. Server-side
apply is the correct default for provider-kubernetes; the Spaces API gateway
is the exception, so disabling it stays a permanent part of this setup rather
than something awaiting an upstream fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor: model Crossplane's own CRDs instead of hand-writing dicts

Adds Crossplane's CRDs as a git apiDependency, pinned to release-2.4 to match
the Crossplane the target control planes run, so pkg.crossplane.io and
protection.crossplane.io objects get generated models.

Two places were writing untyped dicts for want of them:

  - pKubernetesHelper.k built its protection.crossplane.io/v1beta1 Usage by
    hand. This predates the e2e work; the v2 migration only changed the
    apiVersion string inside it.
  - the e2e suite's initResources built DeploymentRuntimeConfig and Provider
    by hand, which I had written off as not worth a dependency. It is worth
    it - a typo in any of those literals would have surfaced as a confusing
    server-side rejection during a 40-minute e2e run rather than at build.

What stays a literal is deliberate: apiVersion/kind pairs inside
`runtimeConfigRef` and inside Usage's `of`/`by` are references to a type, not
objects being constructed, and every other remaining string sits inside an
Object's forProvider.manifest - a resource on a *remote* cluster, which this
project has no models for by definition.

Costs a network fetch of the crossplane repo at build time, alongside the
existing k8s api-dependency.

13/13 composition tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): scope composition tests to tests/test-*

`up test run tests/*` generates manifests for every directory it matches,
including e2e ones it will not execute. tests/e2etest-environment asserts
that its UP_E2E_* variables are set, so it failed generation in the
composition-tests job, taking the whole run down.

The assert is deliberate and stays - the alternative is a run that provisions
a real control plane and real AWS resources before discovering the credential
was empty. It is the glob that was wrong.

composition-tests now runs tests/test-*; the e2e workflow keeps tests/* since
it supplies the variables. README says the same and explains why, because
`tests/*` is the obvious thing to type.

Verified with the e2e variables explicitly unset: 13/13.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* style: fix yamllint sequence indentation in the Environment XRD

Five sequences were indented +2 under their key while the rest of the file
puts sequence items at the parent's indent. With yamllint's default
`spaces: consistent` the minority style is an error, so the file has been
failing the linter - on main as well, at the same five places one line
earlier.

They surfaced on this PR rather than earlier because the yamllint workflow
runs reviewdog with `filter_mode: nofilter`, which reports the whole file
rather than just changed lines, and renaming apis/xenvironments ->
apis/environments makes git treat the file as touched. The job itself still
reported success - reviewdog's github-pr-review reporter posts comments but
does not fail the run, despite fail_on_error: true - so this was only visible
as five review comments.

Whitespace only: `git diff -w` is empty and the parsed YAML is byte-identical
after a round trip. `yamllint apis/` is now clean, 13/13 tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(e2e): read the variable names the workflow actually exports

The e2e suite asked for UP_E2E_API_TOKEN / UP_E2E_ORG / UP_E2E_GROUP /
UP_E2E_SPACE_HOST. The workflow resolves the UP_E2E_* secrets into
UP_API_TOKEN / UP_ORG / UP_GROUP and exports those, so every one of the
suite's asserts would have fired and the run would have died at manifest
generation the moment someone applied the run-e2e-tests label.

The suite now reads the names the workflow exports. UP_SPACE_HOST is gone -
the host is derived from UP_SPACE instead, which the workflow also uses for
its `up ctx` step, so the two cannot drift. That step had the space
hardcoded; it now reads UP_SPACE as well, defaulting to the value it
previously hardcoded.

No new repository secret is needed after all.

Verified three ways: composition tests pass with nothing exported (13/13),
the e2e generates with the workflow's variables set, and it still refuses
loudly when they are missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(e2e): default UP_SPACE instead of asserting on it

The first labelled run failed at manifest generation with "UP_SPACE must be
set". e2e.yaml triggers on pull_request_target, which takes the workflow
definition from the base branch and the code from the PR head - so the
UP_SPACE export added to the workflow in this PR is not set when this PR is
tested, while the test demanding it is. Any variable introduced alongside a
test in the same PR hits this.

UP_SPACE now falls back to the space the workflow switches to. That is a
different kind of value from the credentials: a missing token has no safe
default and must fail loudly, whereas the space name only has to agree with
the `up ctx` step, and it does.

The credential asserts are unchanged - UP_API_TOKEN, UP_ORG and UP_GROUP
still fail generation when unset.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: do not report Ready before the Environment has initialised

Until the bootstrap kubeconfig is observed and status.upbound populated, the
composition emits exactly one composed resource: the observer watching that
kubeconfig Secret. It goes ready as soon as the Secret exists, so
function-auto-ready saw "every composed resource is ready" and reported the
whole Environment as Ready - before a single group, control plane,
ProviderConfig or IAM resource had been created.

Anything polling Ready to decide the environment is up got a false positive
for that window. It is easy to miss by hand, because by the time you look the
XR has genuinely converged; it only bites something checking within seconds
of creation.

The observer is now marked krm.kcl.dev/ready False until _initReady, using
the same annotation the ProviderConfigs here already use to force readiness.
Renders confirm both directions: uninitialised, the XR is not Ready and names
the observer as the unready resource; initialised, the observer drops out of
the unready list and readiness follows the real resources.

Found because the e2e suite passed in two seconds flat. The suite asserts
Ready, so it was asserting this false positive and tearing down before any
provisioning began - a green run that tested nothing. With this fix that
assertion becomes meaningful.

No composition test guards it: the signal is the composite's Ready condition,
and its conditions carry lastTransitionTime, which assertResources cannot
match (list assertions are exact). The e2e is the guard.

* fix(e2e): set uptest.upbound.io/timeout on the Environment XR

spec.timeoutSeconds does not reach uptest's per-resource assertion, which
defaults to 30 seconds; only this annotation overrides it. I had wrongly
concluded timeoutSeconds was propagating because an assert ran for 292s -
that was uptest's own retry wrapper, not the 3600 configured below.

The timeout has to outlast provider *installation*, not just provisioning.
`up test run` begins asserting once the configuration package reports ready,
which is not the same as its dependency providers being ready. Provider
images are large, so on a fresh control plane there is a multi-minute window
where the managed resource CRDs do not exist and the composition cannot
succeed.

That window is what made the two runs disagree. Both were fresh control
planes; the first got providers ready within seconds and initialised, the
second was still installing and the XR never advanced past the bootstrap
observer. Same code, different install timing - a flake, not a defect in the
composition.

* fix: restore the observer's composition resource name

My readiness change added an annotations block onto metadata built by
utils._metadata(), assuming `|` merged into the annotations map. It replaces
it. That silently dropped krm.kcl.dev/composition-resource-name, so the
observer was filed under its object name instead of "observedCtpKubeconfig".

main.k looks it up by that key:

    ocds.observedCtpKubeconfig?.Resource?.status?.atProvider?.manifest?.data?.kubeconfig

which then returned nothing. The kubeconfig was observed correctly and the
regexes match it fine - the function simply could not find the resource. So
status.upbound stayed {} and the Environment never initialised, which is what
the last two e2e runs were reporting.

Diagnosed by keeping a failed e2e control plane alive (skipDelete plus
--skip-control-plane-cleanup) and reading the live objects: the observer had
data.kubeconfig present, the XR had "upbound":{}, and the annotation read
crossplane.io/composition-resource-name: e2e-bootstrap-ctp-kubeconfig-observed.

Both annotations are now set explicitly rather than merged, with a comment
about the replace-not-merge behaviour.

Note the same pattern exists on several ProviderConfigs here, which lose
their resource name the same way. It is harmless - nothing looks those up by
key - and changing them now would rename live composed resources, orphaning
and recreating them in existing environments. Left alone deliberately.

* fix: compose the group ProviderConfig regardless of createGroup

The ControlPlane and the nested SharedAWSSecret both reference the
environment group's ProviderConfig, but it was only composed when
createGroup was true. Setting createGroup: false therefore left both
pointing at a ProviderConfig that never existed.

Only the group object itself and the space-level ProviderConfig that
creates it belong behind createGroup; managing resources inside the
group needs that ProviderConfig either way.

Covers it with tests/test-environment-existing-group.

* test(e2e): deploy into a pre-existing group

CI authenticates as a team-scoped robot. Upbound grants RBAC per group -
a team is bound to one group with an ObjectRoleBinding, and nothing
grants "create any group" short of an organization owner - so the robot
could not create the <UP_GROUP>-e2e group the environment provisions,
and every Object landing inside it came back forbidden.

The group and its ObjectRoleBinding are now provisioned once by hand and
outlive a run; the suite sets createGroup: false. Teardown still removes
the control plane, secret stores and AWS resources.

Documents the identity an Environment needs, and the pre-created group
alternative, in the README.

* feat: expose recoveryWindowInDays on the Secrets Manager secret

AWS does not delete a Secrets Manager secret outright - it schedules it
and reserves the name for the recovery window, 30 days by default. An
environment torn down and recreated under the same name fails with
"You can't create this secret because a secret with this name is already
scheduled for deletion" until the window closes, which is what the e2e
suite hit on its first successful teardown.

Exposes the window on SharedAWSSecret, passes it through from
Environment, and sets 0 in the e2e so the suite is re-runnable. Left
unset the AWS default still applies, so nothing changes for anyone who
wants the secret recoverable.

The pass-through guards on `!= Undefined` rather than truthiness: 0 is
both the meaningful value and a falsy one.

* fix: depend on version ranges rather than exact versions

A Configuration's dependency version is a constraint, and an exact
version is the narrowest one there is. A control plane that already runs
a newer patch or minor of provider-kubernetes (or any other dependency)
cannot satisfy it, and the Lock fails with DependencyResolutionFailed -
the same way v0.7.0 failed on merck, just in the other direction.

Use floor-plus-major ranges: the tested version is the minimum, and
anything up to the next major is accepted. The k8s API dependency stays
exact; it is a git ref, not a package constraint.

* fix: keep Environment names unique across namespaces

With a Namespaced XRD, team-a/prod and team-b/prod are both valid, but
every name outside the XR's namespace was built from metadata.name
alone. Both XRs mapped to the same Upbound group - and so the same
ControlPlane, Team, Robot and Argo secret - the same AWS IAM and Secrets
Manager names, and the same kubeconfig Secrets in the bootstrap control
plane's `default` namespace. With deletionPolicy: Delete, deleting one
tore down the other.

- The group is now <bootstrapGroup>-<namespace>-<name>. Everything
  org-wide or AWS-side derives from it.
- Kubeconfig Secrets are written to the XR's namespace, and the
  ProviderConfigs read them from there.
- The IAM Role name, which the namespace lengthens, gets the same
  64-character truncation SharedAWSSecret applies to its IAM user.

Covered by tests/test-environment-namespaced-names. The e2e's
pre-created group is renamed to match.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haarchri haarchri haarchri approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ytsarev @haarchri