diff --git a/Earthfile b/Earthfile index fd086b1df..1eec76e68 100644 --- a/Earthfile +++ b/Earthfile @@ -391,7 +391,13 @@ ci-promote-image: ARG --required CHANNEL FROM alpine:3.20 RUN apk add docker - RUN --secret DOCKER_USER --secret DOCKER_PASSWORD docker login -u ${DOCKER_USER} -p ${DOCKER_PASSWORD} ${CROSSPLANE_REPO} + # We need to omit the registry argument when we're logging into Docker Hub. + # Otherwise login will appear to succeed, but buildx will fail on auth. + IF [[ "${CROSSPLANE_REPO}" == *docker.io/* ]] + RUN --secret DOCKER_USER --secret DOCKER_PASSWORD docker login -u ${DOCKER_USER} -p ${DOCKER_PASSWORD} + ELSE + RUN --secret DOCKER_USER --secret DOCKER_PASSWORD docker login -u ${DOCKER_USER} -p ${DOCKER_PASSWORD} ${CROSSPLANE_REPO} + END RUN --push docker buildx imagetools create \ --tag ${CROSSPLANE_REPO}:${CHANNEL} \ --tag ${CROSSPLANE_REPO}:${CROSSPLANE_VERSION}-${CHANNEL} \ diff --git a/apis/apiextensions/v1/composition_revision_types.go b/apis/apiextensions/v1/composition_revision_types.go index fc8c07886..301262e51 100644 --- a/apis/apiextensions/v1/composition_revision_types.go +++ b/apis/apiextensions/v1/composition_revision_types.go @@ -126,7 +126,11 @@ type CompositionRevisionSpec struct { PublishConnectionDetailsWithStoreConfigRef *StoreConfigReference `json:"publishConnectionDetailsWithStoreConfigRef,omitempty"` // Revision number. Newer revisions have larger numbers. - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" + // + // This number can change. When a Composition transitions from state A + // -> B -> A there will be only two CompositionRevisions. Crossplane will + // edit the original CompositionRevision to change its revision number from + // 0 to 2. Revision int64 `json:"revision"` } diff --git a/apis/apiextensions/v1beta1/zz_generated.composition_revision_types.go b/apis/apiextensions/v1beta1/zz_generated.composition_revision_types.go index 680575832..f110dccb7 100644 --- a/apis/apiextensions/v1beta1/zz_generated.composition_revision_types.go +++ b/apis/apiextensions/v1beta1/zz_generated.composition_revision_types.go @@ -128,7 +128,11 @@ type CompositionRevisionSpec struct { PublishConnectionDetailsWithStoreConfigRef *StoreConfigReference `json:"publishConnectionDetailsWithStoreConfigRef,omitempty"` // Revision number. Newer revisions have larger numbers. - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" + // + // This number can change. When a Composition transitions from state A + // -> B -> A there will be only two CompositionRevisions. Crossplane will + // edit the original CompositionRevision to change its revision number from + // 0 to 2. Revision int64 `json:"revision"` } diff --git a/cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml b/cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml index 22c60210f..682c12578 100644 --- a/cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml +++ b/cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml @@ -1587,12 +1587,16 @@ spec: type: object type: array revision: - description: Revision number. Newer revisions have larger numbers. + description: |- + Revision number. Newer revisions have larger numbers. + + + This number can change. When a Composition transitions from state A + -> B -> A there will be only two CompositionRevisions. Crossplane will + edit the original CompositionRevision to change its revision number from + 0 to 2. format: int64 type: integer - x-kubernetes-validations: - - message: Value is immutable - rule: self == oldSelf writeConnectionSecretsToNamespace: description: |- WriteConnectionSecretsToNamespace specifies the namespace in which the @@ -3234,12 +3238,16 @@ spec: type: object type: array revision: - description: Revision number. Newer revisions have larger numbers. + description: |- + Revision number. Newer revisions have larger numbers. + + + This number can change. When a Composition transitions from state A + -> B -> A there will be only two CompositionRevisions. Crossplane will + edit the original CompositionRevision to change its revision number from + 0 to 2. format: int64 type: integer - x-kubernetes-validations: - - message: Value is immutable - rule: self == oldSelf writeConnectionSecretsToNamespace: description: |- WriteConnectionSecretsToNamespace specifies the namespace in which the diff --git a/internal/controller/pkg/revision/dependency.go b/internal/controller/pkg/revision/dependency.go index 9da209088..af7b76f0b 100644 --- a/internal/controller/pkg/revision/dependency.go +++ b/internal/controller/pkg/revision/dependency.go @@ -137,6 +137,22 @@ func (m *PackageDependencyManager) Resolve(ctx context.Context, pkg runtime.Obje Dependencies: sources, } + // Delete packages in lock with same name and distinct source + // This is a corner case when source is updated but image SHA is not (i.e. relocate same image + // to another registry) + for _, lp := range lock.Packages { + if self.Name == lp.Name && self.Type == lp.Type && self.Source != lp.Identifier() { + if err := m.RemoveSelf(ctx, pr); err != nil { + return found, installed, invalid, err + } + // refresh the lock to be in sync with the contents + if err = m.client.Get(ctx, types.NamespacedName{Name: lockName}, lock); err != nil { + return found, installed, invalid, err + } + break + } + } + prExists := false for _, lp := range lock.Packages { if lp.Name == pr.GetName() { diff --git a/internal/controller/pkg/revision/dependency_test.go b/internal/controller/pkg/revision/dependency_test.go index 9b3dd0ee2..f8e79d009 100644 --- a/internal/controller/pkg/revision/dependency_test.go +++ b/internal/controller/pkg/revision/dependency_test.go @@ -42,6 +42,7 @@ var _ DependencyManager = &PackageDependencyManager{} func TestResolve(t *testing.T) { errBoom := errors.New("boom") + mockUpdateCallCount := 0 type args struct { dep *PackageDependencyManager @@ -553,9 +554,68 @@ func TestResolve(t *testing.T) { invalid: 0, }, }, + "SuccessfulLockPackageSourceMismatch": { + reason: "Should not return error if source in packages does not match provider revision package.", + args: args{ + dep: &PackageDependencyManager{ + client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + l := obj.(*v1beta1.Lock) + if mockUpdateCallCount < 1 { + l.Packages = []v1beta1.LockPackage{ + { + Name: "config-nop-a-abc123", + // Source mistmatch provider revision package + Source: "hasheddan/config-nop-b", + }, + } + } else { + l.Packages = []v1beta1.LockPackage{} + } + return nil + }), + MockUpdate: func(_ context.Context, _ client.Object, _ ...client.UpdateOption) error { + mockUpdateCallCount++ + return nil + }, + }, + newDag: func() dag.DAG { + return &dagfake.MockDag{ + MockInit: func(_ []dag.Node) ([]dag.Node, error) { + return []dag.Node{}, nil + }, + MockTraceNode: func(s string) (map[string]dag.Node, error) { + if s == "hasheddan/config-nop-a" { + return map[string]dag.Node{ + s: &v1beta1.Dependency{}, + }, nil + } + return nil, errors.New("missing node in tree") + }, + MockAddOrUpdateNodes: func(_ ...dag.Node) {}, + } + }, + }, + meta: &pkgmetav1.Configuration{}, + pr: &v1.ConfigurationRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config-nop-a-abc123", + }, + Spec: v1.PackageRevisionSpec{ + Package: "hasheddan/config-nop-a:v0.0.1", + DesiredState: v1.PackageRevisionActive, + }, + }, + }, + want: want{ + total: 1, + installed: 1, + }, + }, } for name, tc := range cases { + mockUpdateCallCount = 0 t.Run(name, func(t *testing.T) { total, installed, invalid, err := tc.args.dep.Resolve(context.TODO(), tc.args.meta, tc.args.pr) diff --git a/internal/validation/apiextensions/v1/xrd/handler.go b/internal/validation/apiextensions/v1/xrd/handler.go index 40b5ed9e9..ce58cacec 100644 --- a/internal/validation/apiextensions/v1/xrd/handler.go +++ b/internal/validation/apiextensions/v1/xrd/handler.go @@ -25,6 +25,7 @@ import ( apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -145,16 +146,18 @@ func (v *validator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.O } func (v *validator) dryRunUpdateOrCreateIfNotFound(ctx context.Context, crd *apiextv1.CustomResourceDefinition) error { - got := crd.DeepCopy() - err := v.client.Get(ctx, client.ObjectKey{Name: crd.Name}, got) - if err == nil { - got.Spec = crd.Spec - return v.client.Update(ctx, got, client.DryRunAll) - } - if kerrors.IsNotFound(err) { - return v.client.Create(ctx, crd, client.DryRunAll) - } - return err + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + got := crd.DeepCopy() + err := v.client.Get(ctx, client.ObjectKey{Name: crd.Name}, got) + if err == nil { + got.Spec = crd.Spec + return v.client.Update(ctx, got, client.DryRunAll) + } + if kerrors.IsNotFound(err) { + return v.client.Create(ctx, crd, client.DryRunAll) + } + return err + }) } // ValidateDelete always allows delete requests.