Skip to content

feat/improve-testability #113

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

feat/improve-testability #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ac4ea79
refactor(initializer): improve kubeconfig handling
nexus49 Oct 14, 2025
f27b782
refactor(config): enhance configuration structure for KCP
nexus49 Oct 14, 2025
19554ff
Merge remote-tracking branch 'origin/main' into feat/improve-testability
OlegErshov Oct 15, 2025
2ec28e0
refactor(generator and operator): separated kcp and runtime kubeconfigs
OlegErshov Oct 15, 2025
9658aa4
fix: fixed getting config function
OlegErshov Oct 15, 2025
cb24090
fix: removed extra error handler
OlegErshov Oct 15, 2025
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
20 changes: 10 additions & 10 deletions cmd/initializer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ var initializerCmd = &cobra.Command{
ctx, _, shutdown := pmcontext.StartContext(log, initializerCfg, defaultCfg.ShutdownTimeout)
defer shutdown()

mgrCfg := ctrl.GetConfigOrDie()
restCfg, err := getKubeconfigFromPath(initializerCfg.KCP.Kubeconfig)
if err != nil {
log.Error().Err(err).Msg("unable to get KCP kubeconfig")
os.Exit(1)
}

mgrOpts := ctrl.Options{
Scheme: scheme,
Expand All @@ -57,7 +61,7 @@ var initializerCmd = &cobra.Command{
mgrOpts.LeaderElectionConfig = inClusterCfg
}

provider, err := initializingworkspaces.New(mgrCfg, initializingworkspaces.Options{
provider, err := initializingworkspaces.New(restCfg, initializingworkspaces.Options{
InitializerName: initializerCfg.InitializerName,
Scheme: mgrOpts.Scheme,
})
Expand All @@ -66,7 +70,7 @@ var initializerCmd = &cobra.Command{
os.Exit(1)
}

mgr, err := mcmanager.New(mgrCfg, provider, mgrOpts)
mgr, err := mcmanager.New(restCfg, provider, mgrOpts)
if err != nil {
setupLog.Error(err, "Failed to create manager")
os.Exit(1)
Expand All @@ -82,13 +86,9 @@ var initializerCmd = &cobra.Command{
os.Exit(1)
}

inClusterConfig, err := rest.InClusterConfig()
if err != nil {
log.Error().Err(err).Msg("Failed to create in cluster config")
os.Exit(1)
}
k8sCfg := ctrl.GetConfigOrDie()

inClusterClient, err := client.New(inClusterConfig, client.Options{Scheme: scheme})
runtimeClient, err := client.New(k8sCfg, client.Options{Scheme: scheme})
if err != nil {
log.Error().Err(err).Msg("Failed to create in cluster client")
os.Exit(1)
Expand All @@ -98,7 +98,7 @@ var initializerCmd = &cobra.Command{
initializerCfg.IDP.AdditionalRedirectURLs = []string{}
}

if err := controller.NewLogicalClusterReconciler(log, orgClient, initializerCfg, inClusterClient, mgr).
if err := controller.NewLogicalClusterReconciler(log, orgClient, initializerCfg, runtimeClient, mgr).
SetupWithManager(mgr, defaultCfg); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "LogicalCluster")
os.Exit(1)
Expand Down
11 changes: 7 additions & 4 deletions cmd/model_generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@ import (
var modelGeneratorCmd = &cobra.Command{
Use: "model-generator",
RunE: func(cmd *cobra.Command, args []string) error {

ctrl.SetLogger(log.ComponentLogger("controller-runtime").Logr())

ctx, _, shutdown := platformeshcontext.StartContext(log, defaultCfg, defaultCfg.ShutdownTimeout)
defer shutdown()

cfg := ctrl.GetConfigOrDie()
restCfg, err := getKubeconfigFromPath(generatorCfg.KCP.Kubeconfig)
if err != nil {
log.Error().Err(err).Msg("unable to get KCP kubeconfig")
return err
}

mgrOpts := manager.Options{
Scheme: scheme,
Expand Down Expand Up @@ -67,15 +70,15 @@ var modelGeneratorCmd = &cobra.Command{
return fmt.Errorf("scheme should not be nil")
}

provider, err := apiexport.New(cfg, apiexport.Options{
provider, err := apiexport.New(restCfg, apiexport.Options{
Scheme: mgrOpts.Scheme,
})
if err != nil {
log.Error().Err(err).Msg("Failed to create apiexport provider")
return err
}

mgr, err := mcmanager.New(cfg, provider, mgrOpts)
mgr, err := mcmanager.New(restCfg, provider, mgrOpts)
if err != nil {
log.Error().Err(err).Msg("Failed to create manager")
return err
Expand Down
12 changes: 8 additions & 4 deletions cmd/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ var operatorCmd = &cobra.Command{
ctx, _, shutdown := platformeshcontext.StartContext(log, defaultCfg, defaultCfg.ShutdownTimeout)
defer shutdown()

restCfg, err := getKubeconfigFromPath(operatorCfg.KCP.Kubeconfig)
if err != nil {
log.Error().Err(err).Msg("unable to get KCP kubeconfig")
return err
}

Comment on lines +80 to +85
Copy link

@coderabbitai coderabbitai bot Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Kubeconfig loader: confirm fallback/edge-cases.

Ensure getKubeconfigFromPath handles empty/missing paths (fallback to in-cluster or KUBECONFIG), and respects auth plugins and exec providers. If not, add fallback to ctrl.GetConfig().

Run to verify presence and usage:

🏁 Script executed:

#!/bin/bash
rg -nP --type=go -C2 '\bfunc\s+getKubeconfigFromPath\s*\('
rg -nP --type=go -C2 'getKubeconfigFromPath\s*\('

Length of output: 1357

Support in-cluster and environment kubeconfigs when path is unset
getKubeconfigFromPath currently errors on an empty --kcp-kubeconfig value. Replace that error path with a call to ctrl.GetConfig() so it falls back to in-cluster config or $KUBECONFIG and honors auth plugins/exec providers.

🤖 Prompt for AI Agents 
In cmd/operator.go around lines 80-85, the current call to getKubeconfigFromPath
errors when operatorCfg.KCP.Kubeconfig is empty; change the logic so if
operatorCfg.KCP.Kubeconfig is empty you call ctrl.GetConfig() (which falls back
to in-cluster or $KUBECONFIG and supports auth plugins/exec providers) and
otherwise call getKubeconfigFromPath(path); propagate and log any error from
ctrl.GetConfig() or getKubeconfigFromPath as before.

if defaultCfg.Sentry.Dsn != "" {
err := sentry.Start(ctx,
defaultCfg.Sentry.Dsn, defaultCfg.Environment, defaultCfg.Region,
Expand All @@ -89,8 +95,6 @@ var operatorCmd = &cobra.Command{
defer platformeshcontext.Recover(log)
}

cfg := ctrl.GetConfigOrDie()

mgrOpts := ctrl.Options{
Scheme: scheme,
Metrics: metricsserver.Options{
Expand Down Expand Up @@ -121,15 +125,15 @@ var operatorCmd = &cobra.Command{
return fmt.Errorf("scheme should not be nil")
}

provider, err := apiexport.New(cfg, apiexport.Options{
provider, err := apiexport.New(restCfg, apiexport.Options{
Scheme: mgrOpts.Scheme,
})
if err != nil {
setupLog.Error(err, "unable to construct cluster provider")
return err
}

mgr, err := mcmanager.New(cfg, provider, mgrOpts)
mgr, err := mcmanager.New(restCfg, provider, mgrOpts)
if err != nil {
setupLog.Error(err, "Failed to create manager")
return err
Expand Down
23 changes: 23 additions & 0 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"errors"
"flag"
"strings"

Expand All @@ -9,6 +10,8 @@ import (
"github.com/platform-mesh/golang-commons/logger"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
ctrl "sigs.k8s.io/controller-runtime"

"github.com/platform-mesh/security-operator/internal/config"
Expand All @@ -18,6 +21,7 @@ var (
defaultCfg *platformeshconfig.CommonServiceConfig
initializerCfg config.Config
operatorCfg config.Config
generatorCfg config.Config
log *logger.Logger
setupLog logr.Logger
)
Expand All @@ -43,6 +47,10 @@ func init() {
if err := platformeshconfig.BindConfigToFlags(operatorV, operatorCmd, &operatorCfg); err != nil {
panic(err)
}
generatorV := newViper()
if err := platformeshconfig.BindConfigToFlags(generatorV, modelGeneratorCmd, &generatorCfg); err != nil {
panic(err)
}
initializerV := newViper()
if err := platformeshconfig.BindConfigToFlags(initializerV, initializerCmd, &initializerCfg); err != nil {
panic(err)
Expand All @@ -51,6 +59,21 @@ func init() {
cobra.OnInitialize(initLog)
}

func getKubeconfigFromPath(kubeconfigPath string) (*rest.Config, error) {
if kubeconfigPath == "" {
return nil, errors.New("missing value for required flag --kcp-kubeconfig")
}
cfg, err := clientcmd.LoadFromFile(kubeconfigPath)
if err != nil {
return nil, err
}
restCfg, err := clientcmd.NewDefaultClientConfig(*cfg, &clientcmd.ConfigOverrides{}).ClientConfig()
if err != nil {
return restCfg, err
}
return restCfg, nil
}

func newViper() *viper.Viper {
v := viper.NewWithOptions(
viper.EnvKeyReplacer(strings.NewReplacer("-", "_")),
Expand Down
3 changes: 3 additions & 0 deletions internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ type Config struct {
FGA struct {
Target string `mapstructure:"fga-target"`
} `mapstructure:",squash"`
KCP struct {
Kubeconfig string `mapstructure:"kcp-kubeconfig" default:"/api-kubeconfig/kubeconfig"`
} `mapstructure:",squash"`
APIExportEndpointSliceName string `mapstructure:"api-export-endpoint-slice-name"`
CoreModulePath string `mapstructure:"core-module-path"`
WorkspaceDir string `mapstructure:"workspace-dir" default:"/operator/"`
Expand Down
12 changes: 6 additions & 6 deletions internal/controller/apibinding_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/predicate"

lifecyclecontrollerruntime "github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
"github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
"github.com/platform-mesh/security-operator/internal/subroutine"
mccontext "sigs.k8s.io/multicluster-runtime/pkg/context"
mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
Expand All @@ -21,23 +21,23 @@ import (
func NewAPIBindingReconciler(logger *logger.Logger, mcMgr mcmanager.Manager) *APIBindingReconciler {
return &APIBindingReconciler{
log: logger,
lifecycle: builder.NewBuilder("apibinding", "apibinding-controller", []lifecyclesubroutine.Subroutine{
mclifecycle: builder.NewBuilder("apibinding", "apibinding-controller", []lifecyclesubroutine.Subroutine{
subroutine.NewAuthorizationModelGenerationSubroutine(mcMgr),
}, logger).
BuildMultiCluster(mcMgr),
}
}

type APIBindingReconciler struct {
log *logger.Logger
lifecycle *lifecyclecontrollerruntime.LifecycleManager
log *logger.Logger
mclifecycle *multicluster.LifecycleManager
}

func (r *APIBindingReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) {
ctxWithCluster := mccontext.WithCluster(ctx, req.ClusterName)
return r.lifecycle.Reconcile(ctxWithCluster, req, &kcpv1alpha1.APIBinding{})
return r.mclifecycle.Reconcile(ctxWithCluster, req, &kcpv1alpha1.APIBinding{})
}

func (r *APIBindingReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platformeshconfig.CommonServiceConfig, evp ...predicate.Predicate) error {
return r.lifecycle.SetupWithManager(mgr, cfg.MaxConcurrentReconciles, "apibinding-controller", &kcpv1alpha1.APIBinding{}, cfg.DebugLabelValue, r, r.log, evp...)
return r.mclifecycle.SetupWithManager(mgr, cfg.MaxConcurrentReconciles, "apibinding-controller", &kcpv1alpha1.APIBinding{}, cfg.DebugLabelValue, r, r.log, evp...)
}
12 changes: 6 additions & 6 deletions internal/controller/authorization_model_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
openfgav1 "github.com/openfga/api/proto/openfga/v1"
platformeshconfig "github.com/platform-mesh/golang-commons/config"
"github.com/platform-mesh/golang-commons/controller/lifecycle/builder"
lifecyclecontrollerruntime "github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
"github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine"
"github.com/platform-mesh/golang-commons/logger"
corev1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1"
Expand All @@ -19,14 +19,14 @@ import (
)

type AuthorizationModelReconciler struct {
log *logger.Logger
lifecycle *lifecyclecontrollerruntime.LifecycleManager
log *logger.Logger
mclifecycle *multicluster.LifecycleManager
}

func NewAuthorizationModelReconciler(log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager) *AuthorizationModelReconciler {
return &AuthorizationModelReconciler{
log: log,
lifecycle: builder.NewBuilder("authorizationmodel", "AuthorizationModelReconciler", []lifecyclesubroutine.Subroutine{
mclifecycle: builder.NewBuilder("authorizationmodel", "AuthorizationModelReconciler", []lifecyclesubroutine.Subroutine{
subroutine.NewTupleSubroutine(fga, mcMgr),
}, log).
BuildMultiCluster(mcMgr),
Expand All @@ -35,9 +35,9 @@ func NewAuthorizationModelReconciler(log *logger.Logger, fga openfgav1.OpenFGASe

func (r *AuthorizationModelReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) {
ctxWithCluster := mccontext.WithCluster(ctx, req.ClusterName)
return r.lifecycle.Reconcile(ctxWithCluster, req, &corev1alpha1.AuthorizationModel{})
return r.mclifecycle.Reconcile(ctxWithCluster, req, &corev1alpha1.AuthorizationModel{})
}

func (r *AuthorizationModelReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platformeshconfig.CommonServiceConfig, evp ...predicate.Predicate) error { // coverage-ignore
return r.lifecycle.SetupWithManager(mgr, cfg.MaxConcurrentReconciles, "authorizationmodel", &corev1alpha1.AuthorizationModel{}, cfg.DebugLabelValue, r, r.log, evp...)
return r.mclifecycle.SetupWithManager(mgr, cfg.MaxConcurrentReconciles, "authorizationmodel", &corev1alpha1.AuthorizationModel{}, cfg.DebugLabelValue, r, r.log, evp...)
}
10 changes: 5 additions & 5 deletions internal/controller/initializer_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
platformeshconfig "github.com/platform-mesh/golang-commons/config"
"github.com/platform-mesh/golang-commons/controller/lifecycle/builder"
lifecyclecontrollerruntime "github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
"github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine"
"github.com/platform-mesh/golang-commons/logger"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -23,13 +23,13 @@ import (
type LogicalClusterReconciler struct {
log *logger.Logger

lifecycle *lifecyclecontrollerruntime.LifecycleManager
mclifecycle *multicluster.LifecycleManager
}

func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cfg config.Config, inClusterClient client.Client, mgr mcmanager.Manager) *LogicalClusterReconciler {
return &LogicalClusterReconciler{
log: log,
lifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{
mclifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{
subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr),
subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg),
subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain),
Expand All @@ -42,9 +42,9 @@ func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cf

func (r *LogicalClusterReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) {
ctxWithCluster := mccontext.WithCluster(ctx, req.ClusterName)
return r.lifecycle.Reconcile(ctxWithCluster, req, &kcpcorev1alpha1.LogicalCluster{})
return r.mclifecycle.Reconcile(ctxWithCluster, req, &kcpcorev1alpha1.LogicalCluster{})
}

func (r *LogicalClusterReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platformeshconfig.CommonServiceConfig, evp ...predicate.Predicate) error {
return r.lifecycle.SetupWithManager(mgr, cfg.MaxConcurrentReconciles, "LogicalCluster", &kcpcorev1alpha1.LogicalCluster{}, cfg.DebugLabelValue, r, r.log, evp...)
return r.mclifecycle.SetupWithManager(mgr, cfg.MaxConcurrentReconciles, "LogicalCluster", &kcpcorev1alpha1.LogicalCluster{}, cfg.DebugLabelValue, r, r.log, evp...)
}
10 changes: 5 additions & 5 deletions internal/controller/invite_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

platformeshconfig "github.com/platform-mesh/golang-commons/config"
"github.com/platform-mesh/golang-commons/controller/lifecycle/builder"
lifecycle "github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
"github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine"
"github.com/platform-mesh/golang-commons/logger"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -20,7 +20,7 @@ import (
)

type InviteReconciler struct {
lifecycle *lifecycle.LifecycleManager
mclifecycle *multicluster.LifecycleManager
}

func NewInviteReconciler(ctx context.Context, mgr mcmanager.Manager, cfg *config.Config, log *logger.Logger) *InviteReconciler {
Expand All @@ -35,7 +35,7 @@ func NewInviteReconciler(ctx context.Context, mgr mcmanager.Manager, cfg *config
}

return &InviteReconciler{
lifecycle: builder.NewBuilder(
mclifecycle: builder.NewBuilder(
"invite",
"InviteReconciler",
[]lifecyclesubroutine.Subroutine{
Expand All @@ -46,11 +46,11 @@ func NewInviteReconciler(ctx context.Context, mgr mcmanager.Manager, cfg *config
}

func (r *InviteReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) {
return r.lifecycle.Reconcile(mccontext.WithCluster(ctx, req.ClusterName), req, &v1alpha1.Invite{})
return r.mclifecycle.Reconcile(mccontext.WithCluster(ctx, req.ClusterName), req, &v1alpha1.Invite{})
}

func (r *InviteReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platformeshconfig.CommonServiceConfig, log *logger.Logger) error { // coverage-ignore
return r.lifecycle.
return r.mclifecycle.
SetupWithManager(
mgr,
cfg.MaxConcurrentReconciles,
Expand Down
Loading