feat: update container image generation for stacks (#249)

* feat: update container image generation for stacks

* bump console client and fix stack configuration version usage

* update unit test

* fix lint

* use same golangci-lint version in the CI and Makefile target

* fix lint

.github/workflows/ci.yaml

@@ -64,7 +64,7 @@ jobs:
           check-latest: true
       - uses: golangci/golangci-lint-action@v3
         with:
-          version: latest
+          version: v1.60.2
   publish:
     name: Build and push Agent container
     runs-on: ubuntu-20.04

internal/controller/customhealth_controller.go

@@ -20,16 +20,17 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/pluralsh/deployment-operator/api/v1alpha1"
-	"github.com/pluralsh/deployment-operator/internal/utils"
-	"github.com/pluralsh/deployment-operator/pkg/common"
-	"github.com/pluralsh/deployment-operator/pkg/controller/service"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	"github.com/pluralsh/deployment-operator/api/v1alpha1"
+	"github.com/pluralsh/deployment-operator/internal/utils"
+	"github.com/pluralsh/deployment-operator/pkg/common"
+	"github.com/pluralsh/deployment-operator/pkg/controller/service"
 )
 
 // CustomHealthReconciler reconciles a LuaScript object
@@ -65,7 +66,7 @@ func (r *CustomHealthReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	scope, err := NewClusterScope(ctx, r.Client, script)
 	if err != nil {
 		logger.Error(err, "Failed to create cluster scope")
-		utils.MarkCondition(script.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, err.Error())
+		utils.MarkCondition(script.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "%s", err.Error())
 		return ctrl.Result{}, err
 	}
 	defer func() {

internal/utils/kubernetes.go

@@ -8,7 +8,6 @@ import (
 	"reflect"
 	"strings"
 
-	"github.com/pluralsh/deployment-operator/api/v1alpha1"
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -22,6 +21,8 @@ import (
 	"sigs.k8s.io/cli-utils/pkg/flowcontrol"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	"github.com/pluralsh/deployment-operator/api/v1alpha1"
 )
 
 func TryAddControllerRef(ctx context.Context, client ctrlruntimeclient.Client, owner ctrlruntimeclient.Object, controlled ctrlruntimeclient.Object, scheme *runtime.Scheme) error {

pkg/controller/service/reconciler_status.go

@@ -33,9 +33,8 @@ func (s *ServiceReconciler) UpdatePruneStatus(ctx context.Context, svc *console.
 			gk := e.StatusEvent.Identifier.GroupKind
 			name := e.StatusEvent.Identifier.Name
 			if e.StatusEvent.Error != nil {
-				errorMsg := fmt.Sprintf("%s status %s: %s\n", resourceIDToString(gk, name),
+				err = fmt.Errorf("%s status %s: %s\n", resourceIDToString(gk, name),
 					strings.ToLower(e.StatusEvent.PollResourceInfo.Status.String()), e.StatusEvent.Error.Error())
-				err = fmt.Errorf(errorMsg)
 				logger.Error(err, "status error")
 			} else {
 				logger.Info(resourceIDToString(gk, name),
@@ -97,12 +96,13 @@ func (s *ServiceReconciler) UpdateApplyStatus(
 			}
 
 			if e.ApplyEvent.Error != nil {
-				msg := fmt.Sprintf("%s apply %s: %s\n", resourceIDToString(gk, name),
-					strings.ToLower(e.ApplyEvent.Status.String()), e.ApplyEvent.Error.Error())
 				if e.ApplyEvent.Status == event.ApplyFailed {
-					err = fmt.Errorf(msg)
+					err = fmt.Errorf("%s apply %s: %s\n", resourceIDToString(gk, name),
+						strings.ToLower(e.ApplyEvent.Status.String()), e.ApplyEvent.Error.Error())
 					logger.Error(err, "apply error")
 				} else {
+					msg := fmt.Sprintf("%s apply %s: %s\n", resourceIDToString(gk, name),
+						strings.ToLower(e.ApplyEvent.Status.String()), e.ApplyEvent.Error.Error())
 					logger.Info(msg)
 				}
 			} else if printStatus {
@@ -115,9 +115,8 @@ func (s *ServiceReconciler) UpdateApplyStatus(
 			gk := e.StatusEvent.Identifier.GroupKind
 			name := e.StatusEvent.Identifier.Name
 			if e.StatusEvent.Error != nil {
-				errorMsg := fmt.Sprintf("%s status %s: %s\n", resourceIDToString(gk, name),
+				err = fmt.Errorf("%s status %s: %s\n", resourceIDToString(gk, name),
 					strings.ToLower(e.StatusEvent.PollResourceInfo.Status.String()), e.StatusEvent.Error.Error())
-				err = fmt.Errorf(errorMsg)
 				logger.Error(err, "status error")
 			} else if printStatus {
 				logger.Info(resourceIDToString(gk, name),

pkg/controller/stacks/job.go

@@ -228,18 +228,69 @@ func (r *StackReconciler) getDefaultContainer(run *console.StackRunFragment) cor
 }
 
 func (r *StackReconciler) getDefaultContainerImage(run *console.StackRunFragment) string {
-	image := defaultContainerImages[run.Type]
-	version := defaultContainerVersions[run.Type]
-	if run.Configuration.Version != "" {
-		version = run.Configuration.Version
+	// In case image is not provided, it will use our default image.
+	// Image name format: <defaultImage>:<tag>
+	// Note: User has to make sure that the tag is correct and matches our naming scheme.
+	//
+	// In case image is provided, it will replace both image and tag with provided values.
+	// Image name format: <image>:<tag>
+	if r.hasCustomTag(run) {
+		return fmt.Sprintf("%s:%s", r.getImage(run), *run.Configuration.Tag)
 	}
 
-	if run.Configuration.Image != nil && *run.Configuration.Image != "" {
-		image = *run.Configuration.Image
-		return fmt.Sprintf("%s:%s", image, version)
+	// In case there is a custom version and a custom image provided, it will replace both image and version
+	// with provided values.
+	// Image name format: <image>:<version>
+	if r.hasCustomImage(run) && r.hasCustomVersion(run) {
+		return fmt.Sprintf("%s:%s", *run.Configuration.Image, *run.Configuration.Version)
 	}
 
-	return fmt.Sprintf("%s:%s-%s-%s", image, defaultImageTag, strings.ToLower(string(run.Type)), version)
+	// In case only image is provided, do not follow our default naming scheme.
+	// Image name format: <image>:<defaultTag>
+	// Note: User has to make sure that the image contains the tag matching our defaults.
+	if r.hasCustomImage(run) {
+		return fmt.Sprintf("%s:%s", *run.Configuration.Image, r.getTag(run))
+	}
+
+	// In any other case return image in the default format: <defaultImage>:<defaultTag>-<stackType>-<defaultVersionOrVersion>.
+	// In this case only a custom tool version is ever provided to override our defaults.
+	return fmt.Sprintf("%s:%s-%s-%s", r.getImage(run), r.getTag(run), strings.ToLower(string(run.Type)), r.getVersion(run))
+}
+
+func (r *StackReconciler) hasCustomImage(run *console.StackRunFragment) bool {
+	return run.Configuration.Image != nil && len(*run.Configuration.Image) > 0
+}
+
+func (r *StackReconciler) getImage(run *console.StackRunFragment) string {
+	if r.hasCustomImage(run) {
+		return *run.Configuration.Image
+	}
+
+	return defaultContainerImages[run.Type]
+}
+
+func (r *StackReconciler) hasCustomVersion(run *console.StackRunFragment) bool {
+	return run.Configuration.Version != nil && len(*run.Configuration.Version) > 0
+}
+
+func (r *StackReconciler) getVersion(run *console.StackRunFragment) string {
+	if r.hasCustomVersion(run) {
+		return *run.Configuration.Version
+	}
+
+	return defaultContainerVersions[run.Type]
+}
+
+func (r *StackReconciler) hasCustomTag(run *console.StackRunFragment) bool {
+	return run.Configuration.Tag != nil && len(*run.Configuration.Tag) > 0
+}
+
+func (r *StackReconciler) getTag(run *console.StackRunFragment) string {
+	if r.hasCustomTag(run) {
+		return *run.Configuration.Tag
+	}
+
+	return defaultImageTag
 }
 
 func (r *StackReconciler) getDefaultContainerArgs(runID string) []string {

pkg/controller/stacks/job_test.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	console "github.com/pluralsh/console/go/client"
+	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -16,13 +17,88 @@ func TestGetDefaultContainerImage(t *testing.T) {
 	fakeConsoleClient := mocks.NewClientMock(t)
 	namespace := "default"
 	reconciler := NewStackReconciler(fakeConsoleClient, kClient, time.Minute, 0, namespace, "", "")
-	run := &console.StackRunFragment{
-		Type: console.StackTypeTerraform,
-		Configuration: console.StackConfigurationFragment{
-			Version: "1.8.4",
+	cases := []struct {
+		name          string
+		run           *console.StackRunFragment
+		expectedImage string
+	}{
+		{
+			name: "use_defaults_when_no_configuration_provided",
+			run: &console.StackRunFragment{
+				Type:          console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{},
+			},
+			expectedImage: "ghcr.io/pluralsh/harness:0.4.29-terraform-1.8.2",
+		},
+		{
+			name: "custom_tool_version_provided",
+			run: &console.StackRunFragment{
+				Type: console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{
+					Version: lo.ToPtr("1.8.4"),
+				},
+			},
+			expectedImage: "ghcr.io/pluralsh/harness:0.4.29-terraform-1.8.4",
+		},
+		{
+			name: "custom_tag_provided",
+			run: &console.StackRunFragment{
+				Type: console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{
+					Tag: lo.ToPtr("0.4.99"),
+				},
+			},
+			expectedImage: "ghcr.io/pluralsh/harness:0.4.99",
+		},
+		{
+			name: "custom_image_and_tag_provided",
+			run: &console.StackRunFragment{
+				Type: console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{
+					Image: lo.ToPtr("ghcr.io/pluralsh/custom"),
+					Tag:   lo.ToPtr("0.4.99"),
+				},
+			},
+			expectedImage: "ghcr.io/pluralsh/custom:0.4.99",
+		},
+		{
+			name: "custom_image_provided",
+			run: &console.StackRunFragment{
+				Type: console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{
+					Image: lo.ToPtr("ghcr.io/pluralsh/custom"),
+				},
+			},
+			expectedImage: "ghcr.io/pluralsh/custom:0.4.29",
+		},
+		{
+			name: "custom_image_and_version_provided",
+			run: &console.StackRunFragment{
+				Type: console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{
+					Image:   lo.ToPtr("ghcr.io/pluralsh/custom"),
+					Version: lo.ToPtr("1.8.4"),
+				},
+			},
+			expectedImage: "ghcr.io/pluralsh/custom:1.8.4",
+		},
+		{
+			name: "ignore_version_when_custom_tag_provided",
+			run: &console.StackRunFragment{
+				Type: console.StackTypeTerraform,
+				Configuration: console.StackConfigurationFragment{
+					Tag:     lo.ToPtr("1.8.4"),
+					Version: lo.ToPtr("1.8.0"),
+				},
+			},
+			expectedImage: "ghcr.io/pluralsh/harness:1.8.4",
 		},
 	}
 
-	img := reconciler.getDefaultContainerImage(run)
-	assert.Equal(t, img, "ghcr.io/pluralsh/harness:0.4.29-terraform-1.8.4")
+	for _, test := range cases {
+		t.Run(test.name, func(t *testing.T) {
+			img := reconciler.getDefaultContainerImage(test.run)
+			assert.Equal(t, img, test.expectedImage)
+		})
+	}
 }

pkg/controller/stacks/reconciler_test.go

@@ -70,7 +70,7 @@ var _ = Describe("Reconciler", Ordered, func() {
 		It("should exit without errors as stack run is already deleted", func() {
 			fakeConsoleClient := mocks.NewClientMock(mocks.TestingT)
 			fakeConsoleClient.On("GetStackRun", mock.Anything).Return(nil, &clientv2.ErrorResponse{
-				GqlErrors: &gqlerror.List{gqlerror.Errorf(errors2.ErrorNotFound.String())},
+				GqlErrors: &gqlerror.List{gqlerror.Errorf("%s", errors2.ErrorNotFound.String())},
 			})
 
 			reconciler := stacks.NewStackReconciler(fakeConsoleClient, kClient, time.Minute, 0, namespace, "", "")

tools.go

@@ -6,4 +6,5 @@ import (
 	_ "github.com/elastic/crd-ref-docs"
 	_ "github.com/vektra/mockery/v2"
 	_ "sigs.k8s.io/controller-runtime/tools/setup-envtest"
+	_ "github.com/golangci/golangci-lint/cmd/golangci-lint"
 )