diff --git a/internal/config/config.go b/internal/config/config.go index dc5dd7c8..d6ea7c73 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,16 +14,15 @@ type Config struct { 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/"` - BaseDomain string `mapstructure:"base-domain" default:"portal.dev.local:8443"` - GroupClaim string `mapstructure:"group-claim" default:"groups"` - UserClaim string `mapstructure:"user-claim" default:"email"` - InitializerName string `mapstructure:"initializer-name" default:"root:security"` - DomainCALookup bool `mapstructure:"domain-ca-lookup" default:"false"` - SecretWaitingTimeoutInSeconds int `mapstructure:"secret-waiting-timeout-seconds" default:"60"` - IDP struct { + APIExportEndpointSliceName string `mapstructure:"api-export-endpoint-slice-name"` + CoreModulePath string `mapstructure:"core-module-path"` + BaseDomain string `mapstructure:"base-domain" default:"portal.dev.local:8443"` + GroupClaim string `mapstructure:"group-claim" default:"groups"` + UserClaim string `mapstructure:"user-claim" default:"email"` + InitializerName string `mapstructure:"initializer-name" default:"root:security"` + DomainCALookup bool `mapstructure:"domain-ca-lookup" default:"false"` + HttpClientTimeoutSeconds int `mapstructure:"http-client-timeout-seconds" default:"30"` + IDP struct { // SMTP settings SMTPServer string `mapstructure:"idp-smtp-server"` SMTPPort int `mapstructure:"idp-smtp-port"` @@ -34,11 +33,8 @@ type Config struct { StartTLS bool `mapstructure:"idp-smtp-starttls" default:"false"` // Auth settings - SMTPUser string `mapstructure:"idp-smtp-user"` - // secret name and key will be removed with idp creation subroutine - SMTPPasswordSecretName string `mapstructure:"idp-smtp-password-secret-name"` - SMTPPasswordSecretKey string `mapstructure:"idp-smtp-password-secret-key" default:"password"` - SMTPPassword string `mapstructure:"idp-smtp-password"` + SMTPUser string `mapstructure:"idp-smtp-user"` + SMTPPassword string `mapstructure:"idp-smtp-password"` AdditionalRedirectURLs []string `mapstructure:"idp-additional-redirect-urls"` } `mapstructure:",squash"` diff --git a/internal/controller/initializer_controller.go b/internal/controller/initializer_controller.go index 8aa3d5ae..5711d7bd 100644 --- a/internal/controller/initializer_controller.go +++ b/internal/controller/initializer_controller.go @@ -31,12 +31,13 @@ func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cf log: log, mclifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{ subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr), - subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg), - subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain), + subroutine.NewIDPSubroutine(orgClient, mgr, cfg), subroutine.NewInviteSubroutine(orgClient, mgr), - subroutine.NewRemoveInitializer(mgr, cfg, inClusterClient), + subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg), + subroutine.NewRemoveInitializer(mgr, cfg), }, log). WithReadOnly(). + WithStaticThenExponentialRateLimiter(). BuildMultiCluster(mgr), } } diff --git a/internal/subroutine/idp.go b/internal/subroutine/idp.go new file mode 100644 index 00000000..25cd77f2 --- /dev/null +++ b/internal/subroutine/idp.go @@ -0,0 +1,132 @@ +package subroutine + +import ( + "context" + "fmt" + "slices" + "strings" + + kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + accountv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" + "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" + lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" + "github.com/rs/zerolog/log" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/platform-mesh/golang-commons/errors" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + + "github.com/platform-mesh/security-operator/api/v1alpha1" + "github.com/platform-mesh/security-operator/internal/config" +) + +func NewIDPSubroutine(orgsClient client.Client, mgr mcmanager.Manager, cfg config.Config) *IDPSubroutine { + return &IDPSubroutine{ + orgsClient: orgsClient, + mgr: mgr, + additionalRedirectURLs: cfg.IDP.AdditionalRedirectURLs, + baseDomain: cfg.BaseDomain, + } +} + +var _ lifecyclesubroutine.Subroutine = &IDPSubroutine{} + +type IDPSubroutine struct { + orgsClient client.Client + mgr mcmanager.Manager + additionalRedirectURLs []string + baseDomain string +} + +func (w *IDPSubroutine) Finalize(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { + return ctrl.Result{}, nil +} + +func (w *IDPSubroutine) Finalizers(_ runtimeobject.RuntimeObject) []string { + return nil +} + +func (w *IDPSubroutine) GetName() string { return "IDPSubroutine" } + +func (w *IDPSubroutine) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { + lc := instance.(*kcpv1alpha1.LogicalCluster) + + workspaceName := getWorkspaceName(lc) + if workspaceName == "" { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get workspace name"), true, false) + } + + cl, err := w.mgr.ClusterFromContext(ctx) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get cluster from context %w", err), true, true) + } + + var account accountv1alpha1.Account + err = w.orgsClient.Get(ctx, types.NamespacedName{Name: workspaceName}, &account) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get account resource %w", err), true, true) + } + + if account.Spec.Type != accountv1alpha1.AccountTypeOrg { + log.Debug().Str("workspace", workspaceName).Msg("account is not of type organization, skipping idp creation") + return ctrl.Result{}, nil + } + + clientConfig := v1alpha1.IdentityProviderClientConfig{ + ClientName: workspaceName, + ClientType: v1alpha1.IdentityProviderClientTypeConfidential, + RedirectURIs: append(w.additionalRedirectURLs, fmt.Sprintf("https://%s.%s/*", workspaceName, w.baseDomain)), + PostLogoutRedirectURIs: []string{fmt.Sprintf("https://%s.%s/logout*", workspaceName, w.baseDomain)}, + SecretRef: corev1.SecretReference{ + Name: fmt.Sprintf("portal-client-secret-%s-%s", workspaceName, workspaceName), + Namespace: "default", + }, + } + + idp := &v1alpha1.IdentityProviderConfiguration{ObjectMeta: metav1.ObjectMeta{Name: workspaceName}} + _, err = controllerutil.CreateOrUpdate(ctx, cl.GetClient(), idp, func() error { + clientIdx := slices.IndexFunc(idp.Spec.Clients, func(c v1alpha1.IdentityProviderClientConfig) bool { + return c.ClientName == clientConfig.ClientName + }) + if clientIdx != -1 { + idp.Spec.Clients[clientIdx].RedirectURIs = clientConfig.RedirectURIs + idp.Spec.Clients[clientIdx].ClientType = clientConfig.ClientType + idp.Spec.Clients[clientIdx].SecretRef = clientConfig.SecretRef + idp.Spec.Clients[clientIdx].PostLogoutRedirectURIs = clientConfig.PostLogoutRedirectURIs + return nil + } + + idp.Spec.Clients = append(idp.Spec.Clients, clientConfig) + return nil + }) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to create idp resource %w", err), true, true) + } + + log.Info().Str("workspace", workspaceName).Msg("idp configuration resource is created") + + if err := cl.GetClient().Get(ctx, types.NamespacedName{Name: workspaceName}, idp); err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get idp resource %w", err), true, true) + } + if !meta.IsStatusConditionTrue(idp.GetConditions(), "Ready") { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("idp resource is not ready yet"), true, false) + } + + log.Info().Str("workspace", workspaceName).Msg("idp resource is ready") + return ctrl.Result{}, nil +} + +func getWorkspaceName(lc *kcpv1alpha1.LogicalCluster) string { + if path, ok := lc.Annotations["kcp.io/path"]; ok { + pathElements := strings.Split(path, ":") + return pathElements[len(pathElements)-1] + } + return "" +} diff --git a/internal/subroutine/idp/subroutine.go b/internal/subroutine/idp/subroutine.go index 81972cf5..99df1f31 100644 --- a/internal/subroutine/idp/subroutine.go +++ b/internal/subroutine/idp/subroutine.go @@ -48,7 +48,7 @@ func New(ctx context.Context, cfg *config.Config, orgsClient client.Client, mgr httpClient := cCfg.Client(ctx) oidcClient := &http.Client{ - Timeout: 30 * time.Second, + Timeout: time.Duration(cfg.HttpClientTimeoutSeconds) * time.Second, } return &subroutine{ @@ -165,6 +165,7 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime log.Info().Str("secretName", managedClient.SecretRef.Name).Msg("Secret not found, client was likely deleted") continue } + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get registration access token from secret: %w", err), true, true) } if err := s.deleteClient(ctx, realmName, managedClient.ClientID, managedClient.RegistrationClientURI, registrationAccessToken); err != nil { @@ -194,7 +195,7 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime var clientInfo clientInfo if clientID != "" { registrationAccessToken, err := s.readRegistrationAccessTokenFromSecret(ctx, clientConfig.SecretRef) - if err != nil { + if err != nil && !kerrors.IsNotFound(err) { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get registration access token from secret: %w", err), true, true) } diff --git a/internal/subroutine/idp_test.go b/internal/subroutine/idp_test.go new file mode 100644 index 00000000..e4573c9e --- /dev/null +++ b/internal/subroutine/idp_test.go @@ -0,0 +1,285 @@ +package subroutine + +import ( + "context" + "testing" + + kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + accountv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" + "github.com/platform-mesh/golang-commons/logger/testlogger" + secopv1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" + "github.com/platform-mesh/security-operator/internal/config" + "github.com/platform-mesh/security-operator/internal/subroutine/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestNewIDPSubroutine(t *testing.T) { + orgsClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cfg := config.Config{} + cfg.IDP.AdditionalRedirectURLs = []string{"https://example.com/callback"} + cfg.BaseDomain = "example.com" + + subroutine := NewIDPSubroutine(orgsClient, mgr, cfg) + + assert.NotNil(t, subroutine) + assert.Equal(t, orgsClient, subroutine.orgsClient) + assert.Equal(t, mgr, subroutine.mgr) + assert.Equal(t, []string{"https://example.com/callback"}, subroutine.additionalRedirectURLs) + assert.Equal(t, "example.com", subroutine.baseDomain) +} + +func TestIDPSubroutine_GetName(t *testing.T) { + orgsClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cfg := config.Config{} + cfg.BaseDomain = "example.com" + subroutine := NewIDPSubroutine(orgsClient, mgr, cfg) + + name := subroutine.GetName() + assert.Equal(t, "IDPSubroutine", name) +} + +func TestIDPSubroutine_Finalizers(t *testing.T) { + orgsClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cfg := config.Config{} + cfg.BaseDomain = "example.com" + subroutine := NewIDPSubroutine(orgsClient, mgr, cfg) + + finalizers := subroutine.Finalizers(nil) + assert.Nil(t, finalizers) +} + +func TestIDPSubroutine_Finalize(t *testing.T) { + orgsClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cfg := config.Config{} + cfg.BaseDomain = "example.com" + subroutine := NewIDPSubroutine(orgsClient, mgr, cfg) + + ctx := context.Background() + instance := &kcpv1alpha1.LogicalCluster{} + + result, opErr := subroutine.Finalize(ctx, instance) + + assert.Nil(t, opErr) + assert.Equal(t, ctrl.Result{}, result) +} + +func TestIDPSubroutine_Process(t *testing.T) { + tests := []struct { + name string + setupMocks func(*mocks.MockClient, *mocks.MockManager, *mocks.MockCluster, config.Config) + lc *kcpv1alpha1.LogicalCluster + expectedErr bool + expectedResult ctrl.Result + }{ + { + name: "Empty workspace name - early return", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + expectedErr: true, + expectedResult: ctrl.Result{}, + }, + { + name: "ClusterFromContext error", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(nil, assert.AnError).Once() + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test", + }, + }, + }, + expectedErr: true, + expectedResult: ctrl.Result{}, + }, + { + name: "Account Get error", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test"}, mock.AnythingOfType("*v1alpha1.Account")). + Return(assert.AnError).Once() + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test", + }, + }, + }, + expectedErr: true, + expectedResult: ctrl.Result{}, + }, + { + name: "Account not of type organization - skip idp creation", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test"}, mock.AnythingOfType("*v1alpha1.Account")). + RunAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + acc := obj.(*accountv1alpha1.Account) + acc.Spec.Type = accountv1alpha1.AccountTypeAccount // Not organization type + return nil + }).Once() + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test", + }, + }, + }, + expectedErr: false, + expectedResult: ctrl.Result{}, + }, + { + name: "CreateOrUpdate and Ready", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.Account")). + RunAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + acc := obj.(*accountv1alpha1.Account) + acc.Spec.Type = accountv1alpha1.AccountTypeOrg + return nil + }).Once() + cluster.EXPECT().GetClient().Return(orgsClient).Maybe() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + Return(apierrors.NewNotFound(schema.GroupResource{Group: "core.platform-mesh.io", Resource: "identityproviderconfigurations"}, "acme")).Once() + orgsClient.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + RunAndReturn(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { + idp := obj.(*secopv1alpha1.IdentityProviderConfiguration) + assert.Len(t, idp.Spec.Clients, 1) + assert.Equal(t, "acme", idp.Spec.Clients[0].ClientName) + assert.Equal(t, "portal-client-secret-acme-acme", idp.Spec.Clients[0].SecretRef.Name) + assert.Equal(t, "default", idp.Spec.Clients[0].SecretRef.Namespace) + idp.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue}} + return nil + }).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + RunAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + idp := obj.(*secopv1alpha1.IdentityProviderConfiguration) + idp.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue}} + return nil + }).Once() + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:acme", + }, + }, + }, + expectedErr: false, + expectedResult: ctrl.Result{}, + }, + { + name: "CreateOrUpdate NotReady", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "beta"}, mock.AnythingOfType("*v1alpha1.Account")). + RunAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + acc := obj.(*accountv1alpha1.Account) + acc.Spec.Type = accountv1alpha1.AccountTypeOrg + return nil + }).Once() + cluster.EXPECT().GetClient().Return(orgsClient).Maybe() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "beta"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + Return(apierrors.NewNotFound(schema.GroupResource{Group: "core.platform-mesh.io", Resource: "identityproviderconfigurations"}, "beta")).Once() + orgsClient.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + RunAndReturn(func(_ context.Context, obj client.Object, _ ...client.CreateOption) error { + idp := obj.(*secopv1alpha1.IdentityProviderConfiguration) + assert.Len(t, idp.Spec.Clients, 1) + assert.Equal(t, "beta", idp.Spec.Clients[0].ClientName) + assert.Equal(t, "portal-client-secret-beta-beta", idp.Spec.Clients[0].SecretRef.Name) + assert.Equal(t, "default", idp.Spec.Clients[0].SecretRef.Namespace) + idp.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionFalse}} + return nil + }).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "beta"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + RunAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + idp := obj.(*secopv1alpha1.IdentityProviderConfiguration) + idp.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionFalse}} + return nil + }).Once() + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:beta", + }, + }, + }, + expectedErr: true, + expectedResult: ctrl.Result{}, + }, + { + name: "Get IDP resource error", + setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster, cfg config.Config) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "gamma"}, mock.AnythingOfType("*v1alpha1.Account")). + RunAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + acc := obj.(*accountv1alpha1.Account) + acc.Spec.Type = accountv1alpha1.AccountTypeOrg + return nil + }).Once() + cluster.EXPECT().GetClient().Return(orgsClient).Maybe() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "gamma"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + Return(apierrors.NewNotFound(schema.GroupResource{Group: "core.platform-mesh.io", Resource: "identityproviderconfigurations"}, "gamma")).Once() + orgsClient.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + Return(nil).Once() + orgsClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "gamma"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration")). + Return(assert.AnError).Once() + }, + lc: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:gamma", + }, + }, + }, + expectedErr: true, + expectedResult: ctrl.Result{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + orgsClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cluster := mocks.NewMockCluster(t) + cfg := config.Config{} + cfg.IDP.AdditionalRedirectURLs = []string{} + cfg.BaseDomain = "example.com" + subroutine := NewIDPSubroutine(orgsClient, mgr, cfg) + + tt.setupMocks(orgsClient, mgr, cluster, cfg) + + l := testlogger.New() + ctx := l.WithContext(context.Background()) + + result, opErr := subroutine.Process(ctx, tt.lc) + + if tt.expectedErr { + assert.NotNil(t, opErr) + } else { + assert.Nil(t, opErr) + } + assert.Equal(t, tt.expectedResult, result) + }) + } +} diff --git a/internal/subroutine/invite.go b/internal/subroutine/invite.go index 7202a0ae..2d544c5c 100644 --- a/internal/subroutine/invite.go +++ b/internal/subroutine/invite.go @@ -90,7 +90,7 @@ func (w *inviteSubroutine) Process(ctx context.Context, instance runtimeobject.R return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to create invite resource %w", err), true, true) } - log.Info().Str("workspace", wsName).Msg("invite resource created") + log.Info().Str("workspace", wsName).Msg("invite resource is created") err = wait.ExponentialBackoffWithContext(ctx, retry.DefaultBackoff, func(ctx context.Context) (bool, error) { @@ -105,6 +105,6 @@ func (w *inviteSubroutine) Process(ctx context.Context, instance runtimeobject.R return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("invite resource is not ready yet"), true, false) } - log.Info().Str("workspace", wsName).Msg("invite resource ready") + log.Info().Str("workspace", wsName).Msg("invite resource is ready") return ctrl.Result{}, nil } diff --git a/internal/subroutine/invite/subroutine.go b/internal/subroutine/invite/subroutine.go index 5af533e5..c61d21ec 100644 --- a/internal/subroutine/invite/subroutine.go +++ b/internal/subroutine/invite/subroutine.go @@ -9,7 +9,7 @@ import ( "net/url" "github.com/coreos/go-oidc" - corev1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" + accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" "github.com/platform-mesh/golang-commons/errors" @@ -108,7 +108,7 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime cl := cluster.GetClient() - var accountInfo corev1alpha1.AccountInfo + var accountInfo accountsv1alpha1.AccountInfo if err := cl.Get(ctx, client.ObjectKey{Name: "account"}, &accountInfo); err != nil { log.Err(err).Msg("Failed to get AccountInfo") return ctrl.Result{}, errors.NewOperatorError(err, true, false) @@ -142,8 +142,14 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime log.Info().Str("email", invite.Spec.Email).Msg("User does not exist, creating user and sending invite") + clientID, err := s.getClientID(ctx, cl, realm) + if err != nil { + log.Err(err).Msg("failed to get client ID") + return ctrl.Result{}, errors.NewOperatorError(err, true, false) + } + clientQueryParams := url.Values{ - "clientId": {realm}, + "clientId": {clientID}, } res, err = s.keycloak.Get(fmt.Sprintf("%s/admin/realms/%s/clients?%s", s.keycloakBaseURL, realm, clientQueryParams.Encode())) @@ -163,11 +169,11 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime } if len(clients) == 0 { - log.Info().Str("clientId", realm).Msg("Client does not exist yet, requeuing") + log.Info().Str("clientId", clientID).Msg("Client does not exist yet, requeuing") return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("client %s does not exist yet", realm), true, false) } - log.Debug().Str("clientId", realm).Msg("Client verified") + log.Debug().Str("clientId", clientID).Msg("Client verified") // Create user newUser := keycloakUser{ @@ -213,7 +219,7 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime queryParams := url.Values{ "redirect_uri": {fmt.Sprintf("https://%s.%s/", realm, s.baseDomain)}, - "client_id": {realm}, + "client_id": {clientID}, } req, err := http.NewRequestWithContext(ctx, http.MethodPut, fmt.Sprintf("%s/admin/realms/%s/users/%s/execute-actions-email?%s", s.keycloakBaseURL, realm, newUser.ID, queryParams.Encode()), http.NoBody) @@ -238,3 +244,16 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime } var _ lifecyclesubroutine.Subroutine = &subroutine{} + +func (s *subroutine) getClientID(ctx context.Context, cl client.Client, clientName string) (string, error) { + idp := &v1alpha1.IdentityProviderConfiguration{} + if err := cl.Get(ctx, client.ObjectKey{Name: clientName}, idp); err != nil { + return "", err + } + + client, ok := idp.Status.ManagedClients[clientName] + if !ok { + return "", fmt.Errorf("client %s not found in idp %s", clientName, idp.Name) + } + return client.ClientID, nil +} diff --git a/internal/subroutine/invite/subroutine_test.go b/internal/subroutine/invite/subroutine_test.go index 742f691a..6b44f0c5 100644 --- a/internal/subroutine/invite/subroutine_test.go +++ b/internal/subroutine/invite/subroutine_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "golang.org/x/oauth2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -67,8 +68,8 @@ func TestSubroutineProcess(t *testing.T) { }, }, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { accountInfo := &accountsv1alpha1.AccountInfo{ Spec: accountsv1alpha1.AccountInfoSpec{ Organization: accountsv1alpha1.AccountLocation{ @@ -78,8 +79,25 @@ func TestSubroutineProcess(t *testing.T) { } *o.(*accountsv1alpha1.AccountInfo) = *accountInfo return nil - }, - ) + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + idp := &v1alpha1.IdentityProviderConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "acme", + }, + Status: v1alpha1.IdentityProviderConfigurationStatus{ + ManagedClients: map[string]v1alpha1.ManagedClient{ + "acme": { + ClientID: "acme", + }, + }, + }, + } + *o.(*v1alpha1.IdentityProviderConfiguration) = *idp + return nil + }).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { users := []map[string]any{} @@ -122,8 +140,8 @@ func TestSubroutineProcess(t *testing.T) { }, expectErr: true, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { accountInfo := &accountsv1alpha1.AccountInfo{ Spec: accountsv1alpha1.AccountInfoSpec{ Organization: accountsv1alpha1.AccountLocation{ @@ -133,8 +151,7 @@ func TestSubroutineProcess(t *testing.T) { } *o.(*accountsv1alpha1.AccountInfo) = *accountInfo return nil - }, - ) + }).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { mux.HandleFunc("GET /admin/realms/acme/users", func(w http.ResponseWriter, r *http.Request) { @@ -152,8 +169,8 @@ func TestSubroutineProcess(t *testing.T) { }, expectErr: true, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { accountInfo := &accountsv1alpha1.AccountInfo{ Spec: accountsv1alpha1.AccountInfoSpec{ Organization: accountsv1alpha1.AccountLocation{ @@ -163,8 +180,25 @@ func TestSubroutineProcess(t *testing.T) { } *o.(*accountsv1alpha1.AccountInfo) = *accountInfo return nil - }, - ) + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + idp := &v1alpha1.IdentityProviderConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "acme", + }, + Status: v1alpha1.IdentityProviderConfigurationStatus{ + ManagedClients: map[string]v1alpha1.ManagedClient{ + "acme": { + ClientID: "acme", + }, + }, + }, + } + *o.(*v1alpha1.IdentityProviderConfiguration) = *idp + return nil + }).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { first := true @@ -195,7 +229,8 @@ func TestSubroutineProcess(t *testing.T) { }, expectErr: true, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("accountinfo not found")) + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + Return(fmt.Errorf("accountinfo not found")).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { }, @@ -209,8 +244,8 @@ func TestSubroutineProcess(t *testing.T) { }, expectErr: true, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { accountInfo := &accountsv1alpha1.AccountInfo{ Spec: accountsv1alpha1.AccountInfoSpec{ Organization: accountsv1alpha1.AccountLocation{ @@ -220,8 +255,25 @@ func TestSubroutineProcess(t *testing.T) { } *o.(*accountsv1alpha1.AccountInfo) = *accountInfo return nil - }, - ) + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + idp := &v1alpha1.IdentityProviderConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "acme", + }, + Status: v1alpha1.IdentityProviderConfigurationStatus{ + ManagedClients: map[string]v1alpha1.ManagedClient{ + "acme": { + ClientID: "acme", + }, + }, + }, + } + *o.(*v1alpha1.IdentityProviderConfiguration) = *idp + return nil + }).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { users := []map[string]any{} @@ -255,8 +307,8 @@ func TestSubroutineProcess(t *testing.T) { }, expectErr: true, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { accountInfo := &accountsv1alpha1.AccountInfo{ Spec: accountsv1alpha1.AccountInfoSpec{ Organization: accountsv1alpha1.AccountLocation{ @@ -266,8 +318,25 @@ func TestSubroutineProcess(t *testing.T) { } *o.(*accountsv1alpha1.AccountInfo) = *accountInfo return nil - }, - ) + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "acme"}, mock.AnythingOfType("*v1alpha1.IdentityProviderConfiguration"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + idp := &v1alpha1.IdentityProviderConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "acme", + }, + Status: v1alpha1.IdentityProviderConfigurationStatus{ + ManagedClients: map[string]v1alpha1.ManagedClient{ + "acme": { + ClientID: "acme", + }, + }, + }, + } + *o.(*v1alpha1.IdentityProviderConfiguration) = *idp + return nil + }).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { users := []map[string]any{} @@ -304,8 +373,8 @@ func TestSubroutineProcess(t *testing.T) { }, expectErr: true, setupK8sMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.AnythingOfType("*v1alpha1.AccountInfo"), mock.Anything). + RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { accountInfo := &accountsv1alpha1.AccountInfo{ Spec: accountsv1alpha1.AccountInfoSpec{ Organization: accountsv1alpha1.AccountLocation{ @@ -315,8 +384,7 @@ func TestSubroutineProcess(t *testing.T) { } *o.(*accountsv1alpha1.AccountInfo) = *accountInfo return nil - }, - ) + }).Once() }, setupKeycloakMocks: func(mux *http.ServeMux) { }, diff --git a/internal/subroutine/realm.go b/internal/subroutine/realm.go deleted file mode 100644 index 6a32aab7..00000000 --- a/internal/subroutine/realm.go +++ /dev/null @@ -1,221 +0,0 @@ -package subroutine - -import ( - "bytes" - "context" - _ "embed" - "encoding/json" - "fmt" - "strings" - "text/template" - - kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" - "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" - lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" - "github.com/platform-mesh/golang-commons/errors" - "github.com/platform-mesh/golang-commons/logger" - "github.com/platform-mesh/security-operator/internal/config" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/yaml" -) - -var ( - - //go:embed manifests/organizationIdp/helmrelease.yaml - helmRelease string -) - -type realmSubroutine struct { - k8s client.Client - baseDomain string - cfg *config.Config -} - -func NewRealmSubroutine(k8s client.Client, cfg *config.Config, baseDomain string) *realmSubroutine { - return &realmSubroutine{ - k8s, - baseDomain, - cfg, - } -} - -var _ lifecyclesubroutine.Subroutine = &realmSubroutine{} - -func (r *realmSubroutine) GetName() string { return "Realm" } - -func (r *realmSubroutine) Finalizers(_ runtimeobject.RuntimeObject) []string { - return []string{} -} - -func (r *realmSubroutine) Finalize(ctx context.Context, instance runtimeobject.RuntimeObject) (reconcile.Result, errors.OperatorError) { - log := logger.LoadLoggerFromContext(ctx) - - lc := instance.(*kcpv1alpha1.LogicalCluster) - workspaceName := getWorkspaceName(lc) - if workspaceName == "" { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get workspace path"), true, false) - } - - helmObj, err := unstructuredFromString(helmRelease, nil, log) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to load HelmRelease manifest: %w", err), true, true) - } - helmObj.SetName(workspaceName) - if err := r.k8s.Delete(ctx, &helmObj); err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to delete HelmRelease: %w", err), true, true) - } - - log.Info().Str("realm", workspaceName).Msg("Successfully finalized resources") - return ctrl.Result{}, nil -} - -func (r *realmSubroutine) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (reconcile.Result, errors.OperatorError) { - lc := instance.(*kcpv1alpha1.LogicalCluster) - - workspaceName := getWorkspaceName(lc) - if workspaceName == "" { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get workspace path"), true, false) - } - - patch := map[string]any{ - "crossplane": map[string]any{ - "realm": map[string]any{ - "name": workspaceName, - "displayName": workspaceName, - }, - "client": map[string]any{ - "name": workspaceName, - "displayName": workspaceName, - "validRedirectUris": append(r.cfg.IDP.AdditionalRedirectURLs, fmt.Sprintf("https://%s.%s/*", workspaceName, r.baseDomain)), - }, - "organization": map[string]any{ - "domain": "example.com", // TODO: change - }, - }, - "keycloakConfig": map[string]any{ - "client": map[string]any{ - "name": workspaceName, - "targetSecret": map[string]any{ - "name": fmt.Sprintf("portal-client-secret-%s", workspaceName), - }, - }, - }, - } - - if r.cfg.IDP.SMTPServer != "" { - - smtpConfig := map[string]any{ - "host": r.cfg.IDP.SMTPServer, - "port": fmt.Sprintf("%d", r.cfg.IDP.SMTPPort), - "from": r.cfg.IDP.FromAddress, - "ssl": r.cfg.IDP.SSL, - "starttls": r.cfg.IDP.StartTLS, - } - - if r.cfg.IDP.SMTPUser != "" { - smtpConfig["auth"] = map[string]any{ - "username": r.cfg.IDP.SMTPUser, - "passwordSecretRef": map[string]any{ - "namespace": "platform-mesh-system", - "name": r.cfg.IDP.SMTPPasswordSecretName, - "key": r.cfg.IDP.SMTPPasswordSecretKey, - }, - } - } - - err := unstructured.SetNestedField(patch, []any{smtpConfig}, "crossplane", "realm", "smtpConfig") - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to set SMTP server config: %w", err), true, true) - } - } - - marshalledPatch, err := json.Marshal(patch) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to marshall patch map: %w", err), true, true) - } - - values := apiextensionsv1.JSON{Raw: marshalledPatch} - releaseName := fmt.Sprintf("%s-idp", workspaceName) - - err = applyReleaseWithValues(ctx, helmRelease, r.k8s, values, releaseName) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to create HelmRelease: %w", err), true, true) - } - - return ctrl.Result{}, nil -} - -func getWorkspaceName(lc *kcpv1alpha1.LogicalCluster) string { - if path, ok := lc.Annotations["kcp.io/path"]; ok { - pathElements := strings.Split(path, ":") - return pathElements[len(pathElements)-1] - } - return "" -} - -func applyReleaseWithValues(ctx context.Context, release string, k8sClient client.Client, values apiextensionsv1.JSON, releaseName string) error { - log := logger.LoadLoggerFromContext(ctx) - - obj, err := unstructuredFromString(release, map[string]string{}, log) - if err != nil { - return errors.Wrap(err, "Failed to get unstructuredFromFile") - } - obj.SetName(releaseName) - - if err := unstructured.SetNestedField(obj.Object, releaseName, "spec", "releaseName"); err != nil { - return errors.Wrap(err, "failed to set spec.releaseName") - } - - obj.Object["spec"].(map[string]interface{})["values"] = values - - err = k8sClient.Patch(ctx, &obj, client.Apply, client.FieldOwner("security-operator")) - if err != nil { - return errors.Wrap(err, "Failed to apply manifest: (%s/%s)", obj.GetKind(), obj.GetName()) - } - return nil -} - -func unstructuredFromString(manifest string, templateData map[string]string, log *logger.Logger) (unstructured.Unstructured, error) { - manifestBytes := []byte(manifest) - - res, err := ReplaceTemplate(templateData, manifestBytes) - if err != nil { - return unstructured.Unstructured{}, errors.Wrap(err, "Failed to replace template") - } - - var objMap map[string]interface{} - if err := yaml.Unmarshal(res, &objMap); err != nil { - return unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from template. Output:\n%s", string(res)) - } - - log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object") - - obj := unstructured.Unstructured{Object: objMap} - - log.Debug().Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest") - return obj, err -} - -func ReplaceTemplate(templateData map[string]string, templateBytes []byte) ([]byte, error) { - tmpl, err := template.New("manifest").Parse(string(templateBytes)) - if err != nil { - return []byte{}, errors.Wrap(err, "Failed to parse template") - } - var result bytes.Buffer - err = tmpl.Execute(&result, templateData) - if err != nil { - keys := make([]string, 0, len(templateData)) - for k := range templateData { - keys = append(keys, k) - } - return []byte{}, errors.Wrap(err, "Failed to execute template with keys %v", keys) - } - if result.Len() == 0 { - return []byte{}, nil - } - return result.Bytes(), nil -} diff --git a/internal/subroutine/realm_test.go b/internal/subroutine/realm_test.go deleted file mode 100644 index c6595667..00000000 --- a/internal/subroutine/realm_test.go +++ /dev/null @@ -1,303 +0,0 @@ -package subroutine - -import ( - "context" - "strings" - "testing" - - kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" - "github.com/platform-mesh/golang-commons/errors" - "github.com/platform-mesh/golang-commons/logger" - "github.com/platform-mesh/security-operator/internal/config" - "github.com/platform-mesh/security-operator/internal/subroutine/mocks" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const ( - helmReleaseYAML = ` -apiVersion: helm.toolkit.fluxcd.io/v2beta1 -kind: HelmRelease -metadata: - name: helm-min -spec: - releaseName: placeholder - values: {} -` - baseDomain = "portal.dev.local" -) - -func newClientMock(t *testing.T, setup func(m *mocks.MockClient)) *mocks.MockClient { - t.Helper() - m := new(mocks.MockClient) - if setup != nil { - setup(m) - } - t.Cleanup(func() { m.AssertExpectations(t) }) - return m -} - -func testLogger() *logger.Logger { - l, _ := logger.New(logger.DefaultConfig()) - return l -} - -func trim(s string) string { return strings.TrimSpace(s) } - -func TestApplyReleaseAndManifest(t *testing.T) { - cases := []struct { - name string - call string // "release" or "manifest" - content string - setupMocks func(m *mocks.MockClient) - expectErr bool - }{ - {"release: invalid YAML", "release", "not: : valid: yaml", nil, true}, - { - "release: spec scalar", - "release", - trim(` -apiVersion: helm.toolkit.fluxcd.io/v2beta1 -kind: HelmRelease -metadata: - name: test-release - namespace: default -spec: "test spec" -`), - nil, - true, - }, - { - "release: patch error wrapped", - "release", - trim(` -apiVersion: helm.toolkit.fluxcd.io/v2beta1 -kind: HelmRelease -metadata: - name: test-release - namespace: default -spec: - chart: - spec: - chart: mychart - version: 1.2.3 -`), - func(m *mocks.MockClient) { - m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(errors.New("simulated patch fail")).Once() - }, - true, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - clientMock := newClientMock(t, tc.setupMocks) - ctx := context.Background() - - switch tc.call { - case "release": - err := applyReleaseWithValues(ctx, tc.content, clientMock, apiextensionsv1.JSON{}, "org-name") - if tc.expectErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - default: - t.Fatalf("unknown call type %q", tc.call) - } - }) - } -} - -func TestRealmSubroutine_ProcessAndFinalize(t *testing.T) { - origHR := helmRelease - defer func() { helmRelease = origHR }() - - t.Run("Process", func(t *testing.T) { - t.Run("success create repo then helmrelease", func(t *testing.T) { - t.Parallel() - clientMock := newClientMock(t, func(m *mocks.MockClient) { - m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything). - RunAndReturn(func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { - hr := obj.(*unstructured.Unstructured) - spec, _, _ := unstructured.NestedFieldNoCopy(hr.Object, "spec") - specMap := spec.(map[string]interface{}) - specValues, _, _ := unstructured.NestedFieldNoCopy(specMap, "values") - _, ok := specValues.(apiextensionsv1.JSON) - require.True(t, ok) - return nil - }).Once() - }) - - helmRelease = trim(helmReleaseYAML) - - rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain) - lc := &kcpv1alpha1.LogicalCluster{} - lc.Annotations = map[string]string{"kcp.io/path": "root:orgs:test"} - res, opErr := rs.Process(context.Background(), lc) - require.Nil(t, opErr) - require.Equal(t, ctrl.Result{}, res) - }) - - // New: success create with SMTP config - t.Run("success create with SMTP config", func(t *testing.T) { - t.Parallel() - clientMock := newClientMock(t, func(m *mocks.MockClient) { - m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() - }) - - cfg := &config.Config{} - cfg.IDP.SMTPServer = "smtp.example.com" - cfg.IDP.SMTPPort = 587 - cfg.IDP.FromAddress = "noreply@example.com" - cfg.IDP.SSL = false - cfg.IDP.StartTLS = true - rs := NewRealmSubroutine(clientMock, cfg, baseDomain) - lc := &kcpv1alpha1.LogicalCluster{} - lc.Annotations = map[string]string{"kcp.io/path": "root:orgs:test"} - res, opErr := rs.Process(context.Background(), lc) - require.Nil(t, opErr) - require.Equal(t, ctrl.Result{}, res) - }) - - t.Run("helmrelease apply fails", func(t *testing.T) { - t.Parallel() - clientMock := newClientMock(t, func(m *mocks.MockClient) { - m.EXPECT().Patch(mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(errors.New("simulated patch failure for HelmRelease")).Once() - }) - - helmRelease = trim(helmReleaseYAML) - rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain) - lc := &kcpv1alpha1.LogicalCluster{} - lc.Annotations = map[string]string{"kcp.io/path": "root:orgs:test"} - res, opErr := rs.Process(context.Background(), lc) - require.NotNil(t, opErr) - require.Equal(t, ctrl.Result{}, res) - }) - - // New: Finalize missing workspace annotation - t.Run("missing workspace annotation in Finalize", func(t *testing.T) { - clientMock := newClientMock(t, nil) - rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain) - lc := &kcpv1alpha1.LogicalCluster{} - res, opErr := rs.Finalize(context.Background(), lc) - require.NotNil(t, opErr) - require.Equal(t, ctrl.Result{}, res) - }) - - t.Run("missing workspace annotation", func(t *testing.T) { - t.Parallel() - clientMock := newClientMock(t, nil) - rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain) - lc := &kcpv1alpha1.LogicalCluster{} - res, opErr := rs.Process(context.Background(), lc) - require.NotNil(t, opErr) - require.Equal(t, ctrl.Result{}, res) - }) - }) - - t.Run("Finalize - delete scenarios", func(t *testing.T) { - t.Parallel() - cases := []struct { - name string - setupMocks func(m *mocks.MockClient) - expectErr bool - expectedResult ctrl.Result - }{ - { - "HelmRelease delete error", - func(m *mocks.MockClient) { - m.EXPECT().Delete(mock.Anything, mock.Anything).Return(errors.New("failed to delete helmRelease")).Once() - }, - true, - ctrl.Result{}, - }, - { - "Delete succeeds", - func(m *mocks.MockClient) { m.EXPECT().Delete(mock.Anything, mock.Anything).Return(nil).Once() }, - false, - ctrl.Result{}, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - clientMock := newClientMock(t, tc.setupMocks) - rs := NewRealmSubroutine(clientMock, &config.Config{}, baseDomain) - lc := &kcpv1alpha1.LogicalCluster{} - lc.Annotations = map[string]string{"kcp.io/path": "root:orgs:test"} - res, opErr := rs.Finalize(context.Background(), lc) - if tc.expectErr { - require.NotNil(t, opErr) - } else { - require.Nil(t, opErr) - } - require.Equal(t, tc.expectedResult, res) - }) - } - }) - -} - -func TestReplaceTemplateAndUnstructured(t *testing.T) { - cases := []struct { - name string - templateData map[string]string - template []byte - expectErr bool - expectOutput string - }{ - {"parse error invalid template", nil, []byte("{{"), true, ""}, - {"empty template yields empty result", map[string]string{}, []byte(""), false, ""}, - {"successful template rendering", map[string]string{"Name": "testing"}, []byte("hello {{ .Name }}"), false, "hello testing"}, - {"execute error indexing missing map", map[string]string{}, []byte(`{{ index .MissingMap "k" }}`), true, ""}, - {"nil template data with static content", nil, []byte("static content"), false, "static content"}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - out, err := ReplaceTemplate(tc.templateData, tc.template) - if tc.expectErr { - require.Error(t, err) - return - } - require.NoError(t, err) - if tc.expectOutput != "" { - require.Equal(t, tc.expectOutput, string(out)) - } - }) - } - - t.Run("unstructured invalid yaml", func(t *testing.T) { - l := testLogger() - _, err := unstructuredFromString("not: : valid: yaml", nil, l) - require.Error(t, err) - }) - - t.Run("unstructured template success", func(t *testing.T) { - l := testLogger() - templ := "kind: Test\nmetadata:\n name: {{ .Name }}\nspec:\n v: 1" - out, err := ReplaceTemplate(map[string]string{"Name": "templated"}, []byte(templ)) - require.NoError(t, err) - _, err2 := unstructuredFromString(string(out), nil, l) - require.NoError(t, err2) - }) -} - -func TestRealmSubroutine_GetName(t *testing.T) { - r := NewRealmSubroutine(nil, &config.Config{}, "") - require.Equal(t, "Realm", r.GetName()) -} - -func TestRealmSubroutine_Finalizers(t *testing.T) { - r := NewRealmSubroutine(nil, &config.Config{}, "") - require.Equal(t, []string{}, r.Finalizers(nil)) -} diff --git a/internal/subroutine/remove_initializer.go b/internal/subroutine/remove_initializer.go index 1810a193..4ae1b837 100644 --- a/internal/subroutine/remove_initializer.go +++ b/internal/subroutine/remove_initializer.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "slices" - "time" "github.com/kcp-dev/kcp/sdk/apis/cache/initialization" kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" @@ -13,23 +12,14 @@ import ( "github.com/platform-mesh/golang-commons/errors" "github.com/platform-mesh/security-operator/internal/config" "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 - secretWaitTimeout time.Duration + initializerName string + mgr mcmanager.Manager } // Finalize implements subroutine.Subroutine. @@ -59,28 +49,6 @@ func (r *removeInitializer) Process(ctx context.Context, instance runtimeobject. return ctrl.Result{}, nil } - // we need to wait until 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) { - age := time.Since(lc.CreationTimestamp.Time) - if age <= r.secretWaitTimeout { - log.Info().Str("secret", secretName).Msg("portal secret not ready yet, requeueing") - return ctrl.Result{RequeueAfter: 5 * time.Second}, nil - } - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("portal client secret %s was not created within %s", secretName, r.secretWaitTimeout.String()), true, true) - } - 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) @@ -93,12 +61,10 @@ func (r *removeInitializer) Process(ctx context.Context, instance runtimeobject. return ctrl.Result{}, nil } -func NewRemoveInitializer(mgr mcmanager.Manager, cfg config.Config, runtimeClient client.Client) *removeInitializer { +func NewRemoveInitializer(mgr mcmanager.Manager, cfg config.Config) *removeInitializer { return &removeInitializer{ - initializerName: cfg.InitializerName, - mgr: mgr, - runtimeClient: runtimeClient, - secretWaitTimeout: time.Duration(cfg.SecretWaitingTimeoutInSeconds) * time.Second, + initializerName: cfg.InitializerName, + mgr: mgr, } } diff --git a/internal/subroutine/remove_initializer_test.go b/internal/subroutine/remove_initializer_test.go index 8bdca849..b525bfdc 100644 --- a/internal/subroutine/remove_initializer_test.go +++ b/internal/subroutine/remove_initializer_test.go @@ -3,7 +3,6 @@ package subroutine_test import ( "context" "testing" - "time" kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" "github.com/platform-mesh/security-operator/internal/config" @@ -11,9 +10,6 @@ import ( "github.com/platform-mesh/security-operator/internal/subroutine/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -48,14 +44,13 @@ 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, config.Config{InitializerName: initializerName, SecretWaitingTimeoutInSeconds: 60}, runtimeClient) + r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: initializerName}) _, err := r.Process(context.Background(), lc) assert.Nil(t, err) }) @@ -63,12 +58,9 @@ func TestRemoveInitializer_Process(t *testing.T) { 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: subroutine.PortalClientSecretNamespace}, mock.Anything).Return(nil) cluster.EXPECT().GetClient().Return(k8s) k8s.EXPECT().Status().Return(&fakeStatusWriter{t: t, expectClear: kcpv1alpha1.LogicalClusterInitializer(initializerName), err: nil}) @@ -77,12 +69,10 @@ 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, config.Config{InitializerName: initializerName, SecretWaitingTimeoutInSeconds: 60}, runtimeClient) + r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: initializerName}) _, err := r.Process(context.Background(), lc) assert.Nil(t, err) - // ensure it's removed in in-memory object as well for _, init := range lc.Status.Initializers { assert.NotEqual(t, initializerName, string(init)) } @@ -91,12 +81,9 @@ 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: subroutine.PortalClientSecretNamespace}, mock.Anything).Return(nil) cluster.EXPECT().GetClient().Return(k8s) k8s.EXPECT().Status().Return(&fakeStatusWriter{t: t, expectClear: kcpv1alpha1.LogicalClusterInitializer(initializerName), err: assert.AnError}) @@ -104,52 +91,8 @@ func TestRemoveInitializer_Process(t *testing.T) { lc.Status.Initializers = []kcpv1alpha1.LogicalClusterInitializer{ kcpv1alpha1.LogicalClusterInitializer(initializerName), } - lc.Annotations = map[string]string{"kcp.io/path": "root:org:test"} - r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: initializerName, SecretWaitingTimeoutInSeconds: 60}, runtimeClient) - _, err := r.Process(context.Background(), lc) - assert.NotNil(t, err) - }) - - t.Run("requeues when secret not found under 1 minute", func(t *testing.T) { - mgr := mocks.NewMockManager(t) - cluster := mocks.NewMockCluster(t) - runtimeClient := mocks.NewMockClient(t) - - mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil) - // Simulate NotFound error - runtimeClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "portal-client-secret-test", Namespace: subroutine.PortalClientSecretNamespace}, mock.Anything).Return(apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "portal-client-secret-test")) - - lc := &kcpv1alpha1.LogicalCluster{} - lc.Status.Initializers = []kcpv1alpha1.LogicalClusterInitializer{ - kcpv1alpha1.LogicalClusterInitializer(initializerName), - } - lc.Annotations = map[string]string{"kcp.io/path": "root:org:test"} - lc.CreationTimestamp.Time = time.Now().Add(-30 * time.Second) - - r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: initializerName, SecretWaitingTimeoutInSeconds: 60}, runtimeClient) - res, err := r.Process(context.Background(), lc) - assert.Nil(t, err) - assert.Equal(t, 5*time.Second, res.RequeueAfter) - }) - - t.Run("errors when secret not found after 1 minute", func(t *testing.T) { - mgr := mocks.NewMockManager(t) - cluster := mocks.NewMockCluster(t) - runtimeClient := mocks.NewMockClient(t) - - mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil) - // Simulate NotFound error - runtimeClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "portal-client-secret-test", Namespace: subroutine.PortalClientSecretNamespace}, mock.Anything).Return(apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "portal-client-secret-test")) - - lc := &kcpv1alpha1.LogicalCluster{} - lc.Status.Initializers = []kcpv1alpha1.LogicalClusterInitializer{ - kcpv1alpha1.LogicalClusterInitializer(initializerName), - } - lc.Annotations = map[string]string{"kcp.io/path": "root:org:test"} - lc.CreationTimestamp.Time = time.Now().Add(-2 * time.Minute) - - r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: initializerName, SecretWaitingTimeoutInSeconds: 60}, runtimeClient) + r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: initializerName}) _, err := r.Process(context.Background(), lc) assert.NotNil(t, err) }) @@ -157,8 +100,7 @@ func TestRemoveInitializer_Process(t *testing.T) { func TestRemoveInitializer_Misc(t *testing.T) { mgr := mocks.NewMockManager(t) - runtimeClient := mocks.NewMockClient(t) - r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: "foo.initializer.kcp.dev", SecretWaitingTimeoutInSeconds: 60}, runtimeClient) + r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: "foo.initializer.kcp.dev"}) assert.Equal(t, "RemoveInitializer", r.GetName()) assert.Equal(t, []string{}, r.Finalizers(nil)) @@ -169,11 +111,9 @@ func TestRemoveInitializer_Misc(t *testing.T) { 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, config.Config{InitializerName: "foo.initializer.kcp.dev", SecretWaitingTimeoutInSeconds: 60}, runtimeClient) + r := subroutine.NewRemoveInitializer(mgr, config.Config{InitializerName: "foo.initializer.kcp.dev"}) _, err := r.Process(context.Background(), &kcpv1alpha1.LogicalCluster{}) assert.NotNil(t, err) } diff --git a/internal/subroutine/worksapce_authorization.go b/internal/subroutine/workspace_authorization.go similarity index 64% rename from internal/subroutine/worksapce_authorization.go rename to internal/subroutine/workspace_authorization.go index 8194da7b..738169f2 100644 --- a/internal/subroutine/worksapce_authorization.go +++ b/internal/subroutine/workspace_authorization.go @@ -10,7 +10,9 @@ import ( lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" "github.com/platform-mesh/golang-commons/errors" "github.com/platform-mesh/security-operator/internal/config" + "github.com/rs/zerolog/log" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -20,14 +22,14 @@ import ( ) type workspaceAuthSubroutine struct { - client client.Client + orgClient client.Client runtimeClient client.Client cfg config.Config } -func NewWorkspaceAuthConfigurationSubroutine(client, runtimeClient client.Client, cfg config.Config) *workspaceAuthSubroutine { +func NewWorkspaceAuthConfigurationSubroutine(orgClient, runtimeClient client.Client, cfg config.Config) *workspaceAuthSubroutine { return &workspaceAuthSubroutine{ - client: client, + orgClient: orgClient, runtimeClient: runtimeClient, cfg: cfg, } @@ -62,7 +64,7 @@ func (r *workspaceAuthSubroutine) Process(ctx context.Context, instance runtimeo } obj := &kcptenancyv1alphav1.WorkspaceAuthenticationConfiguration{ObjectMeta: metav1.ObjectMeta{Name: workspaceName}} - _, err := controllerutil.CreateOrUpdate(ctx, r.client, obj, func() error { + _, err := controllerutil.CreateOrUpdate(ctx, r.orgClient, obj, func() error { obj.Spec = kcptenancyv1alphav1.WorkspaceAuthenticationConfigurationSpec{ JWT: []kcptenancyv1alphav1.JWTAuthenticator{ { @@ -95,5 +97,42 @@ func (r *workspaceAuthSubroutine) Process(ctx context.Context, instance runtimeo return reconcile.Result{}, errors.NewOperatorError(fmt.Errorf("failed to create WorkspaceAuthConfiguration resource: %w", err), true, true) } + err = r.patchWorkspaceType(ctx, r.orgClient, fmt.Sprintf("%s-org", workspaceName), workspaceName) + if err != nil { + return reconcile.Result{}, errors.NewOperatorError(fmt.Errorf("failed to patch workspace type: %w", err), true, true) + } + + err = r.patchWorkspaceType(ctx, r.orgClient, fmt.Sprintf("%s-acc", workspaceName), workspaceName) + if err != nil { + return reconcile.Result{}, errors.NewOperatorError(fmt.Errorf("failed to patch workspace type: %w", err), true, true) + } + return ctrl.Result{}, nil } + +func (r *workspaceAuthSubroutine) patchWorkspaceType(ctx context.Context, cl client.Client, workspaceTypeName, authConfigurationRefName string) error { + wsType := &kcptenancyv1alphav1.WorkspaceType{ + ObjectMeta: metav1.ObjectMeta{Name: workspaceTypeName}, + } + + if err := cl.Get(ctx, client.ObjectKey{Name: workspaceTypeName}, wsType); err != nil { + return fmt.Errorf("failed to get WorkspaceType: %w", err) + } + + desiredAuthConfig := []kcptenancyv1alphav1.AuthenticationConfigurationReference{ + {Name: authConfigurationRefName}, + } + + if equality.Semantic.DeepEqual(wsType.Spec.AuthenticationConfigurations, desiredAuthConfig) { + log.Debug().Msg(fmt.Sprintf("workspaceType %s already has authentication configuration, skip patching", workspaceTypeName)) + return nil + } + + original := wsType.DeepCopy() + wsType.Spec.AuthenticationConfigurations = desiredAuthConfig + + if err := cl.Patch(ctx, wsType, client.MergeFrom(original)); err != nil { + return fmt.Errorf("failed to patch WorkspaceType: %w", err) + } + return nil +} diff --git a/internal/subroutine/workspace_authorization_test.go b/internal/subroutine/workspace_authorization_test.go index b32a6640..1d548759 100644 --- a/internal/subroutine/workspace_authorization_test.go +++ b/internal/subroutine/workspace_authorization_test.go @@ -42,7 +42,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { setupMocks: func(m *mocks.MockClient) { m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace"}, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). Return(apierrors.NewNotFound(kcptenancyv1alphav1.Resource("workspaceauthenticationconfigurations"), "test-workspace")).Once() - m.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). RunAndReturn(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { wac := obj.(*kcptenancyv1alphav1.WorkspaceAuthenticationConfiguration) @@ -53,6 +52,34 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { assert.Equal(t, "email", wac.Spec.JWT[0].ClaimMappings.Username.Claim) return nil }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace-org"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "test-workspace-org" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "test-workspace-org", wt.Name) + assert.Equal(t, "test-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace-acc"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "test-workspace-acc" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "test-workspace-acc", wt.Name) + assert.Equal(t, "test-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() }, expectError: false, expectedResult: ctrl.Result{}, @@ -83,7 +110,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { } return nil }).Once() - m.EXPECT().Update(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). RunAndReturn(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { wac := obj.(*kcptenancyv1alphav1.WorkspaceAuthenticationConfiguration) @@ -91,6 +117,34 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { assert.Equal(t, "https://example.com/keycloak/realms/existing-workspace", wac.Spec.JWT[0].Issuer.URL) return nil }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "existing-workspace-org"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "existing-workspace-org" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "existing-workspace-org", wt.Name) + assert.Equal(t, "existing-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "existing-workspace-acc"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "existing-workspace-acc" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "existing-workspace-acc", wt.Name) + assert.Equal(t, "existing-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() }, expectError: false, expectedResult: ctrl.Result{}, @@ -134,7 +188,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { setupMocks: func(m *mocks.MockClient) { m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace"}, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). Return(apierrors.NewNotFound(kcptenancyv1alphav1.Resource("workspaceauthenticationconfigurations"), "test-workspace")).Once() - m.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). Return(errors.New("create failed")).Once() }, @@ -158,7 +211,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { wac.Name = "test-workspace" return nil }).Once() - m.EXPECT().Update(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). Return(errors.New("update failed")).Once() }, @@ -195,7 +247,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { setupMocks: func(m *mocks.MockClient) { m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "single-workspace"}, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). Return(apierrors.NewNotFound(kcptenancyv1alphav1.Resource("workspaceauthenticationconfigurations"), "single-workspace")).Once() - m.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). RunAndReturn(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { wac := obj.(*kcptenancyv1alphav1.WorkspaceAuthenticationConfiguration) @@ -203,6 +254,34 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { assert.Equal(t, "https://test.domain/keycloak/realms/single-workspace", wac.Spec.JWT[0].Issuer.URL) return nil }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "single-workspace-org"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "single-workspace-org" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "single-workspace-org", wt.Name) + assert.Equal(t, "single-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "single-workspace-acc"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "single-workspace-acc" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "single-workspace-acc", wt.Name) + assert.Equal(t, "single-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() }, expectError: false, expectedResult: ctrl.Result{}, @@ -223,7 +302,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { DomainCALookup: true, }, setupMocks: func(m *mocks.MockClient) { - m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "domain-certificate-ca", Namespace: "platform-mesh-system"}, mock.Anything, mock.Anything). RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { secret := obj.(*corev1.Secret) @@ -235,7 +313,6 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "single-workspace"}, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). Return(apierrors.NewNotFound(kcptenancyv1alphav1.Resource("workspaceauthenticationconfigurations"), "single-workspace")).Once() - m.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). RunAndReturn(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { wac := obj.(*kcptenancyv1alphav1.WorkspaceAuthenticationConfiguration) @@ -243,10 +320,122 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { assert.Equal(t, "https://test.domain/keycloak/realms/single-workspace", wac.Spec.JWT[0].Issuer.URL) return nil }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "single-workspace-org"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "single-workspace-org" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "single-workspace-org", wt.Name) + assert.Equal(t, "single-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "single-workspace-acc"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "single-workspace-acc" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + assert.Equal(t, "single-workspace-acc", wt.Name) + assert.Equal(t, "single-workspace", wt.Spec.AuthenticationConfigurations[0].Name) + return nil + }).Once() }, expectError: false, expectedResult: ctrl.Result{}, }, + { + name: "error - patchWorkspaceType fails for -org", + logicalCluster: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test-workspace", + }, + }, + }, + cfg: config.Config{BaseDomain: "test.domain", GroupClaim: "groups", UserClaim: "email"}, + setupMocks: func(m *mocks.MockClient) { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace"}, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). + Return(apierrors.NewNotFound(kcptenancyv1alphav1.Resource("workspaceauthenticationconfigurations"), "test-workspace")).Once() + m.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything).Return(nil).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace-org"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "test-workspace-org" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + Return(errors.New("failed to patch workspace type")).Once() + }, + expectError: true, + expectedResult: ctrl.Result{}, + }, + { + name: "error - patchWorkspaceType fails for -acc", + logicalCluster: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test-workspace", + }, + }, + }, + cfg: config.Config{BaseDomain: "test.domain", GroupClaim: "groups", UserClaim: "email"}, + setupMocks: func(m *mocks.MockClient) { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace"}, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything). + Return(apierrors.NewNotFound(kcptenancyv1alphav1.Resource("workspaceauthenticationconfigurations"), "test-workspace")).Once() + m.EXPECT().Create(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceAuthenticationConfiguration"), mock.Anything).Return(nil).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace-org"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "test-workspace-org" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything).Return(nil).Once() + + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "test-workspace-acc"}, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + RunAndReturn(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + wt := obj.(*kcptenancyv1alphav1.WorkspaceType) + wt.Name = "test-workspace-acc" + return nil + }).Once() + m.EXPECT().Patch(mock.Anything, mock.AnythingOfType("*v1alpha1.WorkspaceType"), mock.Anything). + Return(errors.New("failed to patch workspace type")).Once() + }, + expectError: true, + expectedResult: ctrl.Result{}, + }, + { + name: "error - domain CA secret Get fails", + logicalCluster: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test-workspace", + }, + }, + }, + cfg: config.Config{ + BaseDomain: "test.domain", + GroupClaim: "groups", + UserClaim: "email", + DomainCALookup: true, + }, + setupMocks: func(m *mocks.MockClient) { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "domain-certificate-ca", Namespace: "platform-mesh-system"}, mock.Anything, mock.Anything). + Return(errors.New("failed to get domain CA secret")).Once() + }, + expectError: true, + expectedResult: ctrl.Result{}, + }, } for _, tt := range tests { diff --git a/internal/test/integration/authorization_model_generation_test.go b/internal/test/integration/authorization_model_generation_test.go index 9b517adb..0df0be39 100644 --- a/internal/test/integration/authorization_model_generation_test.go +++ b/internal/test/integration/authorization_model_generation_test.go @@ -111,9 +111,20 @@ func (suite *IntegrationSuite) TestAuthorizationModelGeneration_Finalize() { var testApiBinding1, testApiBinding2 kcpapiv1alpha1.APIBinding suite.Require().NoError(testAccount1Client.Get(ctx, client.ObjectKey{Name: apiBinding1.Name}, &testApiBinding1)) suite.Require().NoError(testAccount2Client.Get(ctx, client.ObjectKey{Name: apiBinding2.Name}, &testApiBinding2)) + expectedFinalizers := []string{"apis.kcp.io/apibinding-finalizer", "core.platform-mesh.io/apibinding-finalizer"} - suite.Require().Equal(expectedFinalizers, testApiBinding1.Finalizers, "APIBinding %s should have the expected finalizers", testApiBinding1.Name) - suite.Require().Equal(expectedFinalizers, testApiBinding2.Finalizers, "APIBinding %s should have the expected finalizers", testApiBinding2.Name) + + suite.Assert().Eventually(func() bool { + var testApiBinding1, testApiBinding2 kcpapiv1alpha1.APIBinding + if err := testAccount1Client.Get(ctx, client.ObjectKey{Name: apiBinding1.Name}, &testApiBinding1); err != nil { + return false + } + if err := testAccount2Client.Get(ctx, client.ObjectKey{Name: apiBinding2.Name}, &testApiBinding2); err != nil { + return false + } + return suite.Equal(expectedFinalizers, testApiBinding1.Finalizers) && + suite.Equal(expectedFinalizers, testApiBinding2.Finalizers) + }, 5*time.Second, 200*time.Millisecond, "APIBindings should have the expected finalizers") err = testAccount1Client.Delete(ctx, apiBinding1) suite.Require().NoError(err)