feat(directives): optional images for kustomize-set-image (akuity…

…#3115)

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

docs/docs/35-references/10-promotion-steps.md

@@ -312,7 +312,7 @@ to executing `kustomize edit set image`. This step is commonly followed by a
 | Name | Type | Required | Description |
 |------|------|----------|-------------|
 | `path` | `string` | Y | Path to a directory containing a `kustomization.yaml` file. This path is relative to the temporary workspace that Kargo provisions for use by the promotion process. |
-| `images` | `[]object` | Y | The details of changes to be applied to the `kustomization.yaml` file. At least one must be specified. |
+| `images` | `[]object` | N | The details of changes to be applied to the `kustomization.yaml` file. When left unspecified, all images from the Freight collection will be set in the Kustomization file. Unless there is an ambiguous image name (for example, due to two Warehouses subscribing to the same repository), which requires manual configuration. |
 | `images[].image` | `string` | Y | Name/URL of the image being updated. |
 | `images[].tag` | `string` | N | A tag naming a specific revision of `image`. Mutually exclusive with `digest` and `useDigest=true`. If none of these are specified, the tag specified by a piece of Freight referencing `image` will be used as the value of this field. |
 | `images[].digest` | `string` | N | A digest naming a specific revision of `image`. Mutually exclusive with `tag` and `useDigest=true`. If none of these are specified, the tag specified by a piece of Freight referencing `image` will be used as the value of `tag`. |

internal/controller/freight/finder.go

@@ -168,6 +168,50 @@ func FindImage(
 	}
 }
 
+func HasAmbiguousImageRequest(
+	ctx context.Context,
+	cl client.Client,
+	project string,
+	freightReqs []kargoapi.FreightRequest,
+) (bool, error) {
+	var subscribedRepositories = make(map[string]any)
+
+	for i := range freightReqs {
+		requestedFreight := freightReqs[i]
+		warehouse, err := kargoapi.GetWarehouse(
+			ctx,
+			cl,
+			types.NamespacedName{
+				Name:      requestedFreight.Origin.Name,
+				Namespace: project,
+			},
+		)
+		if err != nil {
+			return false, err
+		}
+		if warehouse == nil {
+			return false, fmt.Errorf(
+				"Warehouse %q not found in namespace %q",
+				requestedFreight.Origin.Name, project,
+			)
+		}
+
+		for _, sub := range warehouse.Spec.Subscriptions {
+			if sub.Image != nil {
+				if _, ok := subscribedRepositories[sub.Image.RepoURL]; ok {
+					return true, fmt.Errorf(
+						"multiple requested Freight could potentially provide a container image from repository %s",
+						sub.Image.RepoURL,
+					)
+				}
+				subscribedRepositories[sub.Image.RepoURL] = struct{}{}
+			}
+		}
+	}
+
+	return false, nil
+}
+
 func FindChart(
 	ctx context.Context,
 	cl client.Client,

internal/controller/freight/finder_test.go

@@ -480,6 +480,135 @@ func TestFindImage(t *testing.T) {
 	}
 }
 
+func TestHasAmbiguousImageRequest(t *testing.T) {
+	const testNamespace = "test-namespace"
+	const testRepoURL = "fake-repo-url"
+
+	testOrigin1 := kargoapi.FreightOrigin{
+		Kind: kargoapi.FreightOriginKindWarehouse,
+		Name: "test-warehouse",
+	}
+	testOrigin2 := kargoapi.FreightOrigin{
+		Kind: kargoapi.FreightOriginKindWarehouse,
+		Name: "some-other-warehouse",
+	}
+
+	scheme := runtime.NewScheme()
+	require.NoError(t, kargoapi.AddToScheme(scheme))
+
+	testCases := []struct {
+		name       string
+		client     func() client.Client
+		freight    []kargoapi.FreightRequest
+		assertions func(*testing.T, bool, error)
+	}{
+		{
+			name: "no ambiguous image request",
+			client: func() client.Client {
+				return fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+					&kargoapi.Warehouse{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: testNamespace,
+							Name:      testOrigin1.Name,
+						},
+						Spec: kargoapi.WarehouseSpec{
+							Subscriptions: []kargoapi.RepoSubscription{{
+								Image: &kargoapi.ImageSubscription{
+									RepoURL: testRepoURL,
+								},
+							}},
+						},
+					},
+				).Build()
+			},
+			freight: []kargoapi.FreightRequest{
+				{
+					Origin: testOrigin1,
+				},
+			},
+			assertions: func(t *testing.T, ambiguous bool, err error) {
+				require.NoError(t, err)
+				require.False(t, ambiguous)
+			},
+		},
+		{
+			name: "ambiguous image request",
+			client: func() client.Client {
+				return fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+					&kargoapi.Warehouse{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: testNamespace,
+							Name:      testOrigin1.Name,
+						},
+						Spec: kargoapi.WarehouseSpec{
+							Subscriptions: []kargoapi.RepoSubscription{{
+								Image: &kargoapi.ImageSubscription{
+									RepoURL: testRepoURL,
+								},
+							}},
+						},
+					},
+					&kargoapi.Warehouse{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: testNamespace,
+							Name:      testOrigin2.Name,
+						},
+						Spec: kargoapi.WarehouseSpec{
+							Subscriptions: []kargoapi.RepoSubscription{{
+								Image: &kargoapi.ImageSubscription{
+									RepoURL: testRepoURL,
+								},
+							}},
+						},
+					},
+				).Build()
+			},
+			freight: []kargoapi.FreightRequest{
+				{
+					Origin: testOrigin1,
+				},
+				{
+					Origin: testOrigin2,
+				},
+			},
+			assertions: func(t *testing.T, ambiguous bool, err error) {
+				require.True(t, ambiguous)
+				require.ErrorContains(t, err, "multiple requested Freight could potentially provide")
+			},
+		},
+		{
+			name: "origin not found",
+			client: func() client.Client {
+				return fake.NewClientBuilder().WithScheme(scheme).Build()
+			},
+			freight: []kargoapi.FreightRequest{
+				{
+					Origin: testOrigin1,
+				},
+			},
+			assertions: func(t *testing.T, _ bool, err error) {
+				require.ErrorContains(t, err, "not found in namespace")
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			var cl client.Client
+			if testCase.client != nil {
+				cl = testCase.client()
+			}
+			ok, err := HasAmbiguousImageRequest(
+				context.Background(),
+				cl,
+				testNamespace,
+				testCase.freight,
+			)
+			testCase.assertions(t, ok, err)
+		})
+	}
+}
+
 func TestFindChart(t *testing.T) {
 	const testNamespace = "test-namespace"
 	const testRepoURL = "fake-repo-url"

internal/directives/kustomize_image_setter.go

@@ -91,8 +91,15 @@ func (k *kustomizeImageSetter) runPromotionStep(
 			fmt.Errorf("could not discover kustomization file: %w", err)
 	}
 
-	// Discover image origins and collect target images.
-	targetImages, err := k.buildTargetImages(ctx, stepCtx, cfg.Images)
+	var targetImages map[string]kustypes.Image
+	switch {
+	case len(cfg.Images) > 0:
+		// Discover image origins and collect target images.
+		targetImages, err = k.buildTargetImagesFromConfig(ctx, stepCtx, cfg.Images)
+	default:
+		// Attempt to automatically set target images based on the Freight references.
+		targetImages, err = k.buildTargetImagesAutomatically(ctx, stepCtx)
+	}
 	if err != nil {
 		return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, err
 	}
@@ -111,7 +118,7 @@ func (k *kustomizeImageSetter) runPromotionStep(
 	return result, nil
 }
 
-func (k *kustomizeImageSetter) buildTargetImages(
+func (k *kustomizeImageSetter) buildTargetImagesFromConfig(
 	ctx context.Context,
 	stepCtx *PromotionStepContext,
 	images []KustomizeSetImageConfigImage,
@@ -159,6 +166,42 @@ func (k *kustomizeImageSetter) buildTargetImages(
 	return targetImages, nil
 }
 
+func (k *kustomizeImageSetter) buildTargetImagesAutomatically(
+	ctx context.Context,
+	stepCtx *PromotionStepContext,
+) (map[string]kustypes.Image, error) {
+	// Check if there are any ambiguous image requests.
+	//
+	// We do this based on the request because the Freight references may not
+	// contain all the images that are requested, which could lead eventually
+	// to an ambiguous result.
+	if ambiguous, ambErr := freight.HasAmbiguousImageRequest(
+		ctx, stepCtx.KargoClient, stepCtx.Project, stepCtx.FreightRequests,
+	); ambErr != nil || ambiguous {
+		err := errors.New("manual configuration required due to ambiguous result")
+		if ambErr != nil {
+			err = fmt.Errorf("%w: %v", err, ambErr)
+		}
+		return nil, err
+	}
+
+	var images = make(map[string]kustypes.Image)
+	for _, freightRef := range stepCtx.Freight.References() {
+		if len(freightRef.Images) == 0 {
+			continue
+		}
+
+		for _, img := range freightRef.Images {
+			images[img.RepoURL] = kustypes.Image{
+				Name:   img.RepoURL,
+				NewTag: img.Tag,
+				Digest: img.Digest,
+			}
+		}
+	}
+	return images, nil
+}
+
 func (k *kustomizeImageSetter) generateCommitMessage(path string, images map[string]kustypes.Image) string {
 	if len(images) == 0 {
 		return ""
@@ -171,7 +214,17 @@ func (k *kustomizeImageSetter) generateCommitMessage(path string, images map[str
 	}
 	_, _ = commitMsg.WriteString("\n")
 
-	for _, i := range images {
+	// Sort the images by name, in descending order for consistency.
+	imageNames := make([]string, 0, len(images))
+	for name := range images {
+		imageNames = append(imageNames, name)
+	}
+	slices.Sort(imageNames)
+
+	// Append each image to the commit message.
+	for _, name := range imageNames {
+		i := images[name]
+
 		ref := i.Name
 		if i.NewName != "" {
 			ref = i.NewName