Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(directives): optional images for kustomize-set-image #3115

Merged
merged 5 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/docs/35-references/10-promotion-steps.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`. |
Expand Down
44 changes: 44 additions & 0 deletions internal/controller/freight/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,50 @@
}
}

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
}

Check warning on line 191 in internal/controller/freight/finder.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/freight/finder.go#L190-L191

Added lines #L190 - L191 were not covered by tests
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,
Expand Down
129 changes: 129 additions & 0 deletions internal/controller/freight/finder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
61 changes: 57 additions & 4 deletions internal/directives/kustomize_image_setter.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,15 @@
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
}
Expand All @@ -111,7 +118,7 @@
return result, nil
}

func (k *kustomizeImageSetter) buildTargetImages(
func (k *kustomizeImageSetter) buildTargetImagesFromConfig(
ctx context.Context,
stepCtx *PromotionStepContext,
images []KustomizeSetImageConfigImage,
Expand Down Expand Up @@ -159,6 +166,42 @@
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

Check warning on line 191 in internal/directives/kustomize_image_setter.go

View check run for this annotation

Codecov / codecov/patch

internal/directives/kustomize_image_setter.go#L191

Added line #L191 was not covered by tests
}

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 ""
Expand All @@ -171,7 +214,17 @@
}
_, _ = 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
Expand Down
Loading
Loading