Refactoring misc (#156)

* refactor client factory

* refactor context package

* add explaining comments

* refactor deep merging of maps

* rename test suite descriptions

* refactor version handling for executables

.github/workflows/publish-scaffold.yaml

@@ -28,7 +28,11 @@ jobs:
           for arch in amd64 arm64; do
             file=bin/scaffold-$os-$arch
             echo "Building $file ..."
-            GOOS=$os GOARCH=$arch go build -o $file -ldflags "-X main.version=${{ github.event.release.tag_name }}" ./scaffold
+            LDFLAGS=""
+            LDFLAGS+=" -X github.com/sap/component-operator-runtime/internal/version.version=${{ github.event.release.tag_name }}"
+            LDFLAGS+=" -X github.com/sap/component-operator-runtime/internal/version.gitCommit=${{ github.sha }}"
+            LDFLAGS+=" -X github.com/sap/component-operator-runtime/internal/version.gitTreeState=clean"
+            GOOS=$os GOARCH=$arch go build -o $file -ldflags "$LDFLAGS" ./scaffold
           done
         done
 

internal/cluster/client.go

@@ -8,7 +8,12 @@ package cluster
 import (
 	"time"
 
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/kubernetes"
+	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -21,6 +26,35 @@ func NewClient(clnt client.Client, discoveryClient discovery.DiscoveryInterface,
 	}
 }
 
+func NewClientFor(config *rest.Config, scheme *runtime.Scheme, name string) (Client, error) {
+	return newClientFor(config, scheme, name)
+}
+
+func newClientFor(config *rest.Config, scheme *runtime.Scheme, name string) (*clientImpl, error) {
+	httpClient, err := rest.HTTPClientFor(config)
+	if err != nil {
+		return nil, err
+	}
+	ctrlClient, err := client.New(config, client.Options{HTTPClient: httpClient, Scheme: scheme})
+	if err != nil {
+		return nil, err
+	}
+	clientset, err := kubernetes.NewForConfigAndClient(config, httpClient)
+	if err != nil {
+		return nil, err
+	}
+	eventBroadcaster := record.NewBroadcaster()
+	eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: clientset.CoreV1().Events("")})
+	eventRecorder := eventBroadcaster.NewRecorder(scheme, corev1.EventSource{Component: name})
+	clnt := &clientImpl{
+		Client:           ctrlClient,
+		discoveryClient:  clientset,
+		eventBroadcaster: eventBroadcaster,
+		eventRecorder:    eventRecorder,
+	}
+	return clnt, nil
+}
+
 type clientImpl struct {
 	client.Client
 	discoveryClient  discovery.DiscoveryInterface

internal/cluster/factory.go

@@ -14,17 +14,12 @@ import (
 	"sync"
 	"time"
 
-	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/kubernetes"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
-	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/client-go/tools/record"
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/sap/component-operator-runtime/internal/metrics"
 	"github.com/sap/component-operator-runtime/pkg/types"
@@ -132,28 +127,11 @@ func (f *ClientFactory) Get(kubeConfig []byte, impersonationUser string, imperso
 			return rt.RoundTrip(r)
 		})
 	})
-	httpClient, err := rest.HTTPClientFor(config)
+	clnt, err := newClientFor(config, f.scheme, f.name)
 	if err != nil {
 		return nil, err
 	}
-	ctrlClient, err := client.New(config, client.Options{HTTPClient: httpClient, Scheme: f.scheme})
-	if err != nil {
-		return nil, err
-	}
-	clientset, err := kubernetes.NewForConfigAndClient(config, httpClient)
-	if err != nil {
-		return nil, err
-	}
-	eventBroadcaster := record.NewBroadcaster()
-	eventBroadcaster.StartRecordingToSink(&typedcorev1.EventSinkImpl{Interface: clientset.CoreV1().Events("")})
-	eventRecorder := eventBroadcaster.NewRecorder(f.scheme, corev1.EventSource{Component: f.name})
-	clnt := &clientImpl{
-		Client:           ctrlClient,
-		discoveryClient:  clientset,
-		eventBroadcaster: eventBroadcaster,
-		eventRecorder:    eventRecorder,
-		validUntil:       time.Now().Add(validity),
-	}
+	clnt.validUntil = time.Now().Add(validity)
 	f.clients[key] = clnt
 	metrics.CreatedClients.WithLabelValues(f.controllerName).Inc()
 	metrics.ActiveClients.WithLabelValues(f.controllerName).Set(float64(len(f.clients)))

internal/helm/suite_test.go

@@ -14,5 +14,5 @@ import (
 
 func TestComponent(t *testing.T) {
 	RegisterFailHandler(Fail)
-	RunSpecs(t, "Component Suite")
+	RunSpecs(t, "Package tests")
 }

internal/version/version.go

@@ -0,0 +1,40 @@
+/*
+SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and component-operator-runtime contributors
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package version
+
+import (
+	"runtime"
+)
+
+var (
+	version      = "latest"
+	metadata     = ""
+	gitCommit    = ""
+	gitTreeState = ""
+)
+
+type BuildInfo struct {
+	Version      string `json:"version,omitempty"`
+	GitCommit    string `json:"gitCommit,omitempty"`
+	GitTreeState string `json:"gitTreeState,omitempty"`
+	GoVersion    string `json:"goVersion,omitempty"`
+}
+
+func GetVersion() string {
+	if metadata == "" {
+		return version
+	}
+	return version + "+" + metadata
+}
+
+func GetBuildInfo() BuildInfo {
+	return BuildInfo{
+		Version:      GetVersion(),
+		GitCommit:    gitCommit,
+		GitTreeState: gitTreeState,
+		GoVersion:    runtime.Version(),
+	}
+}

pkg/component/context.go

@@ -26,28 +26,36 @@ var (
 	componentDigestContextKey = componentDigestContextKeyType{}
 )
 
-func newContext(ctx context.Context) *reconcileContext {
-	return &reconcileContext{Context: ctx}
+type Context interface {
+	context.Context
+	WithReconcilerName(reconcilerName string) Context
+	WithClient(clnt cluster.Client) Context
+	WithComponent(component Component) Context
+	WithComponentDigest(componentDigest string) Context
+}
+
+func NewContext(ctx context.Context) Context {
+	return &contextImpl{Context: ctx}
 }
 
-type reconcileContext struct {
+type contextImpl struct {
 	context.Context
 }
 
-func (c *reconcileContext) WithReconcilerName(reconcilerName string) *reconcileContext {
-	return &reconcileContext{Context: context.WithValue(c, reconcilerNameContextKey, reconcilerName)}
+func (c *contextImpl) WithReconcilerName(reconcilerName string) Context {
+	return &contextImpl{Context: context.WithValue(c, reconcilerNameContextKey, reconcilerName)}
 }
 
-func (c *reconcileContext) WithClient(clnt cluster.Client) *reconcileContext {
-	return &reconcileContext{Context: context.WithValue(c, clientContextKey, clnt)}
+func (c *contextImpl) WithClient(clnt cluster.Client) Context {
+	return &contextImpl{Context: context.WithValue(c, clientContextKey, clnt)}
 }
 
-func (c *reconcileContext) WithComponent(component Component) *reconcileContext {
-	return &reconcileContext{Context: context.WithValue(c, componentContextKey, component)}
+func (c *contextImpl) WithComponent(component Component) Context {
+	return &contextImpl{Context: context.WithValue(c, componentContextKey, component)}
 }
 
-func (c *reconcileContext) WithComponentDigest(componentDigest string) *reconcileContext {
-	return &reconcileContext{Context: context.WithValue(c, componentDigestContextKey, componentDigest)}
+func (c *contextImpl) WithComponentDigest(componentDigest string) Context {
+	return &contextImpl{Context: context.WithValue(c, componentDigestContextKey, componentDigest)}
 }
 
 func ReconcilerNameFromContext(ctx context.Context) (string, error) {

pkg/component/reconciler.go

@@ -311,7 +311,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result
 	// note: it's important that this happens after deferring the status handler
 	// TODO: enhance ctx with tailored logger and event recorder
 	// TODO: enhance ctx  with the local client
-	hookCtx := newContext(ctx).WithReconcilerName(r.name)
+	hookCtx := NewContext(ctx).WithReconcilerName(r.name)
 	for hookOrder, hook := range r.postReadHooks {
 		if err := hook(hookCtx, r.client, component); err != nil {
 			return ctrl.Result{}, errors.Wrapf(err, "error running post-read hook (%d)", hookOrder)
@@ -337,7 +337,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result
 	})
 	// TODO: enhance ctx with tailored logger and event recorder
 	// TODO: enhance ctx  with the local client
-	hookCtx = newContext(ctx).WithReconcilerName(r.name).WithClient(targetClient)
+	hookCtx = NewContext(ctx).WithReconcilerName(r.name).WithClient(targetClient)
 
 	// do the reconciliation
 	if component.GetDeletionTimestamp().IsZero() {

pkg/component/target.go

@@ -52,7 +52,7 @@ func (t *reconcileTarget[T]) Apply(ctx context.Context, component T) (bool, stri
 	componentDigest := calculateComponentDigest(component)
 
 	// TODO: enhance ctx with local client
-	generateCtx := newContext(ctx).
+	generateCtx := NewContext(ctx).
 		WithReconcilerName(t.reconcilerName).
 		WithClient(t.client).
 		WithComponent(component).

pkg/manifests/helm/generator.go

@@ -25,6 +25,8 @@ import (
 // HelmGenerator is a Generator implementation that basically renders a given Helm chart.
 // A few restrictions apply to the provided Helm chart: it must not contain any subcharts, some template functions are not supported,
 // some bultin variables are not supported, and hooks are processed in a slightly different fashion.
+// Note: HelmGenerator's Generate() method expects client and reconciler name to be set in the passed context;
+// see: Context.WithClient() and Context.WithReconcilerName() in package pkg/component.
 type HelmGenerator struct {
 	client client.Client
 	chart  *helm.Chart

pkg/manifests/kustomize/generator.go

@@ -53,6 +53,8 @@ type KustomizeGeneratorOptions struct {
 }
 
 // KustomizeGenerator is a Generator implementation that basically renders a given Kustomization.
+// Note: KustomizeGenerator's Generate() method expects client and component to be set in the passed context;
+// see: Context.WithClient() and Context.WithComponent() in package pkg/component.
 type KustomizeGenerator struct {
 	kustomizer *krusty.Kustomizer
 	files      map[string][]byte

pkg/manifests/suite_test.go

@@ -0,0 +1,18 @@
+/*
+SPDX-FileCopyrightText: 2023 SAP SE or an SAP affiliate company and component-operator-runtime contributors
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package manifests_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestComponent(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Package tests")
+}

pkg/manifests/util.go

@@ -14,26 +14,34 @@ import (
 // Deep-merge two maps with the usual logic and return the result.
 // The first map (x) must be deeply JSON (i.e. consist deeply of JSON values only).
 // The maps given as input will not be changed.
+// Both maps can be passed as nil.
 func MergeMaps(x, y map[string]any) map[string]any {
 	if x == nil {
 		x = make(map[string]any)
 	} else {
 		x = runtime.DeepCopyJSON(x)
 	}
-	for k, w := range y {
-		if v, ok := x[k]; ok {
-			if v, ok := v.(map[string]any); ok {
-				if _w, ok := w.(map[string]any); ok {
-					x[k] = MergeMaps(v, _w)
+	MergeMapInto(x, y)
+	return x
+}
+
+// Deep-merge second map (y) over first map (x) with the usual logic.
+// The first map will be changed (unless y is empty or nil), the second map will not be changed.
+// The first map must not be nil, the second map is allowed to be nil.
+func MergeMapInto(x map[string]any, y map[string]any) {
+	for k := range y {
+		if _, ok := x[k]; ok {
+			if v, ok := x[k].(map[string]any); ok {
+				if w, ok := y[k].(map[string]any); ok {
+					MergeMapInto(v, w)
 				} else {
 					x[k] = w
 				}
 			} else {
-				x[k] = w
+				x[k] = y[k]
 			}
 		} else {
-			x[k] = w
+			x[k] = y[k]
 		}
 	}
-	return x
 }