diff --git a/internal/directives/kustomize_set_image_directive.go b/internal/directives/kustomize_set_image_directive.go index f065c7d99..06dd2fdd6 100644 --- a/internal/directives/kustomize_set_image_directive.go +++ b/internal/directives/kustomize_set_image_directive.go @@ -12,14 +12,13 @@ import ( securejoin "github.com/cyphar/filepath-securejoin" "github.com/xeipuuv/gojsonschema" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/kustomize/api/konfig" kustypes "sigs.k8s.io/kustomize/api/types" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" yaml "sigs.k8s.io/yaml/goyaml.v3" kargoapi "github.com/akuity/kargo/api/v1alpha1" + "github.com/akuity/kargo/internal/controller/freight" intyaml "github.com/akuity/kargo/internal/yaml" ) @@ -82,11 +81,7 @@ func (d *kustomizeSetImageDirective) run( } // Discover image origins and collect target images. - images, err := discoverImages(ctx, stepCtx.KargoClient, stepCtx.Project, cfg.Images, stepCtx.FreightRequests) - if err != nil { - return Result{Status: StatusFailure}, err - } - targetImages, err := buildTargetImages(images, stepCtx.Freight.Freight) + targetImages, err := d.buildTargetImages(ctx, stepCtx, cfg.Images) if err != nil { return Result{Status: StatusFailure}, err } @@ -99,122 +94,56 @@ func (d *kustomizeSetImageDirective) run( return Result{Status: StatusSuccess}, nil } -func discoverImages( +func (d *kustomizeSetImageDirective) buildTargetImages( ctx context.Context, - c client.Client, - namespace string, + stepCtx *StepContext, images []KustomizeSetImageConfigImage, - freight []kargoapi.FreightRequest, -) ([]KustomizeSetImageConfigImage, error) { - discoveredImages := slices.Clone(images) - for i, img := range discoveredImages { - discoveredImage, err := discoverImage(ctx, c, namespace, img, freight) +) (map[string]kustypes.Image, error) { + targetImages := make(map[string]kustypes.Image, len(images)) + + for _, img := range images { + var desiredOrigin *kargoapi.FreightOrigin + if img.FromOrigin != nil { + desiredOrigin = &kargoapi.FreightOrigin{ + Kind: kargoapi.FreightOriginKind(img.FromOrigin.Kind), + Name: img.FromOrigin.Name, + } + } + + discoveredImage, err := freight.FindImage( + ctx, + stepCtx.KargoClient, + stepCtx.Project, + stepCtx.FreightRequests, + desiredOrigin, + stepCtx.Freight.References(), + img.Image, + ) if err != nil { return nil, fmt.Errorf("unable to discover image for %q: %w", img.Image, err) } if discoveredImage == nil { return nil, fmt.Errorf("no image found for %q", img.Image) } - discoveredImages[i] = *discoveredImage - } - return discoveredImages, nil -} -func discoverImage( - ctx context.Context, - c client.Client, - namespace string, - image KustomizeSetImageConfigImage, - requestedFreight []kargoapi.FreightRequest, -) (*KustomizeSetImageConfigImage, error) { - if image.FromOrigin != nil { - return &image, nil - } - - var discoverdImage *KustomizeSetImageConfigImage - for _, req := range requestedFreight { - warehouse, err := kargoapi.GetWarehouse(ctx, c, types.NamespacedName{ - Name: req.Origin.Name, - Namespace: namespace, - }) - if err != nil { - return nil, fmt.Errorf("error getting Warehouse %q in namespace %q: %w", req.Origin.Name, namespace, err) + targetImage := kustypes.Image{ + Name: img.Image, + NewName: img.NewName, + NewTag: discoveredImage.Tag, } - if warehouse == nil { - return nil, fmt.Errorf("Warehouse %q not found in namespace %q", req.Origin.Name, namespace) + if img.Name != "" { + targetImage.Name = img.Name } - for _, sub := range warehouse.Spec.Subscriptions { - if sub.Image != nil && sub.Image.RepoURL == image.Image { - if discoverdImage != nil { - return nil, fmt.Errorf( - "multiple requested Freight could provide a container image from repository %q: "+ - "please provide an origin manually to disambiguate", image.Image) - } - image.FromOrigin = &ChartFromOrigin{ - Kind: Kind(warehouse.Kind), - Name: warehouse.Name, - } - discoverdImage = &image - } + if img.UseDigest { + targetImage.Digest = discoveredImage.Digest } - } - return discoverdImage, nil -} - -func buildTargetImages( - images []KustomizeSetImageConfigImage, - freight map[string]kargoapi.FreightReference, -) (map[string]kustypes.Image, error) { - targetImages := make(map[string]kustypes.Image, len(images)) - for _, img := range images { - targetImage, err := buildTargetImage(img, freight) - if err != nil { - return nil, err - } targetImages[targetImage.Name] = targetImage } return targetImages, nil } -func buildTargetImage( - img KustomizeSetImageConfigImage, - freight map[string]kargoapi.FreightReference, -) (kustypes.Image, error) { - if img.FromOrigin == nil { - return kustypes.Image{}, fmt.Errorf("image %q has no origin specified", img.Image) - } - - for _, f := range freight { - if !f.Origin.Equals(&kargoapi.FreightOrigin{ - Kind: kargoapi.FreightOriginKind(img.FromOrigin.Kind), - Name: img.FromOrigin.Name, - }) { - continue - } - - for _, i := range f.Images { - if i.RepoURL == img.Image { - targetImage := kustypes.Image{ - Name: img.Image, - NewName: img.NewName, - NewTag: i.Tag, - } - if img.Name != "" { - targetImage.Name = img.Name - } - if img.UseDigest { - targetImage.Digest = i.Digest - } - return targetImage, nil - } - } - } - - return kustypes.Image{}, fmt.Errorf("no matching image found in freight for %q", img.Image) -} - func updateKustomizationFile(kusPath string, targetImages map[string]kustypes.Image) error { // Read the Kustomization file, and unmarshal it. node, err := readKustomizationFile(kusPath) diff --git a/internal/directives/kustomize_set_image_directive_test.go b/internal/directives/kustomize_set_image_directive_test.go index 36d49632e..a8719783a 100644 --- a/internal/directives/kustomize_set_image_directive_test.go +++ b/internal/directives/kustomize_set_image_directive_test.go @@ -162,29 +162,26 @@ images: } } -func Test_discoverImages(t *testing.T) { +func Test_kustomizeSetImageDirective_buildTargetImages(t *testing.T) { const testNamespace = "test-project" tests := []struct { - name string - images []KustomizeSetImageConfigImage - requestedFreight []kargoapi.FreightRequest - objects []runtime.Object - assertions func(*testing.T, []KustomizeSetImageConfigImage, error) + name string + images []KustomizeSetImageConfigImage + freightRequests []kargoapi.FreightRequest + objects []runtime.Object + freightReferences map[string]kargoapi.FreightReference + assertions func(*testing.T, map[string]kustypes.Image, error) }{ { - name: "discovers origins for all images", + name: "discovers origins and builds target images", images: []KustomizeSetImageConfigImage{ {Image: "nginx:latest"}, {Image: "redis:6"}, }, - requestedFreight: []kargoapi.FreightRequest{ - { - Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}, - }, - { - Origin: kargoapi.FreightOrigin{Name: "warehouse2", Kind: "Warehouse"}, - }, + freightRequests: []kargoapi.FreightRequest{ + {Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}}, + {Origin: kargoapi.FreightOrigin{Name: "warehouse2", Kind: "Warehouse"}}, }, objects: []runtime.Object{ mockWarehouse(testNamespace, "warehouse1", kargoapi.WarehouseSpec{ @@ -198,11 +195,21 @@ func Test_discoverImages(t *testing.T) { }, }), }, - assertions: func(t *testing.T, result []KustomizeSetImageConfigImage, err error) { + freightReferences: map[string]kargoapi.FreightReference{ + "Warehouse/warehouse1": { + Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse1"}, + Images: []kargoapi.Image{{RepoURL: "nginx:latest", Tag: "1.21.0", Digest: "sha256:123"}}, + }, + "Warehouse/warehouse2": { + Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse2"}, + Images: []kargoapi.Image{{RepoURL: "redis:6", Tag: "6.2.5", Digest: "sha256:456"}}, + }, + }, + assertions: func(t *testing.T, result map[string]kustypes.Image, err error) { require.NoError(t, err) - assert.ElementsMatch(t, []KustomizeSetImageConfigImage{ - {Image: "nginx:latest", FromOrigin: &ChartFromOrigin{Kind: "Warehouse", Name: "warehouse1"}}, - {Image: "redis:6", FromOrigin: &ChartFromOrigin{Kind: "Warehouse", Name: "warehouse2"}}, + assert.Equal(t, map[string]kustypes.Image{ + "nginx:latest": {Name: "nginx:latest", NewTag: "1.21.0"}, + "redis:6": {Name: "redis:6", NewTag: "6.2.5"}, }, result) }, }, @@ -211,10 +218,8 @@ func Test_discoverImages(t *testing.T) { images: []KustomizeSetImageConfigImage{ {Image: "mysql:8"}, }, - requestedFreight: []kargoapi.FreightRequest{ - { - Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}, - }, + freightRequests: []kargoapi.FreightRequest{ + {Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}}, }, objects: []runtime.Object{ mockWarehouse(testNamespace, "warehouse1", kargoapi.WarehouseSpec{ @@ -223,63 +228,23 @@ func Test_discoverImages(t *testing.T) { }, }), }, - assertions: func(t *testing.T, _ []KustomizeSetImageConfigImage, err error) { - require.ErrorContains(t, err, "no image found") - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - scheme := runtime.NewScheme() - require.NoError(t, kargoapi.AddToScheme(scheme)) - c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build() - - result, err := discoverImages(context.Background(), c, testNamespace, tt.images, tt.requestedFreight) - tt.assertions(t, result, err) - }) - } -} - -func Test_discoverImage(t *testing.T) { - const testNamespace = "test-project" - - tests := []struct { - name string - image KustomizeSetImageConfigImage - requestedFreight []kargoapi.FreightRequest - objects []runtime.Object - assertions func(*testing.T, *KustomizeSetImageConfigImage, error) - }{ - { - name: "finds origin for image", - image: KustomizeSetImageConfigImage{Image: "nginx:latest"}, - requestedFreight: []kargoapi.FreightRequest{ - { - Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}, + freightReferences: map[string]kargoapi.FreightReference{ + "Warehouse/warehouse1": { + Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse1"}, + Images: []kargoapi.Image{{RepoURL: "nginx:latest", Tag: "1.21.0", Digest: "sha256:123"}}, }, }, - objects: []runtime.Object{ - mockWarehouse(testNamespace, "warehouse1", kargoapi.WarehouseSpec{ - Subscriptions: []kargoapi.RepoSubscription{ - {Image: &kargoapi.ImageSubscription{RepoURL: "nginx:latest"}}, - }, - }), - }, - assertions: func(t *testing.T, result *KustomizeSetImageConfigImage, err error) { - require.NoError(t, err) - assert.Equal(t, &KustomizeSetImageConfigImage{ - Image: "nginx:latest", - FromOrigin: &ChartFromOrigin{Kind: "Warehouse", Name: "warehouse1"}}, - result) + assertions: func(t *testing.T, _ map[string]kustypes.Image, err error) { + require.ErrorContains(t, err, "no image found") }, }, { - name: "error when multiple origins found", - image: KustomizeSetImageConfigImage{Image: "nginx:latest"}, - requestedFreight: []kargoapi.FreightRequest{ + name: "uses provided origin", + images: []KustomizeSetImageConfigImage{ + {Image: "nginx:latest", FromOrigin: &ChartFromOrigin{Kind: "Warehouse", Name: "warehouse1"}}, + }, + freightRequests: []kargoapi.FreightRequest{ {Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}}, - {Origin: kargoapi.FreightOrigin{Name: "warehouse2", Kind: "Warehouse"}}, }, objects: []runtime.Object{ mockWarehouse(testNamespace, "warehouse1", kargoapi.WarehouseSpec{ @@ -287,229 +252,110 @@ func Test_discoverImage(t *testing.T) { {Image: &kargoapi.ImageSubscription{RepoURL: "nginx:latest"}}, }, }), - mockWarehouse(testNamespace, "warehouse2", kargoapi.WarehouseSpec{ - Subscriptions: []kargoapi.RepoSubscription{ - {Image: &kargoapi.ImageSubscription{RepoURL: "nginx:latest"}}, - }, - }), }, - assertions: func(t *testing.T, result *KustomizeSetImageConfigImage, err error) { - require.ErrorContains(t, err, "please provide an origin manually to disambiguate") - assert.Nil(t, result) - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - scheme := runtime.NewScheme() - require.NoError(t, kargoapi.AddToScheme(scheme)) - c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build() - - result, err := discoverImage(context.Background(), c, testNamespace, tt.image, tt.requestedFreight) - tt.assertions(t, result, err) - }) - } -} - -func Test_buildTargetImages(t *testing.T) { - tests := []struct { - name string - images []KustomizeSetImageConfigImage - freight map[string]kargoapi.FreightReference - assertions func(*testing.T, map[string]kustypes.Image, error) - }{ - { - name: "collects target images", - images: []KustomizeSetImageConfigImage{ - {Image: "nginx:latest", FromOrigin: &ChartFromOrigin{Kind: "Warehouse", Name: "warehouse1"}}, - {Image: "redis:6", FromOrigin: &ChartFromOrigin{Kind: "Warehouse", Name: "warehouse2"}}, - }, - freight: map[string]kargoapi.FreightReference{ + freightReferences: map[string]kargoapi.FreightReference{ "Warehouse/warehouse1": { Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse1"}, Images: []kargoapi.Image{{RepoURL: "nginx:latest", Tag: "1.21.0", Digest: "sha256:123"}}, }, - "Warehouse/warehouse2": { - Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse2"}, - Images: []kargoapi.Image{{RepoURL: "redis:6", Tag: "6.2.5", Digest: "sha256:456"}}, - }, }, assertions: func(t *testing.T, result map[string]kustypes.Image, err error) { require.NoError(t, err) assert.Equal(t, map[string]kustypes.Image{ "nginx:latest": {Name: "nginx:latest", NewTag: "1.21.0"}, - "redis:6": {Name: "redis:6", NewTag: "6.2.5"}, }, result) }, }, { - name: "error when image has no origin", + name: "uses custom name and digest", images: []KustomizeSetImageConfigImage{ - {Image: "nginx:latest"}, - }, - freight: map[string]kargoapi.FreightReference{}, - assertions: func(t *testing.T, _ map[string]kustypes.Image, err error) { - require.ErrorContains(t, err, "has no origin specified") - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, err := buildTargetImages(tt.images, tt.freight) - tt.assertions(t, result, err) - }) - } -} - -func Test_buildTargetImage(t *testing.T) { - tests := []struct { - name string - img KustomizeSetImageConfigImage - freight map[string]kargoapi.FreightReference - assertions func(*testing.T, kustypes.Image, error) - }{ - { - name: "builds target image", - img: KustomizeSetImageConfigImage{ - Image: "nginx:latest", - FromOrigin: &ChartFromOrigin{ - Kind: "Warehouse", - Name: "warehouse1", - }, - }, - freight: map[string]kargoapi.FreightReference{ - "Warehouse/warehouse1": { - Origin: kargoapi.FreightOrigin{ + { + Image: "nginx:latest", + Name: "custom-nginx", + UseDigest: true, + FromOrigin: &ChartFromOrigin{ Kind: "Warehouse", Name: "warehouse1", }, - Images: []kargoapi.Image{ - { - RepoURL: "nginx:latest", - Tag: "1.21.0", - Digest: "sha256:abcdef", - }, - }, }, }, - assertions: func(t *testing.T, result kustypes.Image, err error) { - require.NoError(t, err) - assert.Equal(t, "nginx:latest", result.Name) - assert.Equal(t, "1.21.0", result.NewTag) - assert.Empty(t, result.Digest) + freightRequests: []kargoapi.FreightRequest{ + {Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}}, }, - }, - { - name: "builds target image with custom name", - img: KustomizeSetImageConfigImage{ - Image: "nginx:latest", - Name: "custom-nginx", - FromOrigin: &ChartFromOrigin{ - Kind: "Warehouse", - Name: "warehouse1", - }, + objects: []runtime.Object{ + mockWarehouse(testNamespace, "warehouse1", kargoapi.WarehouseSpec{ + Subscriptions: []kargoapi.RepoSubscription{ + {Image: &kargoapi.ImageSubscription{RepoURL: "nginx:latest"}}, + }, + }), }, - freight: map[string]kargoapi.FreightReference{ + freightReferences: map[string]kargoapi.FreightReference{ "Warehouse/warehouse1": { - Origin: kargoapi.FreightOrigin{ - Kind: "Warehouse", - Name: "warehouse1", - }, - Images: []kargoapi.Image{ - { - RepoURL: "nginx:latest", - Tag: "1.21.0", - Digest: "sha256:abcdef", - }, - }, + Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse1"}, + Images: []kargoapi.Image{{RepoURL: "nginx:latest", Tag: "1.21.0", Digest: "sha256:123"}}, }, }, - assertions: func(t *testing.T, result kustypes.Image, err error) { + assertions: func(t *testing.T, result map[string]kustypes.Image, err error) { require.NoError(t, err) - assert.Equal(t, "custom-nginx", result.Name) - assert.Equal(t, "1.21.0", result.NewTag) - assert.Empty(t, result.Digest) + assert.Equal(t, map[string]kustypes.Image{ + "custom-nginx": {Name: "custom-nginx", NewTag: "1.21.0", Digest: "sha256:123"}, + }, result) }, }, { - name: "builds target image with digest", - img: KustomizeSetImageConfigImage{ - Image: "nginx:latest", - UseDigest: true, - FromOrigin: &ChartFromOrigin{ - Kind: "Warehouse", - Name: "warehouse1", - }, + name: "error when multiple origins found", + images: []KustomizeSetImageConfigImage{ + {Image: "nginx:latest"}, }, - freight: map[string]kargoapi.FreightReference{ - "Warehouse/warehouse1": { - Origin: kargoapi.FreightOrigin{ - Kind: "Warehouse", - Name: "warehouse1", + freightRequests: []kargoapi.FreightRequest{ + {Origin: kargoapi.FreightOrigin{Name: "warehouse1", Kind: "Warehouse"}}, + {Origin: kargoapi.FreightOrigin{Name: "warehouse2", Kind: "Warehouse"}}, + }, + objects: []runtime.Object{ + mockWarehouse(testNamespace, "warehouse1", kargoapi.WarehouseSpec{ + Subscriptions: []kargoapi.RepoSubscription{ + {Image: &kargoapi.ImageSubscription{RepoURL: "nginx:latest"}}, }, - Images: []kargoapi.Image{ - { - RepoURL: "nginx:latest", - Tag: "1.21.0", - Digest: "sha256:abcdef", - }, + }), + mockWarehouse(testNamespace, "warehouse2", kargoapi.WarehouseSpec{ + Subscriptions: []kargoapi.RepoSubscription{ + {Image: &kargoapi.ImageSubscription{RepoURL: "nginx:latest"}}, }, - }, - }, - assertions: func(t *testing.T, result kustypes.Image, err error) { - assert.NoError(t, err) - assert.Equal(t, "nginx:latest", result.Name) - assert.Equal(t, "1.21.0", result.NewTag) - assert.Equal(t, "sha256:abcdef", result.Digest) - }, - }, - { - name: "no origin specified", - img: KustomizeSetImageConfigImage{ - Image: "nginx:latest", - }, - freight: map[string]kargoapi.FreightReference{}, - assertions: func(t *testing.T, result kustypes.Image, err error) { - require.ErrorContains(t, err, "has no origin specified") - assert.Empty(t, result) - }, - }, - { - name: "no matching origin", - img: KustomizeSetImageConfigImage{ - Image: "nginx:latest", - FromOrigin: &ChartFromOrigin{ - Kind: "Warehouse", - Name: "warehouse2", - }, + }), }, - freight: map[string]kargoapi.FreightReference{ + freightReferences: map[string]kargoapi.FreightReference{ "Warehouse/warehouse1": { - Origin: kargoapi.FreightOrigin{ - Kind: "Warehouse", - Name: "warehouse1", - }, - Images: []kargoapi.Image{ - { - RepoURL: "nginx:latest", - Tag: "1.21.0", - Digest: "sha256:abcdef", - }, - }, + Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse1"}, + Images: []kargoapi.Image{{RepoURL: "nginx:latest", Tag: "1.21.0", Digest: "sha256:123"}}, + }, + "Warehouse/warehouse2": { + Origin: kargoapi.FreightOrigin{Kind: "Warehouse", Name: "warehouse2"}, + Images: []kargoapi.Image{{RepoURL: "nginx:latest", Tag: "1.21.0", Digest: "sha256:456"}}, }, }, - assertions: func(t *testing.T, result kustypes.Image, err error) { - require.ErrorContains(t, err, "no matching image found in freight") - assert.Empty(t, result) + assertions: func(t *testing.T, _ map[string]kustypes.Image, err error) { + require.ErrorContains(t, err, "multiple requested Freight could potentially provide a container image") }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := buildTargetImage(tt.img, tt.freight) + scheme := runtime.NewScheme() + require.NoError(t, kargoapi.AddToScheme(scheme)) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build() + + stepCtx := &StepContext{ + KargoClient: fakeClient, + Project: testNamespace, + FreightRequests: tt.freightRequests, + Freight: kargoapi.FreightCollection{ + Freight: tt.freightReferences, + }, + } + + d := &kustomizeSetImageDirective{} + result, err := d.buildTargetImages(context.Background(), stepCtx, tt.images) tt.assertions(t, result, err) }) }