Merge pull request #141 from turkenh/sync-upstream-release-1.17

Sync upstream release 1.17 for v1.17.1

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} \

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"`
 }
 

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"`
 }
 

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

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() {

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)
 

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.