Skip to content

fix: added wait functionality for portal secret #123

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

Merged
merged 16 commits into from
Oct 24, 2025
Merged

fix: added wait functionality for portal secret #123

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 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
e7228e8
fix: added wait functionaly for portal secret
OlegErshov Oct 16, 2025
15376b1
chore: removed log
OlegErshov Oct 16, 2025
c920b66
improved test coverage
OlegErshov Oct 16, 2025
296dac5
chore: fixed typos
OlegErshov Oct 16, 2025
7aa4739
chore: refactored tests
OlegErshov Oct 16, 2025
ed947cb
feat: added timeout for secret waiting
OlegErshov Oct 17, 2025
e8f9bc6
chore: made timeout configurable by config
OlegErshov Oct 17, 2025
400a9d1
Merge branch 'feat/improve-testability' into fix/wait-until-the-secre…
OlegErshov Oct 23, 2025
2aee8fc
Merge branch 'main' into fix/wait-until-the-secret-is-created
OlegErshov Oct 23, 2025
186131f
fix: fixed merge errors
OlegErshov Oct 23, 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
2 changes: 1 addition & 1 deletion internal/controller/initializer_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cf
subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr),
subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg),
subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain),
subroutine.NewRemoveInitializer(mgr, cfg.InitializerName),
subroutine.NewRemoveInitializer(mgr, cfg.InitializerName, inClusterClient),
}, log).
WithReadOnly().
BuildMultiCluster(mgr),
Expand Down
31 changes: 30 additions & 1 deletion internal/subroutine/remove_initializer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,30 @@ import (
"context"
"fmt"
"slices"
"time"

"github.com/kcp-dev/kcp/sdk/apis/cache/initialization"
kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
"github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject"
"github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine"
"github.com/platform-mesh/golang-commons/errors"
"github.com/rs/zerolog/log"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
)

const (
portalClientSecretNamespace = "platform-mesh-system"
)

type removeInitializer struct {
initializerName string
mgr mcmanager.Manager
runtimeClient client.Client
}
Copy link

@coderabbitai coderabbitai bot Oct 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Defensive nil check for runtimeClient to prevent panics.

If runtimeClient is ever nil, r.runtimeClient.Get will panic.

 func (r *removeInitializer) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) {
@@
-	secretName := fmt.Sprintf("portal-client-secret-%s", workspaceName)
+	secretName := fmt.Sprintf("portal-client-secret-%s", workspaceName)
 	key := types.NamespacedName{Name: secretName, Namespace: PortalClientSecretNamespace}
 
+	if r.runtimeClient == nil {
+		return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("runtime client is not configured"), false, true)
+	}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In internal/subroutine/remove_initializer.go around lines 30 to 31, add a
defensive nil check for r.runtimeClient before any use: if r.runtimeClient is
nil return an appropriate error (or wrap and return context-aware error) instead
of calling r.runtimeClient.Get which would panic; update the function to detect
nil early, return the error up the call stack, and ensure callers handle that
error.


// Finalize implements subroutine.Subroutine.
Expand Down Expand Up @@ -48,6 +57,25 @@ func (r *removeInitializer) Process(ctx context.Context, instance runtimeobject.
return ctrl.Result{}, nil
}

// we need to wait untill keycloak crossplane provider creates a portal secret
workspaceName := getWorkspaceName(lc)
if workspaceName == "" {
return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get workspace path"), true, false)
}

secretName := fmt.Sprintf("portal-client-secret-%s", workspaceName)
key := types.NamespacedName{Name: secretName, Namespace: portalClientSecretNamespace}

var secret corev1.Secret
if err := r.runtimeClient.Get(ctx, key, &secret); err != nil {
if apierrors.IsNotFound(err) {
log.Info().Msg(fmt.Sprintf("realm secret %s is not ready yet, trying again", secretName))
return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
Copy link
Contributor

@nexus49 nexus49 Oct 17, 2025

Choose a reason for hiding this comment

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

I also think it would be good to transition to return an error after a certain time has passed, that would also be the time where a sentry notification should be send and a error log should be printed. Maybe we start with a 1 minute (if more then a minute has passed since logicalcluster creation)

}
log.Info().Msg(fmt.Sprintf("failed to get realm secret %s, err -- %s", secretName, err))
return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get secret %s: %w", secretName, err), true, true)
}

patch := client.MergeFrom(lc.DeepCopy())

lc.Status.Initializers = initialization.EnsureInitializerAbsent(initializer, lc.Status.Initializers)
Expand All @@ -60,10 +88,11 @@ func (r *removeInitializer) Process(ctx context.Context, instance runtimeobject.
return ctrl.Result{}, nil
}

func NewRemoveInitializer(mgr mcmanager.Manager, initializerName string) *removeInitializer {
func NewRemoveInitializer(mgr mcmanager.Manager, initializerName string, runtimeClient client.Client) *removeInitializer {
return &removeInitializer{
initializerName: initializerName,
mgr: mgr,
runtimeClient: runtimeClient,
}
}

Expand Down
22 changes: 17 additions & 5 deletions internal/subroutine/remove_initializer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/platform-mesh/security-operator/internal/subroutine/mocks"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -43,23 +44,27 @@ func TestRemoveInitializer_Process(t *testing.T) {

t.Run("skips when initializer is absent", func(t *testing.T) {
mgr := mocks.NewMockManager(t)
runtimeClient := mocks.NewMockClient(t)
cluster := mocks.NewMockCluster(t)
mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil)

lc := &kcpv1alpha1.LogicalCluster{}
lc.Status.Initializers = []kcpv1alpha1.LogicalClusterInitializer{"other.initializer"}

r := subroutine.NewRemoveInitializer(mgr, initializerName)
r := subroutine.NewRemoveInitializer(mgr, initializerName, runtimeClient)
_, err := r.Process(context.Background(), lc)
assert.Nil(t, err)
})

t.Run("removes initializer and patches status", func(t *testing.T) {
mgr := mocks.NewMockManager(t)
cluster := mocks.NewMockCluster(t)
runtimeClient := mocks.NewMockClient(t)
k8s := mocks.NewMockClient(t)

mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil)
// Secret must exist for the flow to proceed
runtimeClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "portal-client-secret-test", Namespace: "platform-mesh-system"}, mock.Anything).Return(nil)
cluster.EXPECT().GetClient().Return(k8s)
k8s.EXPECT().Status().Return(&fakeStatusWriter{t: t, expectClear: kcpv1alpha1.LogicalClusterInitializer(initializerName), err: nil})

Expand All @@ -68,8 +73,9 @@ func TestRemoveInitializer_Process(t *testing.T) {
kcpv1alpha1.LogicalClusterInitializer(initializerName),
"another.initializer",
}
lc.Annotations = map[string]string{"kcp.io/path": "root:org:test"}

r := subroutine.NewRemoveInitializer(mgr, initializerName)
r := subroutine.NewRemoveInitializer(mgr, initializerName, runtimeClient)
_, err := r.Process(context.Background(), lc)
assert.Nil(t, err)
// ensure it's removed in in-memory object as well
Expand All @@ -81,26 +87,31 @@ func TestRemoveInitializer_Process(t *testing.T) {
t.Run("returns error when status patch fails", func(t *testing.T) {
mgr := mocks.NewMockManager(t)
cluster := mocks.NewMockCluster(t)
runtimeClient := mocks.NewMockClient(t)
k8s := mocks.NewMockClient(t)

mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil)
// Secret exists so we hit the patch failure path
runtimeClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "portal-client-secret-test", Namespace: "platform-mesh-system"}, mock.Anything).Return(nil)
cluster.EXPECT().GetClient().Return(k8s)
k8s.EXPECT().Status().Return(&fakeStatusWriter{t: t, expectClear: kcpv1alpha1.LogicalClusterInitializer(initializerName), err: assert.AnError})

lc := &kcpv1alpha1.LogicalCluster{}
lc.Status.Initializers = []kcpv1alpha1.LogicalClusterInitializer{
kcpv1alpha1.LogicalClusterInitializer(initializerName),
}
lc.Annotations = map[string]string{"kcp.io/path": "root:org:test"}

r := subroutine.NewRemoveInitializer(mgr, initializerName)
r := subroutine.NewRemoveInitializer(mgr, initializerName, runtimeClient)
_, err := r.Process(context.Background(), lc)
assert.NotNil(t, err)
})
}

func TestRemoveInitializer_Misc(t *testing.T) {
mgr := mocks.NewMockManager(t)
r := subroutine.NewRemoveInitializer(mgr, "foo.initializer.kcp.dev")
runtimeClient := mocks.NewMockClient(t)
r := subroutine.NewRemoveInitializer(mgr, "foo.initializer.kcp.dev", runtimeClient)

assert.Equal(t, "RemoveInitializer", r.GetName())
assert.Equal(t, []string{}, r.Finalizers(nil))
Expand All @@ -113,8 +124,9 @@ func TestRemoveInitializer_ManagerError(t *testing.T) {
mgr := mocks.NewMockManager(t)
// Simulate error fetching cluster from context
mgr.EXPECT().ClusterFromContext(mock.Anything).Return(nil, assert.AnError)
runtimeClient := mocks.NewMockClient(t)

r := subroutine.NewRemoveInitializer(mgr, "foo.initializer.kcp.dev")
r := subroutine.NewRemoveInitializer(mgr, "foo.initializer.kcp.dev", runtimeClient)
_, err := r.Process(context.Background(), &kcpv1alpha1.LogicalCluster{})
assert.NotNil(t, err)
}
Loading