From 96ed34359eefbac0bc277ce3ea9f8a60bae0c8a5 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Tue, 20 Aug 2024 12:51:43 -0700 Subject: [PATCH 1/5] Omit docker login registry arg when promoting tag in Docker Hub Apparently you get a magic URL in your Docker config file when you omit the registry. It seems to be needed to successfully push. Signed-off-by: Nic Cope (cherry picked from commit 265dc449ccf2dc36d44e5f13ef42de2e210b15fa) --- Earthfile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Earthfile b/Earthfile index 6532253fd..06f0a6417 100644 --- a/Earthfile +++ b/Earthfile @@ -389,7 +389,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} \ From ec3797a133e26e8970a6e3713502d7333aa92e74 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Thu, 5 Sep 2024 16:37:59 -0700 Subject: [PATCH 2/5] Make composition revision numbers mutable They were made immutable by mistake. Signed-off-by: Nic Cope (cherry picked from commit c0afe70bace1ce39efc2b74ba604fe36ec37bf1b) --- .../v1/composition_revision_types.go | 6 ++++- ...zz_generated.composition_revision_types.go | 6 ++++- ...ns.crossplane.io_compositionrevisions.yaml | 24 ++++++++++++------- 3 files changed, 26 insertions(+), 10 deletions(-) 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 From 6a27ce1c6f605c99a3b29754221689cf68d3832c Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Mon, 9 Sep 2024 15:02:02 +0000 Subject: [PATCH 3/5] Fix "Missing node in tree error" after updating a package source Delete packages in lock having same name and distinct identifier. Signed-off-by: Jose Francisco Dillet Alfonso (cherry picked from commit 519e70726ebfabf30f54c9852af779d7095672e7) --- .../controller/pkg/revision/dependency.go | 18 ++++++ .../pkg/revision/dependency_test.go | 60 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/internal/controller/pkg/revision/dependency.go b/internal/controller/pkg/revision/dependency.go index 9da209088..69e381d87 100644 --- a/internal/controller/pkg/revision/dependency.go +++ b/internal/controller/pkg/revision/dependency.go @@ -137,6 +137,24 @@ 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) From 83ea52f25a622bf3aaf6d3d7492ebb476a9784e6 Mon Sep 17 00:00:00 2001 From: Jose Francisco Dillet Alfonso Date: Tue, 10 Sep 2024 09:00:09 +0000 Subject: [PATCH 4/5] Do not wrap line Signed-off-by: Jose Francisco Dillet Alfonso (cherry picked from commit 991ac5fbd0c0b7edd56c2b1ac9912867dff022f6) --- internal/controller/pkg/revision/dependency.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/controller/pkg/revision/dependency.go b/internal/controller/pkg/revision/dependency.go index 69e381d87..af7b76f0b 100644 --- a/internal/controller/pkg/revision/dependency.go +++ b/internal/controller/pkg/revision/dependency.go @@ -141,9 +141,7 @@ func (m *PackageDependencyManager) Resolve(ctx context.Context, pkg runtime.Obje // 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 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 } From 50b248e34b4ab0b12dba1fd0dea9d947a2819537 Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Tue, 6 Aug 2024 13:06:18 +0300 Subject: [PATCH 5/5] Retry on conflict during CRD dry-run updates in XRD validation webhook Signed-off-by: Alper Rifat Ulucinar (cherry picked from commit a045274ab7f82cd51a966208d5ba6ebbcc37eecf) --- .../apiextensions/v1/xrd/handler.go | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) 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.