From a9fff5d935b1794bd30f4a709a5959e3f034f878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Fri, 24 Apr 2026 11:23:32 +0200 Subject: [PATCH 1/7] Set defaults before reconciling function --- api/v1alpha1/function_defaults.go | 36 ++++++++++++++++++++++ internal/controller/function_controller.go | 8 ++--- 2 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 api/v1alpha1/function_defaults.go diff --git a/api/v1alpha1/function_defaults.go b/api/v1alpha1/function_defaults.go new file mode 100644 index 0000000..af91b09 --- /dev/null +++ b/api/v1alpha1/function_defaults.go @@ -0,0 +1,36 @@ +package v1alpha1 + +import "context" + +func (f *Function) SetDefaults(ctx context.Context) { + if f == nil { + return + } + + f.Spec.SetDefaults(ctx) +} + +func (s *FunctionSpec) SetDefaults(ctx context.Context) { + if s == nil { + return + } + + s.Repository.SetDefaults(ctx) + s.Registry.SetDefaults(ctx) +} + +func (r *FunctionSpecRepository) SetDefaults(ctx context.Context) { + if r == nil { + return + } + + if r.Branch == "" { + r.Branch = "main" + } +} + +func (r *FunctionSpecRegistry) SetDefaults(ctx context.Context) { + if r == nil { + return + } +} diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index d70d41f..20ae4ad 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -103,6 +103,7 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } function := original.DeepCopy() + function.SetDefaults(ctx) // Create tracker and add to context statusTracker := NewStatusTracker(r.Client, function) @@ -188,11 +189,6 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F // prepareSource clones the git repository and retrieves function metadata func (r *FunctionReconciler) prepareSource(ctx context.Context, function *v1alpha1.Function) (*git.Repository, *funcfn.Function, error) { - branchReference := "main" - if function.Spec.Repository.Branch != "" { - branchReference = function.Spec.Repository.Branch - } - gitAuthSecret := v1.Secret{} if function.Spec.Repository.AuthSecretRef != nil { if err := r.Get(ctx, types.NamespacedName{Namespace: function.Namespace, Name: function.Spec.Repository.AuthSecretRef.Name}, &gitAuthSecret); err != nil { @@ -201,7 +197,7 @@ func (r *FunctionReconciler) prepareSource(ctx context.Context, function *v1alph } } - repo, err := r.GitManager.CloneRepository(ctx, function.Spec.Repository.URL, function.Spec.Repository.Path, branchReference, gitAuthSecret.Data) + repo, err := r.GitManager.CloneRepository(ctx, function.Spec.Repository.URL, function.Spec.Repository.Path, function.Spec.Repository.Branch, gitAuthSecret.Data) if err != nil { function.MarkSourceNotReady("GitCloneFailed", "Failed to clone repository: %s", err.Error()) return nil, nil, fmt.Errorf("failed to setup git repository: %w", err) From 1510bc53e5df0b8091c3556f9c4b175ddf75d628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Fri, 24 Apr 2026 12:46:33 +0200 Subject: [PATCH 2/7] Refactor handleMiddlewareUpdate for readability Extract middleware state checking into checkMiddlewareState, which returns a sum type (middlewareUpToDate | middlewareOutdated) so that handleMiddlewareUpdate becomes a flat type-switch orchestrator. This eliminates the redundant Describe and GetMiddlewareVersion calls, and removes the now-unused isMiddlewareLatest method. --- internal/controller/function_controller.go | 230 +++++++++++------- .../controller/function_controller_test.go | 20 +- 2 files changed, 140 insertions(+), 110 deletions(-) diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index 20ae4ad..b467f37 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -131,29 +131,6 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, nil } -func (r *FunctionReconciler) removeFuncAnnotations(ctx context.Context, function *v1alpha1.Function) error { - return retry.RetryOnConflict(retry.DefaultRetry, func() error { - latest := &v1alpha1.Function{} - if err := r.Get(ctx, types.NamespacedName{Name: function.Name, Namespace: function.Namespace}, latest); err != nil { - return err - } - - if !hasFuncAnnotations(latest) { - return nil - } - - annotations := latest.GetAnnotations() - for key := range annotations { - if strings.HasPrefix(key, funcAnnotationPrefix) { - delete(annotations, key) - } - } - - latest.SetAnnotations(annotations) - return r.Update(ctx, latest) - }) -} - func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.Function) error { // Initialize conditions to start fresh each reconcile function.InitializeConditions() @@ -165,7 +142,16 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F defer repo.Cleanup() function.Status.Name = metadata.Name + applyLastDeployedAnnotation(ctx, function) + if err := r.ensureDeployment(ctx, function, repo, metadata); err != nil { + return fmt.Errorf("deploying function failed: %w", err) + } + + return nil +} + +func applyLastDeployedAnnotation(ctx context.Context, function *v1alpha1.Function) { if val, ok := function.Annotations[funcAnnotationLastDeployed]; ok { t, err := time.Parse(time.RFC3339, val) if err != nil { @@ -175,16 +161,29 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F function.Status.Deployment.ImageBuilt = metav1.NewTime(t) } } +} - if err := r.ensureDeployment(ctx, function, repo, metadata); err != nil { - return fmt.Errorf("deploying function failed: %w", err) - } +func (r *FunctionReconciler) removeFuncAnnotations(ctx context.Context, function *v1alpha1.Function) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &v1alpha1.Function{} + if err := r.Get(ctx, types.NamespacedName{Name: function.Name, Namespace: function.Namespace}, latest); err != nil { + return err + } - if err := FlushStatus(ctx, function); err != nil { - return fmt.Errorf("failed to update status: %w", err) - } + if !hasFuncAnnotations(latest) { + return nil + } - return nil + annotations := latest.GetAnnotations() + for key := range annotations { + if strings.HasPrefix(key, funcAnnotationPrefix) { + delete(annotations, key) + } + } + + latest.SetAnnotations(annotations) + return r.Update(ctx, latest) + }) } // prepareSource clones the git repository and retrieves function metadata @@ -219,7 +218,7 @@ func (r *FunctionReconciler) prepareSource(ctx context.Context, function *v1alph return repo, &metadata, nil } -// ensureDeployment ensures the function is deployed and up-to-date +// ensureDeployment ensures the functions middleware is up-to-date func (r *FunctionReconciler) ensureDeployment(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function) error { logger := log.FromContext(ctx) logger.Info("Reconciling Function") @@ -232,7 +231,7 @@ func (r *FunctionReconciler) ensureDeployment(ctx context.Context, function *v1a if !deployed { logger.Info("Function is not deployed") - function.MarkDeployNotReady("NotDeployed", "Function not deployed yet") + function.MarkDeployNotReady("NotDeployed", "Function not deployed") return nil } @@ -249,51 +248,117 @@ func (r *FunctionReconciler) ensureDeployment(ctx context.Context, function *v1a return r.handleMiddlewareUpdate(ctx, function, repo, metadata) } -// handleMiddlewareUpdate checks if the function is using the latest middleware and redeploys if needed -func (r *FunctionReconciler) handleMiddlewareUpdate(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function) error { - logger := log.FromContext(ctx) +// middlewareCheck is a sealed interface representing the result of inspecting a function's +// middleware state. Implementations (middlewareUpToDate, middlewareOutdated) carry only the +// fields relevant to their case, so the caller can type-switch without inspecting irrelevant data. +type middlewareCheck interface { + middlewareCheck() +} + +type middlewareUpToDate struct { + currentImage string + serviceReady string + currentVersion string + autoUpdate autoUpdateStatus +} + +func (middlewareUpToDate) middlewareCheck() {} + +type middlewareOutdated struct { + currentImage string + serviceReady string + currentVersion string + availableVersion string + autoUpdate autoUpdateStatus +} - functionDescribe, err := r.FuncCliManager.Describe(ctx, metadata.Name, function.Namespace) +func (middlewareOutdated) middlewareCheck() {} + +type autoUpdateStatus struct { + enabled bool + source string // "function" or "operator" +} + +func (r *FunctionReconciler) checkMiddlewareState(ctx context.Context, function *v1alpha1.Function, metadata *funcfn.Function) (middlewareCheck, error) { + desc, err := r.FuncCliManager.Describe(ctx, metadata.Name, function.Namespace) if err != nil { - return fmt.Errorf("failed to describe function to get image details: %w", err) + return nil, fmt.Errorf("failed to describe function: %w", err) } - function.Status.Deployment.Image = functionDescribe.Image - markServiceStatus(functionDescribe.Ready, function) - isMiddlewareUpdateEnabled, source, err := r.isMiddlewareUpdateEnabled(ctx, function) + autoUpdate, err := r.getAutoUpdateStatus(ctx, function) if err != nil { - function.MarkMiddlewareNotUpToDate("MiddlewareCheckFailed", "Failed to check if middleware should be updated: %s", err) - return fmt.Errorf("failed to check if middleware should be updated: %w", err) + return nil, fmt.Errorf("failed to check middleware update setting: %w", err) } - function.Status.Middleware.AutoUpdate.Enabled = isMiddlewareUpdateEnabled - function.Status.Middleware.AutoUpdate.Source = source - function.Status.Middleware.Current = functionDescribe.Middleware.Version - function.Status.Middleware.PendingRebuild = false - isOnLatestMiddleware, err := r.isMiddlewareLatest(ctx, metadata, function.Namespace) + latestVersion, err := r.FuncCliManager.GetLatestMiddlewareVersion(ctx, metadata.Runtime, metadata.Invoke) if err != nil { - function.MarkMiddlewareNotUpToDate("MiddlewareCheckFailed", "Failed to check middleware version: %s", err.Error()) - return fmt.Errorf("failed to check if function is using latest middleware: %w", err) + return nil, fmt.Errorf("failed to get latest middleware version: %w", err) } - if !isOnLatestMiddleware { - latestMiddleware, err := r.FuncCliManager.GetLatestMiddlewareVersion(ctx, metadata.Runtime, metadata.Invoke) - if err != nil { - return fmt.Errorf("failed to get latest available middleware version: %w", err) - } - function.Status.Middleware.Available = ptr.To(latestMiddleware) + if latestVersion == desc.Middleware.Version { + return middlewareUpToDate{ + currentImage: desc.Image, + serviceReady: desc.Ready, + currentVersion: desc.Middleware.Version, + autoUpdate: autoUpdate, + }, nil + } + + return middlewareOutdated{ + currentImage: desc.Image, + serviceReady: desc.Ready, + currentVersion: desc.Middleware.Version, + availableVersion: latestVersion, + autoUpdate: autoUpdate, + }, nil +} + +func (r *FunctionReconciler) getAutoUpdateStatus(ctx context.Context, function *v1alpha1.Function) (autoUpdateStatus, error) { + enabled, source, err := r.isMiddlewareUpdateEnabled(ctx, function) + if err != nil { + return autoUpdateStatus{}, err + } + return autoUpdateStatus{enabled: enabled, source: source}, nil +} - if !isMiddlewareUpdateEnabled { +// handleMiddlewareUpdate checks if the function is using the latest middleware and redeploys if needed +func (r *FunctionReconciler) handleMiddlewareUpdate(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function) error { + logger := log.FromContext(ctx) + + check, err := r.checkMiddlewareState(ctx, function, metadata) + if err != nil { + function.MarkMiddlewareNotUpToDate("MiddlewareCheckFailed", "Failed to check middleware: %s", err) + return err + } + + switch check := check.(type) { + case middlewareUpToDate: + logger.Info("Function is on latest middleware. No redeploy needed", "version", check.currentVersion) + function.Status.Deployment.Image = check.currentImage + function.Status.Middleware.Current = check.currentVersion + function.Status.Middleware.AutoUpdate.Enabled = check.autoUpdate.enabled + function.Status.Middleware.AutoUpdate.Source = check.autoUpdate.source + function.Status.Middleware.PendingRebuild = false + markServiceStatus(check.serviceReady, function) + function.MarkMiddlewareUpToDate() + + case middlewareOutdated: + function.Status.Deployment.Image = check.currentImage + function.Status.Middleware.Current = check.currentVersion + function.Status.Middleware.AutoUpdate.Enabled = check.autoUpdate.enabled + function.Status.Middleware.AutoUpdate.Source = check.autoUpdate.source + function.Status.Middleware.Available = ptr.To(check.availableVersion) + function.Status.Middleware.PendingRebuild = false + markServiceStatus(check.serviceReady, function) + + if !check.autoUpdate.enabled { logger.Info("Skipping middleware update, as middleware update is disabled") - function.MarkMiddlewareNotUpToDateIntentionally("SkipMiddlewareUpdate", "Skipping middleware update as update is disabled (source: %s)", source) - // Don't return - continue to update deployment status + function.MarkMiddlewareNotUpToDateIntentionally("SkipMiddlewareUpdate", "Skipping middleware update as update is disabled (source: %s)", check.autoUpdate.source) } else { - logger.Info(fmt.Sprintf("Function is not on latest middleware (%q vs %q) and middleware update is enabled. Will redeploy", latestMiddleware, functionDescribe.Middleware.Version)) - function.MarkMiddlewareNotUpToDate("MiddlewareOutdated", "Middleware is outdated (%s available), redeploying...", latestMiddleware) - + logger.Info("Middleware outdated, redeploying", "current", check.currentVersion, "available", check.availableVersion) + function.MarkMiddlewareNotUpToDate("MiddlewareOutdated", "Middleware is outdated (%s available), redeploying...", check.availableVersion) function.Status.Middleware.PendingRebuild = true - // Flush status before long deploy operation if err := FlushStatus(ctx, function); err != nil { logger.Error(err, "Failed to update status before redeployment") } @@ -303,33 +368,24 @@ func (r *FunctionReconciler) handleMiddlewareUpdate(ctx context.Context, functio return fmt.Errorf("failed to redeploy function: %w", err) } + desc, err := r.FuncCliManager.Describe(ctx, metadata.Name, function.Namespace) + if err != nil { + return fmt.Errorf("failed to describe function after deploy: %w", err) + } + function.Status.Deployment.Image = desc.Image + function.Status.Middleware.Current = desc.Middleware.Version function.Status.Middleware.PendingRebuild = false function.Status.Middleware.LastRebuild = metav1.Now() function.Status.Deployment.ImageBuilt = metav1.Now() + function.Status.Middleware.Available = nil + markServiceStatus(desc.Ready, function) - function.RecordHistoryEvent(fmt.Sprintf("Middleware updated from %q to %q", functionDescribe.Middleware.Version, latestMiddleware)) - - // After successful deployment, middleware is now up-to-date + function.RecordHistoryEvent(fmt.Sprintf("Middleware updated from %q to %q", check.currentVersion, check.availableVersion)) function.MarkMiddlewareUpToDate() - function.Status.Middleware.Available = nil // if function is on latest, we don't need to show this field } - } else { - logger.Info(fmt.Sprintf("Function is deployed with latest middleware (%s). No need to redeploy", functionDescribe.Middleware.Version)) - function.MarkMiddlewareUpToDate() - function.Status.Middleware.Available = nil // if function is on latest, we don't need to show this field } - // Update deployment status - functionDescribe, err = r.FuncCliManager.Describe(ctx, metadata.Name, function.Namespace) - if err != nil { - return fmt.Errorf("failed to describe function to get image details: %w", err) - } - function.Status.Deployment.Image = functionDescribe.Image - function.Status.Middleware.Current = functionDescribe.Middleware.Version - markServiceStatus(functionDescribe.Ready, function) - function.MarkDeployReady() - return nil } @@ -622,20 +678,6 @@ func (r *FunctionReconciler) findFunctionsForConfigMap(ctx context.Context, _ cl return requests } -func (r *FunctionReconciler) isMiddlewareLatest(ctx context.Context, metadata *funcfn.Function, namespace string) (bool, error) { - latestMiddleware, err := r.FuncCliManager.GetLatestMiddlewareVersion(ctx, metadata.Runtime, metadata.Invoke) - if err != nil { - return false, fmt.Errorf("failed to get latest available middleware version: %w", err) - } - - functionMiddleware, err := r.FuncCliManager.GetMiddlewareVersion(ctx, metadata.Name, namespace) - if err != nil { - return false, fmt.Errorf("failed to get middleware version of function: %w", err) - } - - return latestMiddleware == functionMiddleware, nil -} - // isMiddlewareUpdateEnabled returns if the middleware should be updated given by the functions spec or the operators // default. func (r *FunctionReconciler) isMiddlewareUpdateEnabled(ctx context.Context, function *v1alpha1.Function) (bool, string, error) { diff --git a/internal/controller/function_controller_test.go b/internal/controller/function_controller_test.go index f08d969..5bac418 100644 --- a/internal/controller/function_controller_test.go +++ b/internal/controller/function_controller_test.go @@ -158,7 +158,7 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v2.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) + funcMock.EXPECT().Deploy(mock.Anything, mock.Anything, resourceNamespace, funccli.DeployOptions{}).Return(nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) @@ -173,7 +173,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -191,7 +190,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "main", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -211,7 +209,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}, WithRepoOptionBranch("my-branch"), WithRepoOptionCommit("foobar")), nil) }, @@ -234,7 +231,6 @@ var _ = Describe("Function Controller", func() { Image: "my-image:v1.2.3", }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "main", mock.Anything).Return(createTmpGitRepo(functions.Function{ Name: "func-go", @@ -264,7 +260,7 @@ var _ = Describe("Function Controller", func() { Image: "my-image:v1.2.3", }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v2.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) + // no funcMock.EXPECT().Deploy call, as no redeploy expected! gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "main", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) @@ -288,7 +284,7 @@ var _ = Describe("Function Controller", func() { Image: "my-image:v1.2.3", }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v2.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) + // no funcMock.EXPECT().Deploy call, as no redeploy expected! gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "main", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) @@ -313,7 +309,6 @@ var _ = Describe("Function Controller", func() { Image: "my-image:v1.2.3", }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v2.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v2.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "main", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -335,7 +330,6 @@ var _ = Describe("Function Controller", func() { Ready: "true", }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -360,7 +354,6 @@ var _ = Describe("Function Controller", func() { Ready: "false", }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -384,7 +377,7 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v2.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) + funcMock.EXPECT().Deploy(mock.Anything, mock.Anything, resourceNamespace, funccli.DeployOptions{}).Return(nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) @@ -406,7 +399,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -431,7 +423,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -454,7 +445,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -477,7 +467,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, @@ -496,7 +485,6 @@ var _ = Describe("Function Controller", func() { }, }, nil) funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) - funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) }, From fb15e785f380509cff87001bef3ffe355ba489c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Fri, 24 Apr 2026 12:51:24 +0200 Subject: [PATCH 3/7] Rename ensureDeployment to reconcileDeployment --- internal/controller/function_controller.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index b467f37..1da798b 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -144,7 +144,7 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F function.Status.Name = metadata.Name applyLastDeployedAnnotation(ctx, function) - if err := r.ensureDeployment(ctx, function, repo, metadata); err != nil { + if err := r.reconcileDeployment(ctx, function, repo, metadata); err != nil { return fmt.Errorf("deploying function failed: %w", err) } @@ -218,8 +218,7 @@ func (r *FunctionReconciler) prepareSource(ctx context.Context, function *v1alph return repo, &metadata, nil } -// ensureDeployment ensures the functions middleware is up-to-date -func (r *FunctionReconciler) ensureDeployment(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function) error { +func (r *FunctionReconciler) reconcileDeployment(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function) error { logger := log.FromContext(ctx) logger.Info("Reconciling Function") From a9fc7212cef9b89d7d620b06a8ea5c747d454bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Fri, 24 Apr 2026 12:54:41 +0200 Subject: [PATCH 4/7] Extract RBAC and deploy logic into separate files Move pipeline RBAC setup (setupPipelineRBAC, ensureDeployFunctionRole, ensureDeployFunctionRoleBinding) to function_rbac.go and deploy mechanics (deploy, persistRegistryAuthSecret) to function_deploy.go, so the main controller file focuses on reconciliation flow. --- internal/controller/function_controller.go | 208 --------------------- internal/controller/function_deploy.go | 100 ++++++++++ internal/controller/function_rbac.go | 166 ++++++++++++++++ 3 files changed, 266 insertions(+), 208 deletions(-) create mode 100644 internal/controller/function_deploy.go create mode 100644 internal/controller/function_rbac.go diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index 1da798b..eed3013 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -19,7 +19,6 @@ package controller import ( "context" "fmt" - "os" "strconv" "strings" "time" @@ -28,7 +27,6 @@ import ( fn "github.com/functions-dev/func-operator/internal/function" "github.com/functions-dev/func-operator/internal/git" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -46,7 +44,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/functions-dev/func-operator/api/v1alpha1" - rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" ) @@ -399,211 +396,6 @@ func markServiceStatus(ready string, function *v1alpha1.Function) { } } -func (r *FunctionReconciler) setupPipelineRBAC(ctx context.Context, function *v1alpha1.Function) error { - if err := r.ensureDeployFunctionRole(ctx, function.Namespace); err != nil { - return fmt.Errorf("failed to ensure deploy-function role: %w", err) - } - - if err := r.ensureDeployFunctionRoleBinding(ctx, function); err != nil { - return fmt.Errorf("failed to ensure deploy-function role binding: %w", err) - } - - return nil -} - -// ensureDeployFunctionRole ensures the deploy-function Role exists in the namespace and is up-to-date. -// This is a namespace-scoped Role so multiple operator instances won't conflict. -func (r *FunctionReconciler) ensureDeployFunctionRole(ctx context.Context, namespace string) error { - logger := log.FromContext(ctx) - - // TODO: only add the rules which are needed for the functions deployer - expectedRole := &rbacv1.Role{ - ObjectMeta: metav1.ObjectMeta{ - Name: deployFunctionRoleName, - Namespace: namespace, - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"serving.knative.dev"}, - Resources: []string{"services", "routes"}, - Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, - }, { - APIGroups: []string{"eventing.knative.dev"}, - Resources: []string{"triggers"}, - Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, - }, { - APIGroups: []string{"apps"}, - Resources: []string{"deployments", "replicasets"}, - Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, - }, { - APIGroups: []string{""}, - Resources: []string{"services", "pods"}, - Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, - }, { - APIGroups: []string{"http.keda.sh"}, - Resources: []string{"httpscaledobjects"}, - Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, - }, - }, - } - - foundRole := &rbacv1.Role{} - err := r.Get(ctx, types.NamespacedName{Name: expectedRole.Name, Namespace: expectedRole.Namespace}, foundRole) - if err != nil { - if apierrors.IsNotFound(err) { - if err := r.Create(ctx, expectedRole); err != nil { - return fmt.Errorf("failed to create role: %w", err) - } - logger.Info("Created deploy-function role") - return nil - } - return fmt.Errorf("failed to get role: %w", err) - } - - // Role exists - update if needed - if !equality.Semantic.DeepEqual(expectedRole.Rules, foundRole.Rules) { - foundRole.Rules = expectedRole.Rules - if err := r.Update(ctx, foundRole); err != nil { - return fmt.Errorf("failed to update role: %w", err) - } - logger.Info("Updated deploy-function role") - } else { - logger.Info("Deploy-function role already up to date") - } - - return nil -} - -// ensureDeployFunctionRoleBinding ensures the RoleBinding for the deploy-function role exists and is up-to-date. -func (r *FunctionReconciler) ensureDeployFunctionRoleBinding(ctx context.Context, function *v1alpha1.Function) error { - logger := log.FromContext(ctx) - - expectedRoleBinding := &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deploy-function-default", - Namespace: function.Namespace, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: v1alpha1.GroupVersion.String(), - Kind: "Function", - Name: function.Name, - UID: function.UID, - Controller: ptr.To(true), - }, - }, - }, - Subjects: []rbacv1.Subject{{ - Kind: "ServiceAccount", - Name: "default", - Namespace: function.Namespace, - }}, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: deployFunctionRoleName, - }, - } - - foundRoleBinding := &rbacv1.RoleBinding{} - err := r.Get(ctx, types.NamespacedName{Name: expectedRoleBinding.Name, Namespace: expectedRoleBinding.Namespace}, foundRoleBinding) - if err != nil { - if apierrors.IsNotFound(err) { - if err := r.Create(ctx, expectedRoleBinding); err != nil { - return fmt.Errorf("failed to create role binding: %w", err) - } - logger.Info("Created deploy-function role binding") - return nil - } - return fmt.Errorf("failed to get role binding: %w", err) - } - - // Update if needed - if !equality.Semantic.DeepDerivative(expectedRoleBinding, foundRoleBinding) { - foundRoleBinding.Subjects = expectedRoleBinding.Subjects - foundRoleBinding.RoleRef = expectedRoleBinding.RoleRef - foundRoleBinding.OwnerReferences = expectedRoleBinding.OwnerReferences - - if err := r.Update(ctx, foundRoleBinding); err != nil { - return fmt.Errorf("failed to update role binding: %w", err) - } - logger.Info("Updated deploy-function role binding") - } else { - logger.Info("Deploy-function role binding already up to date") - } - - return nil -} - -func (r *FunctionReconciler) persistRegistryAuthSecret(ctx context.Context, function *v1alpha1.Function) (string, error) { - logger := log.FromContext(ctx) - - logger.Info("Persist registry auth secret temporarily") - - authSecret := &v1.Secret{} - err := r.Get(ctx, types.NamespacedName{Name: function.Spec.Registry.AuthSecretRef.Name, Namespace: function.Namespace}, authSecret) - if err != nil { - logger.Error(err, "Failed to get registry auth secret", "secret", function.Spec.Registry.AuthSecretRef.Name, "namespace", function.Namespace) - return "", fmt.Errorf("failed to get registry auth secret: %w", err) - } - - if authSecret.Type != v1.SecretTypeDockerConfigJson { - return "", fmt.Errorf("invalid registry auth secret type, must be of type %s", v1.SecretTypeDockerConfigJson) - } - - if authSecret.Data[v1.DockerConfigJsonKey] == nil { - return "", fmt.Errorf("invalid registry auth secret data, must contain key %s", v1.DockerConfigJsonKey) - } - - // persist secret temporarily - authFile, err := os.CreateTemp("", "auth-file-*.json") - if err != nil { - logger.Error(err, "Failed to create temp auth file") - return "", fmt.Errorf("failed to create temp auth file: %w", err) - } - defer authFile.Close() - - _, err = authFile.Write(authSecret.Data[v1.DockerConfigJsonKey]) - if err != nil { - logger.Error(err, "Failed to write temp auth file") - return "", fmt.Errorf("failed to write temp auth file: %w", err) - } - - return authFile.Name(), nil -} - -func (r *FunctionReconciler) deploy(ctx context.Context, function *v1alpha1.Function, repo *git.Repository) error { - logger := log.FromContext(ctx) - - if err := r.setupPipelineRBAC(ctx, function); err != nil { - return fmt.Errorf("failed to setup pipeline RBAC: %w", err) - } - - // deploy function - deployOptions := funccli.DeployOptions{} - - if function.Spec.Registry.AuthSecretRef != nil && function.Spec.Registry.AuthSecretRef.Name != "" { - // we have a registry auth secret referenced -> use this for func deploy - authFile, err := r.persistRegistryAuthSecret(ctx, function) - if err != nil { - return fmt.Errorf("failed to persist registry auth secret temporarily: %w", err) - } - - defer os.Remove(authFile) - - deployOptions.RegistryAuthFile = authFile - } - - logger.Info("Deploying function", "deployOptions", deployOptions) - err := r.FuncCliManager.Deploy(ctx, repo.Path(), function.Namespace, deployOptions) - if err != nil { - return fmt.Errorf("failed to deploy function: %w", err) - } - - logger.Info("function deployed successfully") - - return nil -} - func (r *FunctionReconciler) isDeployed(ctx context.Context, name, namespace string) (bool, error) { _, err := r.FuncCliManager.Describe(ctx, name, namespace) if err != nil { diff --git a/internal/controller/function_deploy.go b/internal/controller/function_deploy.go new file mode 100644 index 0000000..c372c62 --- /dev/null +++ b/internal/controller/function_deploy.go @@ -0,0 +1,100 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "os" + + "github.com/functions-dev/func-operator/api/v1alpha1" + "github.com/functions-dev/func-operator/internal/funccli" + "github.com/functions-dev/func-operator/internal/git" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +func (r *FunctionReconciler) deploy(ctx context.Context, function *v1alpha1.Function, repo *git.Repository) error { + logger := log.FromContext(ctx) + + if err := r.setupPipelineRBAC(ctx, function); err != nil { + return fmt.Errorf("failed to setup pipeline RBAC: %w", err) + } + + // deploy function + deployOptions := funccli.DeployOptions{} + + if function.Spec.Registry.AuthSecretRef != nil && function.Spec.Registry.AuthSecretRef.Name != "" { + // we have a registry auth secret referenced -> use this for func deploy + authFile, err := r.persistRegistryAuthSecret(ctx, function) + if err != nil { + return fmt.Errorf("failed to persist registry auth secret temporarily: %w", err) + } + + defer os.Remove(authFile) + + deployOptions.RegistryAuthFile = authFile + } + + logger.Info("Deploying function", "deployOptions", deployOptions) + err := r.FuncCliManager.Deploy(ctx, repo.Path(), function.Namespace, deployOptions) + if err != nil { + return fmt.Errorf("failed to deploy function: %w", err) + } + + logger.Info("function deployed successfully") + + return nil +} + +func (r *FunctionReconciler) persistRegistryAuthSecret(ctx context.Context, function *v1alpha1.Function) (string, error) { + logger := log.FromContext(ctx) + + logger.Info("Persist registry auth secret temporarily") + + authSecret := &v1.Secret{} + err := r.Get(ctx, types.NamespacedName{Name: function.Spec.Registry.AuthSecretRef.Name, Namespace: function.Namespace}, authSecret) + if err != nil { + logger.Error(err, "Failed to get registry auth secret", "secret", function.Spec.Registry.AuthSecretRef.Name, "namespace", function.Namespace) + return "", fmt.Errorf("failed to get registry auth secret: %w", err) + } + + if authSecret.Type != v1.SecretTypeDockerConfigJson { + return "", fmt.Errorf("invalid registry auth secret type, must be of type %s", v1.SecretTypeDockerConfigJson) + } + + if authSecret.Data[v1.DockerConfigJsonKey] == nil { + return "", fmt.Errorf("invalid registry auth secret data, must contain key %s", v1.DockerConfigJsonKey) + } + + // persist secret temporarily + authFile, err := os.CreateTemp("", "auth-file-*.json") + if err != nil { + logger.Error(err, "Failed to create temp auth file") + return "", fmt.Errorf("failed to create temp auth file: %w", err) + } + defer authFile.Close() + + _, err = authFile.Write(authSecret.Data[v1.DockerConfigJsonKey]) + if err != nil { + logger.Error(err, "Failed to write temp auth file") + return "", fmt.Errorf("failed to write temp auth file: %w", err) + } + + return authFile.Name(), nil +} diff --git a/internal/controller/function_rbac.go b/internal/controller/function_rbac.go new file mode 100644 index 0000000..6c7fb12 --- /dev/null +++ b/internal/controller/function_rbac.go @@ -0,0 +1,166 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + + "github.com/functions-dev/func-operator/api/v1alpha1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +func (r *FunctionReconciler) setupPipelineRBAC(ctx context.Context, function *v1alpha1.Function) error { + if err := r.ensureDeployFunctionRole(ctx, function.Namespace); err != nil { + return fmt.Errorf("failed to ensure deploy-function role: %w", err) + } + + if err := r.ensureDeployFunctionRoleBinding(ctx, function); err != nil { + return fmt.Errorf("failed to ensure deploy-function role binding: %w", err) + } + + return nil +} + +// ensureDeployFunctionRole ensures the deploy-function Role exists in the namespace and is up-to-date. +// This is a namespace-scoped Role so multiple operator instances won't conflict. +func (r *FunctionReconciler) ensureDeployFunctionRole(ctx context.Context, namespace string) error { + logger := log.FromContext(ctx) + + // TODO: only add the rules which are needed for the functions deployer + expectedRole := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: deployFunctionRoleName, + Namespace: namespace, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"serving.knative.dev"}, + Resources: []string{"services", "routes"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, { + APIGroups: []string{"eventing.knative.dev"}, + Resources: []string{"triggers"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, { + APIGroups: []string{"apps"}, + Resources: []string{"deployments", "replicasets"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, { + APIGroups: []string{""}, + Resources: []string{"services", "pods"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, { + APIGroups: []string{"http.keda.sh"}, + Resources: []string{"httpscaledobjects"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + }, + } + + foundRole := &rbacv1.Role{} + err := r.Get(ctx, types.NamespacedName{Name: expectedRole.Name, Namespace: expectedRole.Namespace}, foundRole) + if err != nil { + if apierrors.IsNotFound(err) { + if err := r.Create(ctx, expectedRole); err != nil { + return fmt.Errorf("failed to create role: %w", err) + } + logger.Info("Created deploy-function role") + return nil + } + return fmt.Errorf("failed to get role: %w", err) + } + + // Role exists - update if needed + if !equality.Semantic.DeepEqual(expectedRole.Rules, foundRole.Rules) { + foundRole.Rules = expectedRole.Rules + if err := r.Update(ctx, foundRole); err != nil { + return fmt.Errorf("failed to update role: %w", err) + } + logger.Info("Updated deploy-function role") + } else { + logger.Info("Deploy-function role already up to date") + } + + return nil +} + +// ensureDeployFunctionRoleBinding ensures the RoleBinding for the deploy-function role exists and is up-to-date. +func (r *FunctionReconciler) ensureDeployFunctionRoleBinding(ctx context.Context, function *v1alpha1.Function) error { + logger := log.FromContext(ctx) + + expectedRoleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deploy-function-default", + Namespace: function.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "Function", + Name: function.Name, + UID: function.UID, + Controller: ptr.To(true), + }, + }, + }, + Subjects: []rbacv1.Subject{{ + Kind: "ServiceAccount", + Name: "default", + Namespace: function.Namespace, + }}, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: deployFunctionRoleName, + }, + } + + foundRoleBinding := &rbacv1.RoleBinding{} + err := r.Get(ctx, types.NamespacedName{Name: expectedRoleBinding.Name, Namespace: expectedRoleBinding.Namespace}, foundRoleBinding) + if err != nil { + if apierrors.IsNotFound(err) { + if err := r.Create(ctx, expectedRoleBinding); err != nil { + return fmt.Errorf("failed to create role binding: %w", err) + } + logger.Info("Created deploy-function role binding") + return nil + } + return fmt.Errorf("failed to get role binding: %w", err) + } + + // Update if needed + if !equality.Semantic.DeepDerivative(expectedRoleBinding, foundRoleBinding) { + foundRoleBinding.Subjects = expectedRoleBinding.Subjects + foundRoleBinding.RoleRef = expectedRoleBinding.RoleRef + foundRoleBinding.OwnerReferences = expectedRoleBinding.OwnerReferences + + if err := r.Update(ctx, foundRoleBinding); err != nil { + return fmt.Errorf("failed to update role binding: %w", err) + } + logger.Info("Updated deploy-function role binding") + } else { + logger.Info("Deploy-function role binding already up to date") + } + + return nil +} From a8574896ff0c8285ea4e223202b2e7d44aee15fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Fri, 24 Apr 2026 14:47:10 +0200 Subject: [PATCH 5/7] Move annotation cleanup into reconcile() Move removeFuncAnnotations from the top-level Reconcile method into reconcile(), grouping it with the other reconciliation steps. This keeps Reconcile focused on status tracking and error handling while reconcile() owns the full business logic sequence. --- internal/controller/function_controller.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index eed3013..9e55d90 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -119,11 +119,6 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, reconcileErr } - if err := r.removeFuncAnnotations(ctx, function); err != nil { - logger.Error(err, "Failed to remove func annotations") - return ctrl.Result{}, err - } - logger.Info("Reconciliation complete") return ctrl.Result{}, nil } @@ -145,6 +140,10 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F return fmt.Errorf("deploying function failed: %w", err) } + if err := r.removeFuncAnnotations(ctx, function); err != nil { + return fmt.Errorf("failed to remove func annotations: %w", err) + } + return nil } From 6191bfa63c09342184c19503315c83b2539658b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Sun, 26 Apr 2026 21:05:38 +0200 Subject: [PATCH 6/7] Update log message for function deployment status --- internal/controller/function_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index 9e55d90..797219d 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -226,7 +226,7 @@ func (r *FunctionReconciler) reconcileDeployment(ctx context.Context, function * if !deployed { logger.Info("Function is not deployed") - function.MarkDeployNotReady("NotDeployed", "Function not deployed") + function.MarkDeployNotReady("NotDeployed", "Function not deployed yet") return nil } From c8585af46070cca04d80507e3b27d3d553bb1ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Mon, 27 Apr 2026 10:37:33 +0200 Subject: [PATCH 7/7] Move last-deployed annotation handling into reconcileDeployment --- internal/controller/function_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index 797219d..a17fb76 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -134,7 +134,6 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F defer repo.Cleanup() function.Status.Name = metadata.Name - applyLastDeployedAnnotation(ctx, function) if err := r.reconcileDeployment(ctx, function, repo, metadata); err != nil { return fmt.Errorf("deploying function failed: %w", err) @@ -238,6 +237,7 @@ func (r *FunctionReconciler) reconcileDeployment(ctx context.Context, function * } function.Status.Deployment.Deployer = deployer function.Status.Deployment.Runtime = metadata.Runtime + applyLastDeployedAnnotation(ctx, function) // Function is deployed - check middleware version return r.handleMiddlewareUpdate(ctx, function, repo, metadata)