diff --git a/cmd/crank/xpkg/install.go b/cmd/crank/xpkg/install.go index 710078d43a3..b09b1d7fda5 100644 --- a/cmd/crank/xpkg/install.go +++ b/cmd/crank/xpkg/install.go @@ -87,7 +87,7 @@ Examples: func (c *installCmd) Run(k *kong.Context, logger logging.Logger) error { //nolint:gocyclo // TODO(negz): Can anything be broken out here? pkgName := c.Name if pkgName == "" { - ref, err := name.ParseReference(c.Package, name.WithDefaultRegistry(DefaultRegistry)) + ref, err := name.ParseReference(c.Package, name.WithDefaultRegistry(xpkg.DefaultRegistry)) if err != nil { logger.Debug(errPkgIdentifier, "error", err) return errors.Wrap(err, errPkgIdentifier) diff --git a/cmd/crank/xpkg/push.go b/cmd/crank/xpkg/push.go index 50f29083186..0f7ea5fcd2b 100644 --- a/cmd/crank/xpkg/push.go +++ b/cmd/crank/xpkg/push.go @@ -54,9 +54,6 @@ const ( errFmtWriteIndex = "failed to push an OCI image index of %d packages" ) -// DefaultRegistry for pushing Crossplane packages. -const DefaultRegistry = "xpkg.upbound.io" - // pushCmd pushes a package. type pushCmd struct { // Arguments. @@ -94,7 +91,7 @@ func (c *pushCmd) AfterApply() error { // Run runs the push cmd. func (c *pushCmd) Run(logger logging.Logger) error { //nolint:gocyclo // This feels easier to read as-is. - tag, err := name.NewTag(c.Package, name.WithDefaultRegistry(DefaultRegistry)) + tag, err := name.NewTag(c.Package, name.WithDefaultRegistry(xpkg.DefaultRegistry)) if err != nil { return errors.Wrapf(err, errFmtNewTag, c.Package) } @@ -159,7 +156,7 @@ func (c *pushCmd) Run(logger logging.Logger) error { //nolint:gocyclo // This fe return errors.Wrapf(err, errFmtGetDigest, file) } n := fmt.Sprintf("%s@%s", tag.Repository.Name(), d.String()) - ref, err := name.NewDigest(n, name.WithDefaultRegistry(DefaultRegistry)) + ref, err := name.NewDigest(n, name.WithDefaultRegistry(xpkg.DefaultRegistry)) if err != nil { return errors.Wrapf(err, errFmtNewDigest, n, file) } diff --git a/cmd/crank/xpkg/update.go b/cmd/crank/xpkg/update.go index 60bc4781a6a..97f5322462e 100644 --- a/cmd/crank/xpkg/update.go +++ b/cmd/crank/xpkg/update.go @@ -64,7 +64,7 @@ Examples: func (c *updateCmd) Run(k *kong.Context, logger logging.Logger) error { pkgName := c.Name if pkgName == "" { - ref, err := name.ParseReference(c.Package, name.WithDefaultRegistry(DefaultRegistry)) + ref, err := name.ParseReference(c.Package, name.WithDefaultRegistry(xpkg.DefaultRegistry)) if err != nil { logger.Debug(errPkgIdentifier, "error", err) return errors.Wrap(err, errPkgIdentifier) diff --git a/cmd/crossplane/core/core.go b/cmd/crossplane/core/core.go index 7667af3e858..9697ac54232 100644 --- a/cmd/crossplane/core/core.go +++ b/cmd/crossplane/core/core.go @@ -26,7 +26,6 @@ import ( "time" "github.com/alecthomas/kong" - "github.com/google/go-containerregistry/pkg/name" "github.com/spf13/afero" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -70,7 +69,7 @@ type Command struct { // KongVars represent the kong variables associated with the CLI parser // required for the Registry default variable interpolation. var KongVars = kong.Vars{ - "default_registry": name.DefaultRegistry, + "default_registry": xpkg.DefaultRegistry, "default_user_agent": transport.DefaultUserAgent, } @@ -264,7 +263,6 @@ func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error { //noli Options: o, Namespace: c.Namespace, ServiceAccount: c.ServiceAccount, - Registry: c.Registry, FunctionRunner: functionRunner, } diff --git a/cmd/crossplane/rbac/rbac.go b/cmd/crossplane/rbac/rbac.go index 6435911cb8c..469d81a1cd3 100644 --- a/cmd/crossplane/rbac/rbac.go +++ b/cmd/crossplane/rbac/rbac.go @@ -22,7 +22,6 @@ import ( "time" "github.com/alecthomas/kong" - "github.com/google/go-containerregistry/pkg/name" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/leaderelection/resourcelock" ctrl "sigs.k8s.io/controller-runtime" @@ -35,6 +34,7 @@ import ( "github.com/crossplane/crossplane/internal/controller/rbac" rbaccontroller "github.com/crossplane/crossplane/internal/controller/rbac/controller" + "github.com/crossplane/crossplane/internal/xpkg" ) // Available RBAC management policies. @@ -53,7 +53,7 @@ var KongVars = kong.Vars{ ManagementPolicyBasic, }, ", "), - "default_registry": name.DefaultRegistry, + "rbac_default_registry": xpkg.DefaultRegistry, } // Command runs the crossplane RBAC controllers @@ -74,7 +74,7 @@ type startCommand struct { ProviderClusterRole string `name:"provider-clusterrole" help:"A ClusterRole enumerating the permissions provider packages may request."` LeaderElection bool `name:"leader-election" short:"l" help:"Use leader election for the controller manager." env:"LEADER_ELECTION"` - Registry string `short:"r" help:"Default registry used to fetch packages when not specified in tag." default:"${default_registry}" env:"REGISTRY"` + Registry string `short:"r" help:"Default registry used to fetch packages when not specified in tag." default:"${rbac_default_registry}" env:"REGISTRY"` ManagementPolicy string `name:"manage" short:"m" hidden:""` DeprecatedManagementPolicy string `name:"deprecated-manage" hidden:"" default:"${rbac_manage_default_var}" enum:"${rbac_manage_enum_var}"` diff --git a/internal/controller/apiextensions/controller/options.go b/internal/controller/apiextensions/controller/options.go index caef4d5aaf5..89870a05d61 100644 --- a/internal/controller/apiextensions/controller/options.go +++ b/internal/controller/apiextensions/controller/options.go @@ -35,10 +35,6 @@ type Options struct { // private registry authentication when pulling Composition Functions. ServiceAccount string - // Registry is the default registry to use when pulling containers for - // Composition Functions - Registry string - // FunctionRunner used to run Composition Functions. FunctionRunner *xfn.PackagedFunctionRunner } diff --git a/internal/controller/pkg/resolver/reconciler.go b/internal/controller/pkg/resolver/reconciler.go index 6c4216449ee..ea94c666b27 100644 --- a/internal/controller/pkg/resolver/reconciler.go +++ b/internal/controller/pkg/resolver/reconciler.go @@ -100,13 +100,21 @@ func WithFetcher(f xpkg.Fetcher) ReconcilerOption { } } +// WithDefaultRegistry sets the default registry to use. +func WithDefaultRegistry(registry string) ReconcilerOption { + return func(r *Reconciler) { + r.registry = registry + } +} + // Reconciler reconciles packages. type Reconciler struct { - client client.Client - log logging.Logger - lock resource.Finalizer - newDag dag.NewDAGFn - fetcher xpkg.Fetcher + client client.Client + log logging.Logger + lock resource.Finalizer + newDag dag.NewDAGFn + fetcher xpkg.Fetcher + registry string } // Setup adds a controller that reconciles the Lock. @@ -125,6 +133,7 @@ func Setup(mgr ctrl.Manager, o controller.Options) error { r := NewReconciler(mgr, WithLogger(o.Logger.WithValues("controller", name)), WithFetcher(f), + WithDefaultRegistry(o.DefaultRegistry), ) return ctrl.NewControllerManagedBy(mgr). @@ -231,7 +240,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco log.Debug(errInvalidConstraint, "error", err) return reconcile.Result{Requeue: false}, nil } - ref, err := name.ParseReference(dep.Package) + ref, err := name.ParseReference(dep.Package, name.WithDefaultRegistry(r.registry)) if err != nil { log.Debug(errInvalidDependency, "error", err) return reconcile.Result{Requeue: false}, nil diff --git a/internal/controller/pkg/revision/reconciler.go b/internal/controller/pkg/revision/reconciler.go index 14b2173622c..47076903b1f 100644 --- a/internal/controller/pkg/revision/reconciler.go +++ b/internal/controller/pkg/revision/reconciler.go @@ -321,7 +321,7 @@ func SetupProviderRevision(mgr ctrl.Manager, o controller.Options) error { } if o.PackageRuntime == controller.PackageRuntimeDeployment { - ro = append(ro, WithRuntimeHooks(NewProviderHooks(mgr.GetClient()))) + ro = append(ro, WithRuntimeHooks(NewProviderHooks(mgr.GetClient(), o.DefaultRegistry))) if o.Features.Enabled(features.EnableBetaDeploymentRuntimeConfigs) { cb = cb.Watches(&v1beta1.DeploymentRuntimeConfig{}, &EnqueueRequestForReferencingProviderRevisions{ @@ -429,7 +429,7 @@ func SetupFunctionRevision(mgr ctrl.Manager, o controller.Options) error { } if o.PackageRuntime == controller.PackageRuntimeDeployment { - ro = append(ro, WithRuntimeHooks(NewFunctionHooks(mgr.GetClient()))) + ro = append(ro, WithRuntimeHooks(NewFunctionHooks(mgr.GetClient(), o.DefaultRegistry))) if o.Features.Enabled(features.EnableBetaDeploymentRuntimeConfigs) { cb = cb.Watches(&v1beta1.DeploymentRuntimeConfig{}, &EnqueueRequestForReferencingFunctionRevisions{ diff --git a/internal/controller/pkg/revision/runtime_function.go b/internal/controller/pkg/revision/runtime_function.go index cf4f9267375..b4b0359de12 100644 --- a/internal/controller/pkg/revision/runtime_function.go +++ b/internal/controller/pkg/revision/runtime_function.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + "github.com/google/go-containerregistry/pkg/name" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,20 +48,23 @@ const ( errApplyFunctionService = "cannot apply function package service" errFmtUnavailableFunctionDeployment = "function package deployment is unavailable with message: %s" errNoAvailableConditionFunctionDeployment = "function package deployment has no condition of type \"Available\" yet" + errParseFunctionImage = "cannot parse function package image" ) // FunctionHooks performs runtime operations for function packages. type FunctionHooks struct { - client resource.ClientApplicator + client resource.ClientApplicator + defaultRegistry string } // NewFunctionHooks returns a new FunctionHooks. -func NewFunctionHooks(client client.Client) *FunctionHooks { +func NewFunctionHooks(client client.Client, defaultRegistry string) *FunctionHooks { return &FunctionHooks{ client: resource.ClientApplicator{ Client: client, Applicator: resource.NewAPIPatchingApplicator(client), }, + defaultRegistry: defaultRegistry, } } @@ -116,7 +120,14 @@ func (h *FunctionHooks) Post(ctx context.Context, pkg runtime.Object, pr v1.Pack } sa := build.ServiceAccount() - d := build.Deployment(sa.Name, functionDeploymentOverrides(functionMeta, pr)...) + + // Determine the function's image, taking into account the default registry. + image, err := getFunctionImage(functionMeta, pr, h.defaultRegistry) + if err != nil { + return errors.Wrap(err, errParseFunctionImage) + } + + d := build.Deployment(sa.Name, functionDeploymentOverrides(image)...) // Create/Apply the SA only if the deployment references it. // This is to avoid creating a SA that is NOT used by the deployment when // the SA is managed externally by the user and configured by setting @@ -165,7 +176,7 @@ func (h *FunctionHooks) Deactivate(ctx context.Context, _ v1.PackageRevisionWith return nil } -func functionDeploymentOverrides(functionMeta *pkgmetav1beta1.Function, pr v1.PackageRevisionWithRuntime) []DeploymentOverride { +func functionDeploymentOverrides(image string) []DeploymentOverride { do := []DeploymentOverride{ DeploymentRuntimeWithAdditionalPorts([]corev1.ContainerPort{ { @@ -175,10 +186,6 @@ func functionDeploymentOverrides(functionMeta *pkgmetav1beta1.Function, pr v1.Pa }), } - image := pr.GetSource() - if functionMeta.Spec.Image != nil { - image = *functionMeta.Spec.Image - } do = append(do, DeploymentRuntimeWithOptionalImage(image)) return do @@ -192,3 +199,19 @@ func functionServiceOverrides() []ServiceOverride { ServiceWithClusterIP(corev1.ClusterIPNone), } } + +// getFunctionImage determines a complete function image, taking into account a +// default registry. If the function meta specifies an image, we have a +// preference for that image over what is specified in the package revision. +func getFunctionImage(fm *pkgmetav1beta1.Function, pr v1.PackageRevisionWithRuntime, defaultRegistry string) (string, error) { + image := pr.GetSource() + if fm.Spec.Image != nil { + image = *fm.Spec.Image + } + ref, err := name.ParseReference(image, name.WithDefaultRegistry(defaultRegistry)) + if err != nil { + return "", errors.Wrap(err, errParseFunctionImage) + } + + return ref.Name(), nil +} diff --git a/internal/controller/pkg/revision/runtime_function_test.go b/internal/controller/pkg/revision/runtime_function_test.go index 53439df72d5..c4ac2192f12 100644 --- a/internal/controller/pkg/revision/runtime_function_test.go +++ b/internal/controller/pkg/revision/runtime_function_test.go @@ -36,6 +36,7 @@ import ( pkgmetav1beta1 "github.com/crossplane/crossplane/apis/pkg/meta/v1beta1" v1 "github.com/crossplane/crossplane/apis/pkg/v1" "github.com/crossplane/crossplane/apis/pkg/v1beta1" + "github.com/crossplane/crossplane/internal/xpkg" ) func TestFunctionPreHook(t *testing.T) { @@ -116,7 +117,7 @@ func TestFunctionPreHook(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - h := NewFunctionHooks(tc.args.client) + h := NewFunctionHooks(tc.args.client, xpkg.DefaultRegistry) err := h.Pre(context.TODO(), tc.args.pkg, tc.args.rev, tc.args.manifests) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -182,6 +183,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -207,6 +209,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -221,6 +224,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -249,6 +253,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -263,6 +268,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -288,6 +294,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -302,6 +309,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -335,6 +343,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -349,6 +358,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -381,6 +391,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -394,6 +405,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -444,6 +456,7 @@ func TestFunctionPostHook(t *testing.T) { rev: &v1beta1.FunctionRevision{ Spec: v1beta1.FunctionRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -454,7 +467,7 @@ func TestFunctionPostHook(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - h := NewFunctionHooks(tc.args.client) + h := NewFunctionHooks(tc.args.client, xpkg.DefaultRegistry) err := h.Post(context.TODO(), tc.args.pkg, tc.args.rev, tc.args.manifests) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -558,7 +571,7 @@ func TestFunctionDeactivateHook(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - h := NewFunctionHooks(tc.args.client) + h := NewFunctionHooks(tc.args.client, xpkg.DefaultRegistry) err := h.Deactivate(context.TODO(), tc.args.rev, tc.args.manifests) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -570,3 +583,102 @@ func TestFunctionDeactivateHook(t *testing.T) { }) } } + +func TestGetFunctionImage(t *testing.T) { + type args struct { + functionMeta *pkgmetav1beta1.Function + functionRevision *v1beta1.FunctionRevision + defaultRegistry string + } + + type want struct { + err error + image string + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoOverrideFromMeta": { + reason: "Should use the image from the package revision and add default registry when no override is present.", + args: args{ + functionMeta: &pkgmetav1beta1.Function{ + Spec: pkgmetav1beta1.FunctionSpec{ + Image: nil, + }, + }, + functionRevision: &v1beta1.FunctionRevision{ + Spec: v1beta1.FunctionRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: "crossplane/func-bar:v1.2.3", + }, + }, + }, + defaultRegistry: "registry.default.io", + }, + want: want{ + err: nil, + image: "registry.default.io/crossplane/func-bar:v1.2.3", + }, + }, + "WithOverrideFromMeta": { + reason: "Should use the override from the function meta when present and add default registry.", + args: args{ + functionMeta: &pkgmetav1beta1.Function{ + Spec: pkgmetav1beta1.FunctionSpec{ + Image: ptr.To("crossplane/func-bar-server:v1.2.3"), + }, + }, + functionRevision: &v1beta1.FunctionRevision{ + Spec: v1beta1.FunctionRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: "crossplane/func-bar:v1.2.3", + }, + }, + }, + defaultRegistry: "registry.default.io", + }, + want: want{ + err: nil, + image: "registry.default.io/crossplane/func-bar-server:v1.2.3", + }, + }, + "RegistrySpecified": { + reason: "Should honor the registry as specified on the package, even if its different than the default registry.", + args: args{ + functionMeta: &pkgmetav1beta1.Function{ + Spec: pkgmetav1beta1.FunctionSpec{ + Image: nil, + }, + }, + functionRevision: &v1beta1.FunctionRevision{ + Spec: v1beta1.FunctionRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: "registry.notdefault.io/crossplane/func-bar:v1.2.3", + }, + }, + }, + defaultRegistry: "registry.default.io", + }, + want: want{ + err: nil, + image: "registry.notdefault.io/crossplane/func-bar:v1.2.3", + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + image, err := getFunctionImage(tc.args.functionMeta, tc.args.functionRevision, tc.args.defaultRegistry) + + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ngetFunctionImage(): -want error, +got error:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.image, image, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ngetFunctionImage(): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/internal/controller/pkg/revision/runtime_override_options.go b/internal/controller/pkg/revision/runtime_override_options.go index b5b50b9ee20..1ec9b676415 100644 --- a/internal/controller/pkg/revision/runtime_override_options.go +++ b/internal/controller/pkg/revision/runtime_override_options.go @@ -144,8 +144,9 @@ func DeploymentWithImagePullSecrets(secrets []corev1.LocalObjectReference) Deplo } } -// DeploymentRuntimeWithOptionalImage set the image for the runtime container -// if it is unset, e.g. not specified in the DeploymentRuntimeConfig. +// DeploymentRuntimeWithOptionalImage set the image for the runtime container if +// it is unset, e.g. not specified in the DeploymentRuntimeConfig. Note that if +// the image was already set, we use it exactly as is (i.e., no default registry). func DeploymentRuntimeWithOptionalImage(image string) DeploymentOverride { return func(d *appsv1.Deployment) { if d.Spec.Template.Spec.Containers[0].Image == "" { diff --git a/internal/controller/pkg/revision/runtime_provider.go b/internal/controller/pkg/revision/runtime_provider.go index 02633da1cd7..023d0381991 100644 --- a/internal/controller/pkg/revision/runtime_provider.go +++ b/internal/controller/pkg/revision/runtime_provider.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + "github.com/google/go-containerregistry/pkg/name" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -46,20 +47,23 @@ const ( errApplyProviderService = "cannot apply provider package service" errFmtUnavailableProviderDeployment = "provider package deployment is unavailable with message: %s" errNoAvailableConditionProviderDeployment = "provider package deployment has no condition of type \"Available\" yet" + errParseProviderImage = "cannot parse provider package image" ) // ProviderHooks performs runtime operations for provider packages. type ProviderHooks struct { - client resource.ClientApplicator + client resource.ClientApplicator + defaultRegistry string } // NewProviderHooks returns a new ProviderHooks. -func NewProviderHooks(client client.Client) *ProviderHooks { +func NewProviderHooks(client client.Client, defaultRegistry string) *ProviderHooks { return &ProviderHooks{ client: resource.ClientApplicator{ Client: client, Applicator: resource.NewAPIPatchingApplicator(client), }, + defaultRegistry: defaultRegistry, } } @@ -136,7 +140,14 @@ func (h *ProviderHooks) Post(ctx context.Context, pkg runtime.Object, pr v1.Pack } sa := build.ServiceAccount() - d := build.Deployment(sa.Name, providerDeploymentOverrides(providerMeta, pr)...) + + // Determine the provider's image, taking into account the default registry. + image, err := getProviderImage(providerMeta, pr, h.defaultRegistry) + if err != nil { + return errors.Wrap(err, errParseProviderImage) + } + + d := build.Deployment(sa.Name, providerDeploymentOverrides(providerMeta, pr, image)...) // Create/Apply the SA only if the deployment references it. // This is to avoid creating a SA that is not used by the deployment when // the SA is managed externally by the user and configured by setting @@ -192,7 +203,7 @@ func (h *ProviderHooks) Deactivate(ctx context.Context, pr v1.PackageRevisionWit return nil } -func providerDeploymentOverrides(providerMeta *pkgmetav1.Provider, pr v1.PackageRevisionWithRuntime) []DeploymentOverride { +func providerDeploymentOverrides(pm *pkgmetav1.Provider, pr v1.PackageRevisionWithRuntime, image string) []DeploymentOverride { do := []DeploymentOverride{ DeploymentRuntimeWithAdditionalEnvironments([]corev1.EnvVar{ { @@ -217,13 +228,9 @@ func providerDeploymentOverrides(providerMeta *pkgmetav1.Provider, pr v1.Package // the old selector for backward compatibility with existing providers // and plan to remove this after implementing a migration in a future // release. - DeploymentWithSelectors(providerSelectors(providerMeta, pr)), + DeploymentWithSelectors(providerSelectors(pm, pr)), } - image := pr.GetSource() - if providerMeta.Spec.Controller.Image != nil { - image = *providerMeta.Spec.Controller.Image - } do = append(do, DeploymentRuntimeWithOptionalImage(image)) if pr.GetTLSClientSecretName() != nil { @@ -264,3 +271,19 @@ func providerSelectors(providerMeta *pkgmetav1.Provider, pr v1.PackageRevisionWi "pkg.crossplane.io/provider": providerMeta.GetName(), } } + +// getProviderImage determines a complete provider image, taking into account a +// default registry. If the provider meta specifies an image, we have a +// preference for that image over what is specified in the package revision. +func getProviderImage(pm *pkgmetav1.Provider, pr v1.PackageRevisionWithRuntime, defaultRegistry string) (string, error) { + image := pr.GetSource() + if pm.Spec.Controller.Image != nil { + image = *pm.Spec.Controller.Image + } + ref, err := name.ParseReference(image, name.WithDefaultRegistry(defaultRegistry)) + if err != nil { + return "", errors.Wrap(err, errParseProviderImage) + } + + return ref.Name(), nil +} diff --git a/internal/controller/pkg/revision/runtime_provider_test.go b/internal/controller/pkg/revision/runtime_provider_test.go index 2e4205f136d..58c768c6816 100644 --- a/internal/controller/pkg/revision/runtime_provider_test.go +++ b/internal/controller/pkg/revision/runtime_provider_test.go @@ -35,6 +35,7 @@ import ( pkgmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" v1 "github.com/crossplane/crossplane/apis/pkg/v1" + "github.com/crossplane/crossplane/internal/xpkg" ) const ( @@ -188,7 +189,7 @@ func TestProviderPreHook(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - h := NewProviderHooks(tc.args.client) + h := NewProviderHooks(tc.args.client, xpkg.DefaultRegistry) err := h.Pre(context.TODO(), tc.args.pkg, tc.args.rev, tc.args.manifests) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -254,6 +255,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -279,6 +281,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -293,6 +296,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -321,6 +325,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -335,6 +340,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -360,6 +366,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -374,6 +381,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -407,6 +415,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -421,6 +430,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -453,6 +463,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -466,6 +477,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -528,6 +540,7 @@ func TestProviderPostHook(t *testing.T) { rev: &v1.ProviderRevision{ Spec: v1.ProviderRevisionSpec{ PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, DesiredState: v1.PackageRevisionActive, }, }, @@ -538,7 +551,7 @@ func TestProviderPostHook(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - h := NewProviderHooks(tc.args.client) + h := NewProviderHooks(tc.args.client, xpkg.DefaultRegistry) err := h.Post(context.TODO(), tc.args.pkg, tc.args.rev, tc.args.manifests) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -660,7 +673,7 @@ func TestProviderDeactivateHook(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - h := NewProviderHooks(tc.args.client) + h := NewProviderHooks(tc.args.client, xpkg.DefaultRegistry) err := h.Deactivate(context.TODO(), tc.args.rev, tc.args.manifests) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { @@ -672,3 +685,108 @@ func TestProviderDeactivateHook(t *testing.T) { }) } } + +func TestGetProviderImage(t *testing.T) { + type args struct { + providerMeta *pkgmetav1.Provider + providerRevision *v1.ProviderRevision + defaultRegistry string + } + + type want struct { + err error + image string + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoOverrideFromMeta": { + reason: "Should use the image from the package revision and add default registry when no override is present.", + args: args{ + providerMeta: &pkgmetav1.Provider{ + Spec: pkgmetav1.ProviderSpec{ + Controller: pkgmetav1.ControllerSpec{ + Image: nil, + }, + }, + }, + providerRevision: &v1.ProviderRevision{ + Spec: v1.ProviderRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: "crossplane/provider-bar:v1.2.3", + }, + }, + }, + defaultRegistry: "registry.default.io", + }, + want: want{ + err: nil, + image: "registry.default.io/crossplane/provider-bar:v1.2.3", + }, + }, + "WithOverrideFromMeta": { + reason: "Should use the override from the function meta when present and add default registry.", + args: args{ + providerMeta: &pkgmetav1.Provider{ + Spec: pkgmetav1.ProviderSpec{ + Controller: pkgmetav1.ControllerSpec{ + Image: ptr.To("crossplane/provider-bar-controller:v1.2.3"), + }, + }, + }, + providerRevision: &v1.ProviderRevision{ + Spec: v1.ProviderRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: "crossplane/provider-bar:v1.2.3", + }, + }, + }, + defaultRegistry: "registry.default.io", + }, + want: want{ + err: nil, + image: "registry.default.io/crossplane/provider-bar-controller:v1.2.3", + }, + }, + "RegistrySpecified": { + reason: "Should honor the registry as specified on the package, even if its different than the default registry.", + args: args{ + providerMeta: &pkgmetav1.Provider{ + Spec: pkgmetav1.ProviderSpec{ + Controller: pkgmetav1.ControllerSpec{ + Image: nil, + }, + }, + }, + providerRevision: &v1.ProviderRevision{ + Spec: v1.ProviderRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: "registry.notdefault.io/crossplane/provider-bar:v1.2.3", + }, + }, + }, + defaultRegistry: "registry.default.io", + }, + want: want{ + err: nil, + image: "registry.notdefault.io/crossplane/provider-bar:v1.2.3", + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + image, err := getProviderImage(tc.args.providerMeta, tc.args.providerRevision, tc.args.defaultRegistry) + + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ngetFunctionImage(): -want error, +got error:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.image, image, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ngetFunctionImage(): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/internal/controller/pkg/revision/runtime_test.go b/internal/controller/pkg/revision/runtime_test.go index fa9f85b7e60..ed73fae8164 100644 --- a/internal/controller/pkg/revision/runtime_test.go +++ b/internal/controller/pkg/revision/runtime_test.go @@ -27,7 +27,6 @@ import ( "k8s.io/utils/ptr" pkgmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1" - pkgmetav1beta1 "github.com/crossplane/crossplane/apis/pkg/meta/v1beta1" v1 "github.com/crossplane/crossplane/apis/pkg/v1" "github.com/crossplane/crossplane/apis/pkg/v1alpha1" "github.com/crossplane/crossplane/apis/pkg/v1beta1" @@ -116,7 +115,7 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { namespace: namespace, }, serviceAccountName: providerRevisionName, - overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision), + overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision, providerImage), }, want: want{ want: deploymentProvider(providerName, providerRevisionName, providerImage, DeploymentWithSelectors(map[string]string{ @@ -125,30 +124,6 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { })), }, }, - "ProviderDeploymentWithImageOverride": { - reason: "Image should be overridden if specified in the function spec", - args: args{ - builder: &RuntimeManifestBuilder{ - revision: providerRevision, - namespace: namespace, - }, - serviceAccountName: providerRevisionName, - overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ - ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}, - Spec: pkgmetav1.ProviderSpec{ - Controller: pkgmetav1.ControllerSpec{ - Image: ptr.To("crossplane/provider-foo-controller:v1.2.3"), - }, - }, - }, providerRevision), - }, - want: want{ - want: deploymentProvider(providerName, providerRevisionName, "crossplane/provider-foo-controller:v1.2.3", DeploymentWithSelectors(map[string]string{ - "pkg.crossplane.io/provider": providerMetaName, - "pkg.crossplane.io/revision": providerRevisionName, - })), - }, - }, "ProviderDeploymentWithControllerConfig": { reason: "Overrides from the controller config should be applied to the deployment", args: args{ @@ -176,7 +151,7 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { }, }, serviceAccountName: providerRevisionName, - overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision), + overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision, providerImage), }, want: want{ want: deploymentProvider(providerName, providerRevisionName, providerImage, DeploymentWithSelectors(map[string]string{ @@ -231,7 +206,7 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { }, }, serviceAccountName: providerRevisionName, - overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision), + overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision, providerImage), }, want: want{ want: deploymentProvider(providerName, providerRevisionName, providerImage, DeploymentWithSelectors(map[string]string{ @@ -309,7 +284,7 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { }, }, serviceAccountName: providerRevisionName, - overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision), + overrides: providerDeploymentOverrides(&pkgmetav1.Provider{ObjectMeta: metav1.ObjectMeta{Name: providerMetaName}}, providerRevision, providerImage), }, want: want{ want: deploymentProvider(providerName, providerRevisionName, providerImage, DeploymentWithSelectors(map[string]string{ @@ -353,30 +328,12 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { namespace: namespace, }, serviceAccountName: functionRevisionName, - overrides: functionDeploymentOverrides(&pkgmetav1beta1.Function{}, functionRevision), + overrides: functionDeploymentOverrides(functionImage), }, want: want{ want: deploymentFunction(functionName, functionRevisionName, functionImage), }, }, - "FunctionDeploymentWithImageOverride": { - reason: "Image should be overridden if specified in the function spec", - args: args{ - builder: &RuntimeManifestBuilder{ - revision: functionRevision, - namespace: namespace, - }, - serviceAccountName: functionRevisionName, - overrides: functionDeploymentOverrides(&pkgmetav1beta1.Function{ - Spec: pkgmetav1beta1.FunctionSpec{ - Image: ptr.To("crossplane/function-foo-server:v1.2.3"), - }, - }, functionRevision), - }, - want: want{ - want: deploymentFunction(functionName, functionRevisionName, "crossplane/function-foo-server:v1.2.3"), - }, - }, "FunctionDeploymentWithControllerConfig": { reason: "Overrides from the controller config should be applied to the deployment", args: args{ @@ -390,7 +347,7 @@ func TestRuntimeManifestBuilderDeployment(t *testing.T) { }, }, serviceAccountName: functionRevisionName, - overrides: functionDeploymentOverrides(&pkgmetav1beta1.Function{}, functionRevision), + overrides: functionDeploymentOverrides(functionImage), }, want: want{ want: deploymentFunction(functionName, functionRevisionName, functionImage, func(deployment *appsv1.Deployment) { diff --git a/internal/controller/rbac/provider/roles/reconciler_test.go b/internal/controller/rbac/provider/roles/reconciler_test.go index 6c1209dbe14..80c530e263f 100644 --- a/internal/controller/rbac/provider/roles/reconciler_test.go +++ b/internal/controller/rbac/provider/roles/reconciler_test.go @@ -458,6 +458,12 @@ func TestOrgDiffer(t *testing.T) { b: "index.docker.io/cool/other-provider:v1.0.0", want: true, }, + "DifferentRegistriesWithDefaulting": { + registry: "xpkg.example.org", + a: "index.docker.io/cool/provider:v1.0.0", + b: "cool/other-provider:v1.0.0", + want: true, + }, } for name, tc := range cases { diff --git a/internal/xpkg/name.go b/internal/xpkg/name.go index a2700f826c5..6d69b13b500 100644 --- a/internal/xpkg/name.go +++ b/internal/xpkg/name.go @@ -58,6 +58,10 @@ const ( // layer. // TODO(lsviben) Consider changing this to "examples". ExamplesAnnotation string = "upbound" + + // DefaultRegistry is the registry name that will be used when no registry + // is provided. + DefaultRegistry string = "xpkg.upbound.io" ) const ( diff --git a/test/e2e/funcs/feature.go b/test/e2e/funcs/feature.go index a883431350d..54c30a48db2 100644 --- a/test/e2e/funcs/feature.go +++ b/test/e2e/funcs/feature.go @@ -31,9 +31,6 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/google/go-containerregistry/pkg/crane" - "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1/daemon" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -519,41 +516,6 @@ func DeleteResources(dir, pattern string) features.Func { } } -// CopyImageToRegistry tries to copy the supplied image to the supplied registry within the timeout -func CopyImageToRegistry(clusterName, ns, sName, image string, timeout time.Duration) features.Func { - return func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { - reg, err := ServiceIngressEndPoint(ctx, c, clusterName, ns, sName) - if err != nil { - t.Fatal(err) - } - - t.Logf("registry endpoint %s", reg) - srcRef, err := name.ParseReference(image) - if err != nil { - t.Fatal(err) - } - - src, err := daemon.Image(srcRef) - if err != nil { - t.Fatal(err) - } - - i := strings.Split(srcRef.String(), "/") - err = wait.For(func(_ context.Context) (done bool, err error) { - err = crane.Push(src, fmt.Sprintf("%s/%s", reg, i[1]), crane.Insecure) - if err != nil { - return false, nil //nolint:nilerr // we want to retry and to throw error - } - return true, nil - }, wait.WithTimeout(timeout), wait.WithInterval(DefaultPollInterval)) - if err != nil { - t.Fatalf("copying image `%s` to registry `%s` not successful: %v", image, reg, err) - } - - return ctx - } -} - // ClaimUnderTestMustNotChangeWithin asserts that the claim available in // the test context does not change within the given time func ClaimUnderTestMustNotChangeWithin(d time.Duration) features.Func { diff --git a/test/e2e/manifests/apiextensions/usage/composition/setup/provider.yaml b/test/e2e/manifests/apiextensions/usage/composition/setup/provider.yaml index 75643466057..66236a15226 100644 --- a/test/e2e/manifests/apiextensions/usage/composition/setup/provider.yaml +++ b/test/e2e/manifests/apiextensions/usage/composition/setup/provider.yaml @@ -3,5 +3,5 @@ kind: Provider metadata: name: provider-nop spec: - package: crossplane/provider-nop:v0.1.1 + package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1 ignoreCrossplaneConstraints: true \ No newline at end of file diff --git a/test/e2e/manifests/apiextensions/usage/standalone/setup/provider.yaml b/test/e2e/manifests/apiextensions/usage/standalone/setup/provider.yaml index 75643466057..66236a15226 100644 --- a/test/e2e/manifests/apiextensions/usage/standalone/setup/provider.yaml +++ b/test/e2e/manifests/apiextensions/usage/standalone/setup/provider.yaml @@ -3,5 +3,5 @@ kind: Provider metadata: name: provider-nop spec: - package: crossplane/provider-nop:v0.1.1 + package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1 ignoreCrossplaneConstraints: true \ No newline at end of file diff --git a/test/e2e/manifests/pkg/configuration/dependency/configuration.yaml b/test/e2e/manifests/pkg/configuration/dependency/configuration.yaml index 33e1a1dc6af..bec62c9da65 100644 --- a/test/e2e/manifests/pkg/configuration/dependency/configuration.yaml +++ b/test/e2e/manifests/pkg/configuration/dependency/configuration.yaml @@ -1,6 +1,10 @@ +# This package does not specify a registry, so the default registry will be used +# during installation. The package expresses a dependency on provider-nop, also +# without specifying a registry, so the default registry path is exercised +# heavily in this test. apiVersion: pkg.crossplane.io/v1 kind: Configuration metadata: name: depends-on-provider-nop spec: - package: crossplane/e2e-depends-on-provider-nop:v0.1.0 + package: crossplane/e2e-depends-on-provider-nop:v0.2.0 diff --git a/test/e2e/manifests/pkg/configuration/dependency/package/crossplane.yaml b/test/e2e/manifests/pkg/configuration/dependency/package/crossplane.yaml index 18f5fd66d0a..b3056a98565 100644 --- a/test/e2e/manifests/pkg/configuration/dependency/package/crossplane.yaml +++ b/test/e2e/manifests/pkg/configuration/dependency/package/crossplane.yaml @@ -1,10 +1,16 @@ # This is the package metadata for the Configuration installed by # configuration.yaml. +# +# This package is manually built/pushed to +# xpkg.upbound.io/crossplane/e2e-depends-on-provider-nop. +# +# The dependency does not specify a registry, so the default registry will be +# used during installation. apiVersion: meta.pkg.crossplane.io/v1 kind: Configuration metadata: name: e2e-depends-on-provider-nop spec: dependsOn: - - provider: crossplane/provider-nop - version: "=v0.1.1" \ No newline at end of file + - provider: crossplane-contrib/provider-nop + version: "=v0.2.0" \ No newline at end of file diff --git a/test/e2e/manifests/pkg/configuration/dependency/provider-dependency.yaml b/test/e2e/manifests/pkg/configuration/dependency/provider-dependency.yaml index 90ad88c6d69..152a8342192 100644 --- a/test/e2e/manifests/pkg/configuration/dependency/provider-dependency.yaml +++ b/test/e2e/manifests/pkg/configuration/dependency/provider-dependency.yaml @@ -3,7 +3,7 @@ apiVersion: pkg.crossplane.io/v1 kind: Provider metadata: - name: crossplane-provider-nop + name: crossplane-contrib-provider-nop spec: - package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0 + package: crossplane-contrib/provider-nop:v0.2.0 ignoreCrossplaneConstraints: true \ No newline at end of file diff --git a/test/e2e/manifests/pkg/provider/mr-initial.yaml b/test/e2e/manifests/pkg/provider/mr-initial.yaml index 1f7523beb9b..0b0836be096 100644 --- a/test/e2e/manifests/pkg/provider/mr-initial.yaml +++ b/test/e2e/manifests/pkg/provider/mr-initial.yaml @@ -4,14 +4,12 @@ metadata: name: pkg-provider-upgrade spec: forProvider: - # In provider-nop v0.1.x there was no conditionReason field. Every condition - # was hardwired to have reason: Available, even when that didn't make sense. - # Therefore we simulate an upgrade by first applying this manifest, then - # specifying the new (optional) reason field after we update to 0.2.x. conditionAfter: - conditionType: Ready conditionStatus: "False" + conditionReason: "Creating" time: 0s - conditionType: Ready conditionStatus: "True" + conditionReason: "Available" time: 1s diff --git a/test/e2e/manifests/pkg/provider/mr-upgrade.yaml b/test/e2e/manifests/pkg/provider/mr-upgrade.yaml index 28d81e3f184..0b0836be096 100644 --- a/test/e2e/manifests/pkg/provider/mr-upgrade.yaml +++ b/test/e2e/manifests/pkg/provider/mr-upgrade.yaml @@ -5,10 +5,6 @@ metadata: spec: forProvider: conditionAfter: - # In provider-nop v0.1.x there was no conditionReason field. Every condition - # was hardwired to have reason: Available, even when that didn't make sense. - # Therefore we simulate an upgrade by first applying this manifest, then - # specifying the new (optional) reason field after we update to 0.2.x. - conditionType: Ready conditionStatus: "False" conditionReason: "Creating" diff --git a/test/e2e/manifests/pkg/provider/provider-initial.yaml b/test/e2e/manifests/pkg/provider/provider-initial.yaml index d7a2e4ef2b3..b65befd6ce6 100644 --- a/test/e2e/manifests/pkg/provider/provider-initial.yaml +++ b/test/e2e/manifests/pkg/provider/provider-initial.yaml @@ -3,8 +3,7 @@ kind: Provider metadata: name: provider-nop spec: - # We only started pushing to xpkg.upbound.io/crossplane-contrib with v0.2.0 # This should not be updated by renovate as we need this to be not on the # latest version, ignored explicitly in renovate's configuration - package: crossplanecontrib/provider-nop:v0.1.1 + package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0 ignoreCrossplaneConstraints: true diff --git a/test/e2e/manifests/pkg/provider/provider-upgrade.yaml b/test/e2e/manifests/pkg/provider/provider-upgrade.yaml index 2f8d0708f12..66236a15226 100644 --- a/test/e2e/manifests/pkg/provider/provider-upgrade.yaml +++ b/test/e2e/manifests/pkg/provider/provider-upgrade.yaml @@ -3,5 +3,5 @@ kind: Provider metadata: name: provider-nop spec: - package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0 + package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1 ignoreCrossplaneConstraints: true \ No newline at end of file diff --git a/test/e2e/pkg_test.go b/test/e2e/pkg_test.go index 14592feab2e..b0a0ffd9c84 100644 --- a/test/e2e/pkg_test.go +++ b/test/e2e/pkg_test.go @@ -110,12 +110,6 @@ func TestProviderUpgrade(t *testing.T) { )). Assess("UpgradeProvider", funcs.AllOf( funcs.ApplyResources(FieldManager, manifests, "provider-upgrade.yaml"), - // Note(turkenh): There is a tiny instant after the upgrade where - // the provider was still reporting Installed/Healthy but the - // new version was not yet installed. This causes flakes in the - // test as ".spec.forProvider.conditionAfter[0].conditionReason: field not declared in schema" error. - // The following check is to avoid that. - funcs.ResourcesHaveFieldValueWithin(2*time.Minute, manifests, "provider-upgrade.yaml", "status.currentIdentifier", "xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0"), funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "provider-upgrade.yaml", pkgv1.Healthy(), pkgv1.Active()), )). Assess("UpgradeManagedResource", funcs.AllOf(