feat: drop default registry for packages

[phisco]

Description of your changes

Fixes https://github.com/crossplane/crossplane/issues/6433.

Dropping defaulting and enforcing strict validation where needed.

Decided to address separately adding a validating webhook or pattern on Packages to enforce spec.package to be fully qualified.

Had to rebuild and update the following images to xpkg.crossplane.io, first to avoid relying on xpkg.upbound.io, but also because some didn't fully specify the registry:

• xpkg.upbound.io/crossplane/e2e-depends-on-provider-nop
• xpkg.crossplane.io/crossplane/e2e-nested-configuration

Given that not all tags we use for crossplane-contrib/provider-nop are available through xpkg.crossplane.io, for consistency I moved all references to use xpkg.upbound.io/crossplane-contrib/provider-nop.

I have:

• [x] Read and followed Crossplane's contribution process.
• [x] Run earthly +reviewable to ensure this PR is ready for review.
• [x] Added or updated unit tests.
• [x] Added or updated e2e tests.
• [ ] ~Linked a PR or a docs tracking issue to document this change.~
• [x] Added backport release-x.y labels to auto-backport this PR.
• [x] Followed the API promotion workflow if this PR introduces, removes, or promotes an API.

Need help with this checklist? See the cheat sheet.

[phisco]

a possible regex to validate packages could be ^[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?)+(\:[0-9]+)?\/[a-z0-9]+([._\-][a-z0-9]+)*(\/[a-z0-9]+([._\-][a-z0-9]+)*)*\:[a-zA-Z0-9][a-zA-Z0-9._\-]*$, but I'm not sure it's worth enforcing, maybe we should just improve the runtime error... WDYT?

[negz]

a possible regex to validate packages

I definitely don't love how complex that regex is, heh.

Ideally we'd use the same logic as name.StrictValidation - so e.g. a webhook that validated using that. On the other hand iirc we no longer have any web hooks doing validation in core, and instead do everything with CEL. So I'm reluctant to add a a validation webhook.

What do you think about using a simpler regex that doesn't attempt to fully validate an OCI reference, but instead only rejects OCI references that are missing a registry? e.g. Something like:

^.+/.+/.+/.+$

[phisco]

What do you think about using a simpler regex that doesn't attempt to fully validate an OCI reference, but instead only rejects OCI references that are missing a registry? e.g. Something like:

Good call! Unfortunately, I think it might not be that easy, an oci image can also have a registry and single slash, most registries don't allow that because they are more geared toward multi-tenancy, but it's still valid and we should accept it.

Btw, forgot to mention this is also going to enforce a tag to be specified, it won't default to latest if not specified. I just updated the PR description. I think this is desirable, but LMK.

So, the regex should ensure the following:

• has a registry set: so it starts with at least one dot.
• has at least one slash followed by some image name or repository.
• has :tag and/or @sha256... set.

Something like: ^[^\.\/]+(\.[^\.\/]+)+(\/[^\/:@]+)+(:[^:@]+(@sha256.+)?|@sha256.+)$

[jbw976]

just starting to look at this, but first question from the discussion so far is why would we need to write our own regex for validation? Would the validation functionality in https://github.com/google/go-containerregistry not be reusable for our needs?

Some example validation helpers:

• https://github.com/google/go-containerregistry/blob/main/pkg/name/ref.go#L40
• https://github.com/google/go-containerregistry/blob/main/pkg/name/registry.go#L110

edit: is it so we don't have to run code like a validation webhook, and so it could be done more statically like with CEL?

[phisco]

@jbw976

why would we need to write our own regex for validation? Would the validation functionality in https://github.com/google/go-containerregistry not be reusable for our needs?

is it so we don't have to run code like a validation webhook, and so it could be done more statically like with CEL?

yes, currently we don't have any webhook for packages, introducing one just for this would be a shame.

Also, in general for the UX: what exactly will be the experience when someone has a package installed in the control plane without an explicit registry and then they upgrade to v2? or if they have a package that expresses a dependency without an explicit registry and then they upgrade to v2?

Currently, the experience won't be the best, to be honest. That's why I really want to introduce some kind of validation. Core resources, like pods, do error out immediately when the provided image is malformed, but in our case, it's not that simple. Packages have dependencies, which we can't easily reject without resolving the whole tree, which we should definitely not do in a validating webhook, nor can it be done through CEL validation. So, a simple CEL rule and some clearer events would make sense.

apiVersion: pkg.crossplane.io/v1
kind: Configuration
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"pkg.crossplane.io/v1","kind":"Configuration","metadata":{"annotations":{},"name":"configuration-downgrade"},"spec":{"package":"crossplane/e2e-nested-configuration:v0.2.0"}}
  creationTimestamp: "2025-06-23T19:21:45Z"
  generation: 1
  name: configuration-downgrade
  resourceVersion: "1811"
  uid: 675e51b6-d472-404c-9439-aa7d0ac8e77d
spec:
  ignoreCrossplaneConstraints: false
  package: crossplane/e2e-nested-configuration:v0.2.0
  packagePullPolicy: IfNotPresent
  revisionActivationPolicy: Automatic
  revisionHistoryLimit: 1
  skipDependencyResolution: false
status:
  conditions:
  - lastTransitionTime: "2025-06-23T19:21:45Z"
    message: 'cannot unpack package: package tag is not a valid reference: could not
      parse reference: crossplane/e2e-nested-configuration:v0.2.0'
    observedGeneration: 1
    reason: UnpackingPackage
    status: "False"
    type: Installed
  resolvedPackage: crossplane/e2e-nested-configuration:v0.2.0

As you can see, the error isn't really suggesting why the reference is invalid, but that's because the code we use is pretty opaque there.

I'd leave this for a quick follow-up PR if you agree.

[phisco]

with the regex above it would return: The Configuration "configuration-downgrade" is invalid: spec.package: Invalid value: "crossplane/e2e-nested-configuration:v0.2.0": spec.package in body should match '^[^\.\/]+(\.[^\.\/]+)+(\/[^\/:@]+)+(:[^:@]+(@sha256.+)?|@sha256.+)$' which is hardly more readable 😆

[phisco]

the regex can be run as a CEL rule, so we can customize the error message:

	// +kubebuilder:validation:XValidation:rule="self.matches('^[^\\\\.\\\\/]+(\\\\.[^\\\\.\\\\/]+)+(\\\\/[^\\\\/:@]+)+(:[^:@]+(@sha256.+)?|@sha256.+)$')",message="must be a fully qualified image name, including the registry, repository, and tag or digest. For example, 'registry.example.com/repo/package:tag' or 'registry.example.com/repo/package[:tag]@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'."

results in:

The Configuration "configuration-downgrade" is invalid: spec.package: Invalid value: "string": must be a fully qualified image name, including the registry, repository, and tag or digest. For example, 'registry.example.com/repo/package:tag' or 'registry.example.com/repo/package[:tag]@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'.

[phisco]

Regarding the migration, we should add a pre-upgrade hook checking that all packages are using a fully qualified image and suggest migrating before upgrading maybe?

[jbw976]

currently we don't have any webhook for packages, introducing one just for this would be a shame

agreed! 👍

As you can see, the error isn't really suggesting why the reference is invalid, but that's because the code we use is pretty opaque there.

I'd leave this for a quick follow-up PR if you agree.

That isn't all that terrible as it is now - definitely better than the "must match regex [crazy chars]" experience. The CEL rule with customized error message experience looks to be the best of what you've shown, but I may be starting to confuse two different paths in my mind:

1. when a package without an explicit registry is applied to the control plane - CEL rule validation could definitely happen here and reject the apply, right?
2. when an existing package without an explicit registry is already installed in v1.20 and the user upgrades to v2 - the CEL validation rule wouldn't be triggered on this path, and the current experience you shared that has a status condition and semi-opaque (but not toooooo awful) message would be the experience for a user to discover something is wrong?

would you mind describing those two scenarios a bit more, especially if I'm not understanding them correctly right now, along with your recommendation for how we should handle them, so we have it in one place? just trying to understand it all clearly, then I'm fine if we follow up on any UX improvement implementations in follow-up PRs.

we should add a pre-upgrade hook

i like this idea, but would be hesitant on it if it adds too much more complexity to the upgrade process. what do you think? feel free to include that in your concise recommendation 😂

[phisco]

when a package without an explicit registry is applied to the control plane - CEL rule validation could definitely happen here and reject the apply, right?

Yes, except for dependencies, those would be caught at runtime, with an error in the status of the root package.

when an existing package without an explicit registry is already installed in v1.20 and the user upgrades to v2 - the CEL validation rule wouldn't be triggered on this path, and the current experience you shared that has a status condition and semi-opaque (but not toooooo awful) message would be the experience for a user to discover something is wrong?

Yes, exactly. The pre-upgrade hook could just go through all packages installed and check they have a fully qualified image in the spec, a bash script could be enough.

@jbw976

[jbw976]

I think with your latest commits you've added the CEL validation rule with a custom error message, but no pre-upgrade hook - I think this is a fine approach for this PR and we can consider following up afterward to add the hook (or not).

So I think this is mostly looking good, but now that we have the CEL validation rule in place, I am worrying about a detail of its enforcement. I believe some OCI registries support arbitrary number of namespace segments in an image path, e.g. Oracle Container Registry that allows repo names like project01/acme-web-app/component1/subcomponent1. Azure allows something similar, e.g. contoso.azurecr.io/marketing/2017-fall/concertpromotions/campaign:218.42. So it's possible to have more segments than just the registry/org/repo:tag that is most commonly used.

Will our CEL validation be too strict here, and possibly reject valid package paths because they have too many segments?

[phisco]

@jbw976

Will our CEL validation be too strict here, and possibly reject valid package paths because they have too many segments?

No, it will allow that, see tests here.

[jbw976]

oh nice @phisco OK that's great thank you for confirming that! I also played around a bit myself and added a case for containers.oracle.net/project01/acme-web-app/component1/subcomponent1/container1:tag just to make sure 😇 - it doesn't persist to that set though, so don't worry, your list is still pristine 😉