From 7d491e5d1b0dcd9cdf00b26eaf23ee04633c83eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20Echterh=C3=B6lter?= Date: Tue, 25 Feb 2025 16:40:35 +0100 Subject: [PATCH] feat: add AccountInfo subroutine and related configurations --- .testcoverage.yml | 1 + api/v1alpha1/account_info_types.go | 16 +- api/v1alpha1/zz_generated.deepcopy.go | 80 ++- ...aml => core.openmfp.org_accountinfos.yaml} | 20 +- .../resources/apiexport-core.openmfp.org.yaml | 2 +- ...eschema-AccountInfos.core.openmfp.org.yaml | 22 +- internal/config/config.go | 3 + internal/controller/account_controller.go | 10 +- .../controller/account_controller_test.go | 187 ++++- pkg/service/service.go | 94 --- pkg/service/service_test.go | 357 ---------- pkg/subroutines/accountinfo.go | 74 +- pkg/subroutines/accountinfo_test.go | 132 ++-- pkg/subroutines/fga.go | 112 ++- pkg/subroutines/fga_test.go | 656 ++++++------------ pkg/testing/kcpenvtest/server.go | 17 +- .../apiexport-core.openmfp.org.yaml | 4 +- ...schema-accountinfos.core.openmfp.org.yaml} | 22 +- ...ourceschema-accounts.core.openmfp.org.yaml | 2 +- 19 files changed, 698 insertions(+), 1113 deletions(-) rename config/crd/{core.openmfp.org_AccountInfos.yaml => core.openmfp.org_accountinfos.yaml} (88%) delete mode 100644 pkg/service/service.go delete mode 100644 pkg/service/service_test.go rename test/setup/01-openmfp-system/{apiresourceschema-AccountInfos.core.openmfp.org.yaml => apiresourceschema-accountinfos.core.openmfp.org.yaml} (86%) diff --git a/.testcoverage.yml b/.testcoverage.yml index 4b60e82..4df5e53 100644 --- a/.testcoverage.yml +++ b/.testcoverage.yml @@ -4,3 +4,4 @@ exclude: - api/v1alpha1 # skipping generated files and crd type definitions - main\.go$ # skip covering main.go - ^cmd # skip covering cmd directory + - ^pkg/testing/kcpenvtest # skip covering kcpenvtest diff --git a/api/v1alpha1/account_info_types.go b/api/v1alpha1/account_info_types.go index 0dd23db..59a7f65 100644 --- a/api/v1alpha1/account_info_types.go +++ b/api/v1alpha1/account_info_types.go @@ -24,14 +24,20 @@ import ( type AccountInfoSpec struct { FGA FGAInfo `json:"fga"` Account AccountLocation `json:"account"` - ParentAccount *AccountLocation `json:"parentAccount"` + ParentAccount *AccountLocation `json:"parentAccount,omitempty"` Organization AccountLocation `json:"organization"` + ClusterInfo ClusterInfo `json:"clusterInfo"` +} + +type ClusterInfo struct { + CA string `json:"ca"` } type AccountLocation struct { Name string `json:"name"` ClusterId string `json:"clusterId"` Path string `json:"path"` + URL string `json:"url"` Type AccountType `json:"type"` } @@ -48,9 +54,9 @@ type AccountInfoStatus struct { } // +kubebuilder:object:root=true +// +kubebuilder:resource:path=accountinfos // +kubebuilder:subresource:status // +kubebuilder:resource:scope=Cluster -// +kubebuilder:resource:path=accountinfos // Account is the Schema for the accounts API type AccountInfo struct { metav1.TypeMeta `json:",inline"` @@ -62,13 +68,13 @@ type AccountInfo struct { //+kubebuilder:object:root=true -// AccountLocationList contains a list of Account -type AccountLocationList struct { +// AccountInfoList contains a list of Account +type AccountInfoList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` Items []Account `json:"items"` } func init() { - SchemeBuilder.Register(&AccountInfo{}, &AccountLocationList{}) + SchemeBuilder.Register(&AccountInfo{}, &AccountInfoList{}) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2c7e6b9..1ed8792 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -95,6 +95,38 @@ func (in *AccountInfo) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccountInfoList) DeepCopyInto(out *AccountInfoList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]Account, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccountInfoList. +func (in *AccountInfoList) DeepCopy() *AccountInfoList { + if in == nil { + return nil + } + out := new(AccountInfoList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AccountInfoList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AccountInfoSpec) DeepCopyInto(out *AccountInfoSpec) { *out = *in @@ -106,6 +138,7 @@ func (in *AccountInfoSpec) DeepCopyInto(out *AccountInfoSpec) { **out = **in } out.Organization = in.Organization + out.ClusterInfo = in.ClusterInfo } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccountInfoSpec. @@ -180,38 +213,6 @@ func (in *AccountLocation) DeepCopy() *AccountLocation { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AccountLocationList) DeepCopyInto(out *AccountLocationList) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ListMeta.DeepCopyInto(&out.ListMeta) - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]Account, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccountLocationList. -func (in *AccountLocationList) DeepCopy() *AccountLocationList { - if in == nil { - return nil - } - out := new(AccountLocationList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *AccountLocationList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AccountSpec) DeepCopyInto(out *AccountSpec) { *out = *in @@ -272,6 +273,21 @@ func (in *AccountStatus) DeepCopy() *AccountStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterInfo) DeepCopyInto(out *ClusterInfo) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterInfo. +func (in *ClusterInfo) DeepCopy() *ClusterInfo { + if in == nil { + return nil + } + out := new(ClusterInfo) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Extension) DeepCopyInto(out *Extension) { *out = *in diff --git a/config/crd/core.openmfp.org_AccountInfos.yaml b/config/crd/core.openmfp.org_accountinfos.yaml similarity index 88% rename from config/crd/core.openmfp.org_AccountInfos.yaml rename to config/crd/core.openmfp.org_accountinfos.yaml index ebdfe86..e91e023 100644 --- a/config/crd/core.openmfp.org_AccountInfos.yaml +++ b/config/crd/core.openmfp.org_accountinfos.yaml @@ -12,7 +12,7 @@ spec: listKind: AccountInfoList plural: accountinfos singular: accountinfo - scope: Namespaced + scope: Cluster versions: - name: v1alpha1 schema: @@ -49,11 +49,21 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url + type: object + clusterInfo: + properties: + ca: + type: string + required: + - ca type: object fga: properties: @@ -77,11 +87,14 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url type: object parentAccount: properties: @@ -93,17 +106,20 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url type: object required: - account + - clusterInfo - fga - organization - - parentAccount type: object status: description: AccountInfoStatus defines the observed state of Account diff --git a/config/resources/apiexport-core.openmfp.org.yaml b/config/resources/apiexport-core.openmfp.org.yaml index 20462b9..d6d2a99 100644 --- a/config/resources/apiexport-core.openmfp.org.yaml +++ b/config/resources/apiexport-core.openmfp.org.yaml @@ -6,5 +6,5 @@ metadata: spec: latestResourceSchemas: - v250220-cdcd05c.accounts.core.openmfp.org - - v250224-cafdd19.accountinfos.core.openmfp.org + - v250225-f1af48a.accountinfos.core.openmfp.org status: {} diff --git a/config/resources/apiresourceschema-AccountInfos.core.openmfp.org.yaml b/config/resources/apiresourceschema-AccountInfos.core.openmfp.org.yaml index 2f40013..b1ccfa4 100644 --- a/config/resources/apiresourceschema-AccountInfos.core.openmfp.org.yaml +++ b/config/resources/apiresourceschema-AccountInfos.core.openmfp.org.yaml @@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1 kind: APIResourceSchema metadata: creationTimestamp: null - name: v250224-cafdd19.accountinfos.core.openmfp.org + name: v250225-f1af48a.accountinfos.core.openmfp.org spec: group: core.openmfp.org names: @@ -10,7 +10,7 @@ spec: listKind: AccountInfoList plural: accountinfos singular: accountinfo - scope: Namespaced + scope: Cluster versions: - name: v1alpha1 schema: @@ -46,11 +46,21 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url + type: object + clusterInfo: + properties: + ca: + type: string + required: + - ca type: object fga: properties: @@ -74,11 +84,14 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url type: object parentAccount: properties: @@ -90,17 +103,20 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url type: object required: - account + - clusterInfo - fga - organization - - parentAccount type: object status: description: AccountInfoStatus defines the observed state of Account diff --git a/internal/config/config.go b/internal/config/config.go index ce00c74..aa29eb3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,6 +33,9 @@ type Config struct { Workspace struct { Enabled bool `envconfig:"default=true"` } + AccountInfo struct { + Enabled bool `envconfig:"default=true"` + } FGA struct { Enabled bool `envconfig:"default=true"` RootNamespace string `envconfig:"default=openmfp-root"` diff --git a/internal/controller/account_controller.go b/internal/controller/account_controller.go index 23e9e4a..0b2539f 100644 --- a/internal/controller/account_controller.go +++ b/internal/controller/account_controller.go @@ -31,7 +31,6 @@ import ( corev1alpha1 "github.com/openmfp/account-operator/api/v1alpha1" "github.com/openmfp/account-operator/internal/config" - "github.com/openmfp/account-operator/pkg/service" "github.com/openmfp/account-operator/pkg/subroutines" ) @@ -50,6 +49,10 @@ func NewAccountReconciler(log *logger.Logger, mgr ctrl.Manager, cfg config.Confi if cfg.Subroutines.Workspace.Enabled { subs = append(subs, subroutines.NewWorkspaceSubroutine(mgr.GetClient())) } + if cfg.Subroutines.AccountInfo.Enabled { + subs = append(subs, subroutines.NewAccountInfoSubroutine(mgr.GetClient(), mgr.GetConfig().CAFile)) + } + // The extensions subroutine is temporarily disabled in this release. It will be refactored and activated in a later release //if cfg.Subroutines.Extension.Enabled { // subs = append(subs, subroutines.NewExtensionSubroutine(mgr.GetClient())) //} @@ -68,11 +71,8 @@ func NewAccountReconciler(log *logger.Logger, mgr ctrl.Manager, cfg config.Confi } log.Debug().Msg("FGA client created") - srv := service.NewService(mgr.GetClient(), cfg.Subroutines.FGA.RootNamespace) - fgaClient := openfgav1.NewOpenFGAServiceClient(conn) - - subs = append(subs, subroutines.NewFGASubroutine(mgr.GetClient(), fgaClient, srv, cfg.Subroutines.FGA.RootNamespace, cfg.Subroutines.FGA.CreatorRelation, cfg.Subroutines.FGA.ParentRelation, cfg.Subroutines.FGA.ObjectType)) + subs = append(subs, subroutines.NewFGASubroutine(mgr.GetClient(), fgaClient, cfg.Subroutines.FGA.CreatorRelation, cfg.Subroutines.FGA.ParentRelation, cfg.Subroutines.FGA.ObjectType)) } return &AccountReconciler{ lifecycle: lifecycle.NewLifecycleManager(log, operatorName, accountReconcilerName, mgr.GetClient(), subs).WithSpreadingReconciles().WithConditionManagement(), diff --git a/internal/controller/account_controller_test.go b/internal/controller/account_controller_test.go index b3bae1d..4ed5357 100644 --- a/internal/controller/account_controller_test.go +++ b/internal/controller/account_controller_test.go @@ -1,4 +1,4 @@ -package controller +package controller_test import ( "context" @@ -8,28 +8,30 @@ import ( "testing" "time" + kcpcorev1alpha "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" kcptenancyv1alpha "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" openmfpcontext "github.com/openmfp/golang-commons/context" "github.com/openmfp/golang-commons/logger" "github.com/stretchr/testify/suite" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/kcp" - corev1alpha1 "github.com/openmfp/account-operator/api/v1alpha1" + "github.com/openmfp/account-operator/api/v1alpha1" "github.com/openmfp/account-operator/internal/config" - "github.com/openmfp/account-operator/pkg/subroutines" + "github.com/openmfp/account-operator/internal/controller" "github.com/openmfp/account-operator/pkg/testing/kcpenvtest" ) const ( - defaultTestTimeout = 5 * time.Second + defaultTestTimeout = 15 * time.Minute defaultTickInterval = 250 * time.Millisecond defaultNamespace = "default" ) @@ -42,6 +44,8 @@ type AccountTestSuite struct { testEnv *kcpenvtest.Environment log *logger.Logger cancel context.CancelCauseFunc + rootConfig *rest.Config + scheme *runtime.Scheme } func (suite *AccountTestSuite) SetupSuite() { @@ -73,36 +77,41 @@ func (suite *AccountTestSuite) SetupSuite() { } k8sCfg, vsUrl, err = suite.testEnv.Start(useExistingCluster) if err != nil { - err = suite.testEnv.Stop(useExistingCluster) - suite.Require().NoError(err) + stopErr := suite.testEnv.Stop(useExistingCluster) + suite.Require().NoError(stopErr) } suite.Require().NoError(err) suite.Require().NotNil(k8sCfg) suite.Require().NotEmpty(vsUrl) + suite.rootConfig = k8sCfg - utilruntime.Must(corev1alpha1.AddToScheme(scheme.Scheme)) - utilruntime.Must(v1.AddToScheme(scheme.Scheme)) + suite.scheme = runtime.NewScheme() + utilruntime.Must(v1alpha1.AddToScheme(suite.scheme)) + utilruntime.Must(v1.AddToScheme(suite.scheme)) + utilruntime.Must(kcpcorev1alpha.AddToScheme(suite.scheme)) + utilruntime.Must(kcptenancyv1alpha.AddToScheme(suite.scheme)) - managerCfg := rest.CopyConfig(k8sCfg) + managerCfg := rest.CopyConfig(suite.rootConfig) managerCfg.Host = vsUrl - testDataConfig := rest.CopyConfig(k8sCfg) - testDataConfig.Host = fmt.Sprintf("%s:%s", k8sCfg.Host, "openmfp:orgs:root-org") + testDataConfig := rest.CopyConfig(suite.rootConfig) + testDataConfig.Host = fmt.Sprintf("%s:%s", suite.rootConfig.Host, "openmfp:orgs:root-org") // +kubebuilder:scaffold:scheme suite.kubernetesClient, err = client.New(testDataConfig, client.Options{ - Scheme: scheme.Scheme, + Scheme: suite.scheme, }) suite.Require().NoError(err) suite.kubernetesManager, err = kcp.NewClusterAwareManager(managerCfg, ctrl.Options{ - Scheme: scheme.Scheme, + Scheme: suite.scheme, Logger: log.Logr(), BaseContext: func() context.Context { return testContext }, }) suite.Require().NoError(err) - accountReconciler := NewAccountReconciler(log, suite.kubernetesManager, cfg) + cfg.Subroutines.FGA.Enabled = false + accountReconciler := controller.NewAccountReconciler(log, suite.kubernetesManager, cfg) err = accountReconciler.SetupWithManager(suite.kubernetesManager, cfg, log) suite.Require().NoError(err) @@ -129,12 +138,12 @@ func (suite *AccountTestSuite) TestAddingFinalizer() { testContext := context.Background() accountName := "test-account-finalizer" - account := &corev1alpha1.Account{ + account := &v1alpha1.Account{ ObjectMeta: metav1.ObjectMeta{ Name: accountName, }, - Spec: corev1alpha1.AccountSpec{ - Type: corev1alpha1.AccountTypeOrg, + Spec: v1alpha1.AccountSpec{ + Type: v1alpha1.AccountTypeAccount, }} // When @@ -142,7 +151,7 @@ func (suite *AccountTestSuite) TestAddingFinalizer() { suite.Nil(err) // Then - createdAccount := corev1alpha1.Account{} + createdAccount := v1alpha1.Account{} suite.Assert().Eventually(func() bool { err := suite.kubernetesClient.Get(testContext, types.NamespacedName{ Name: accountName, @@ -151,37 +160,135 @@ func (suite *AccountTestSuite) TestAddingFinalizer() { return err == nil && createdAccount.Finalizers != nil }, defaultTestTimeout, defaultTickInterval) - suite.Equal([]string{subroutines.WorkspaceSubroutineFinalizer, subroutines.ExtensionSubroutineFinalizer, "account.core.openmfp.org/fga"}, createdAccount.ObjectMeta.Finalizers) + suite.Equal([]string{"account.core.openmfp.org/finalizer"}, createdAccount.ObjectMeta.Finalizers) } func (suite *AccountTestSuite) TestWorkspaceCreation() { // Given + var err error testContext := context.Background() accountName := "test-account-ws-creation" - account := &corev1alpha1.Account{ + account := &v1alpha1.Account{ ObjectMeta: metav1.ObjectMeta{ Name: accountName, }, - Spec: corev1alpha1.AccountSpec{ - Type: corev1alpha1.AccountTypeAccount, + Spec: v1alpha1.AccountSpec{ + Type: v1alpha1.AccountTypeAccount, }} // When - err := suite.kubernetesClient.Create(testContext, account) + err = suite.kubernetesClient.Create(testContext, account) suite.Require().NoError(err) // Then + + // Wait for workspace creation and ready createdWorkspace := kcptenancyv1alpha.Workspace{} suite.Assert().Eventually(func() bool { err := suite.kubernetesClient.Get(testContext, types.NamespacedName{ Name: accountName, }, &createdWorkspace) - suite.log.Debug().Err(err).Msg("error") + return err == nil && createdWorkspace.Status.Phase == kcpcorev1alpha.LogicalClusterPhaseReady + }, defaultTestTimeout, defaultTickInterval) + + // Wait for conditions update on account + updatedAccount := &v1alpha1.Account{} + suite.Assert().Eventually(func() bool { + err := suite.kubernetesClient.Get(testContext, types.NamespacedName{ + Name: accountName, + }, updatedAccount) + cond := meta.FindStatusCondition(updatedAccount.Status.Conditions, "WorkspaceSubroutine_Ready") + return err == nil && cond != nil && cond.Status == metav1.ConditionTrue + }, defaultTestTimeout, defaultTickInterval) + + // Verify workspace and account conditions + suite.verifyWorkspace(testContext, accountName) + suite.verifyCondition(updatedAccount.Status.Conditions, "WorkspaceSubroutine_Ready", metav1.ConditionTrue, "Complete") +} + +func (suite *AccountTestSuite) TestAccountInfoCreationForOrganization() { + testContext := context.Background() + + // Then + accountInfo := v1alpha1.AccountInfo{} + suite.Assert().Eventually(func() bool { + err := suite.kubernetesClient.Get(testContext, types.NamespacedName{ + Name: "account", + }, &accountInfo) return err == nil }, defaultTestTimeout, defaultTickInterval) // Test if Workspace exists - suite.verifyWorkspace(testContext, accountName, accountName) + suite.NotNil(accountInfo.Spec.ClusterInfo.CA) + suite.Equal("root-org", accountInfo.Spec.Account.Name) + suite.NotNil(accountInfo.Spec.Account.URL) + suite.Equal("root:openmfp:orgs:root-org", accountInfo.Spec.Account.Path) + suite.Equal("root-org", accountInfo.Spec.Organization.Name) + suite.Equal("root-org", accountInfo.Spec.Organization.Name) + suite.NotNil(accountInfo.Spec.Organization.URL) + suite.Equal("root:openmfp:orgs:root-org", accountInfo.Spec.Organization.Path) + suite.Nil(accountInfo.Spec.ParentAccount) +} + +func (suite *AccountTestSuite) TestAccountInfoCreationForAccount() { + var err error + testContext := context.Background() + accountName := "test-account-account-info-creation1" + account := &v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: accountName, + }, + Spec: v1alpha1.AccountSpec{ + Type: v1alpha1.AccountTypeAccount, + }} + + // When + err = suite.kubernetesClient.Create(testContext, account) + suite.Require().NoError(err) + + // Then + // Wait for Account to be ready + updatedAccount := &v1alpha1.Account{} + suite.Assert().Eventually(func() bool { + err := suite.kubernetesClient.Get(testContext, types.NamespacedName{ + Name: accountName, + }, updatedAccount) + cond := meta.FindStatusCondition(updatedAccount.Status.Conditions, "Ready") + return err == nil && cond != nil && cond.Status == metav1.ConditionTrue + }, defaultTestTimeout, defaultTickInterval) + + // Retrieve account info from workspace + testDataConfig := rest.CopyConfig(suite.rootConfig) + testDataConfig.Host = fmt.Sprintf("%s:%s", suite.rootConfig.Host, "openmfp:orgs:root-org:test-account-account-info-creation1") + testClient, err := client.New(testDataConfig, client.Options{ + Scheme: suite.scheme, + }) + suite.Require().NoError(err) + + accountInfo := v1alpha1.AccountInfo{} + suite.Assert().Eventually(func() bool { + err := testClient.Get(testContext, types.NamespacedName{ + Name: "account", + }, &accountInfo) + return err == nil + }, defaultTestTimeout, defaultTickInterval) + + // Test if Workspace exists + suite.NotNil(accountInfo.Spec.ClusterInfo.CA) + // Account + suite.Equal("test-account-account-info-creation1", accountInfo.Spec.Account.Name) + suite.NotNil(accountInfo.Spec.Account.URL) + suite.Equal("root:openmfp:orgs:root-org:test-account-account-info-creation1", accountInfo.Spec.Account.Path) + // Organization + suite.Equal("root-org", accountInfo.Spec.Organization.Name) + suite.Equal("root-org", accountInfo.Spec.Organization.Name) + suite.NotNil(accountInfo.Spec.Organization.URL) + // Parent Account + suite.Require().NotNil(accountInfo.Spec.ParentAccount) + suite.Equal("root:openmfp:orgs:root-org", accountInfo.Spec.ParentAccount.Path) + suite.Equal("root-org", accountInfo.Spec.ParentAccount.Name) + suite.NotNil(accountInfo.Spec.ParentAccount.URL) + } // func (suite *AccountTestSuite) TestExtensionProcessing() { @@ -196,14 +303,14 @@ func (suite *AccountTestSuite) TestWorkspaceCreation() { // } // }` // -// account := &corev1alpha1.Account{ +// account := &v1alpha1.Account{ // ObjectMeta: metav1.ObjectMeta{ // Name: accountName, // Workspace: defaultNamespace, // }, -// Spec: corev1alpha1.AccountSpec{ -// Type: corev1alpha1.AccountTypeAccount, -// Extensions: []corev1alpha1.Extension{ +// Spec: v1alpha1.AccountSpec{ +// Type: v1alpha1.AccountTypeAccount, +// Extensions: []v1alpha1.Extension{ // { // TypeMeta: metav1.TypeMeta{ // APIVersion: "networking.k8s.io/v1", @@ -221,7 +328,7 @@ func (suite *AccountTestSuite) TestWorkspaceCreation() { // suite.Assert().NoError(err) // // // Then -// createdAccount := corev1alpha1.Account{} +// createdAccount := v1alpha1.Account{} // createdNetworkPolicy := networkv1.NetworkPolicy{} // suite.Assert().Eventually(func() bool { // err := suite.kubernetesClient.Get(context.Background(), types.NamespacedName{ @@ -241,7 +348,7 @@ func (suite *AccountTestSuite) TestWorkspaceCreation() { // }, time.Second*30, time.Millisecond*250) // // } -func (suite *AccountTestSuite) verifyWorkspace(ctx context.Context, accName string, name string) { +func (suite *AccountTestSuite) verifyWorkspace(ctx context.Context, name string) { suite.Require().NotNil(name, "failed to verify namespace name") ns := &kcptenancyv1alpha.Workspace{} @@ -251,6 +358,22 @@ func (suite *AccountTestSuite) verifyWorkspace(ctx context.Context, accName stri suite.Assert().Len(ns.GetOwnerReferences(), 1, "failed to verify owner reference on workspace") } +func (suite *AccountTestSuite) verifyCondition(conditions []metav1.Condition, conditionType string, status metav1.ConditionStatus, reason string) { + condition := getCondition(conditions, conditionType) + suite.Require().NotNil(condition) + suite.Equal(status, condition.Status) + suite.Equal(reason, condition.Reason) +} + +func getCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition { + for _, condition := range conditions { + if condition.Type == conditionType { + return &condition + } + } + return nil +} + func TestAccountTestSuite(t *testing.T) { suite.Run(t, new(AccountTestSuite)) } diff --git a/pkg/service/service.go b/pkg/service/service.go deleted file mode 100644 index f39294b..0000000 --- a/pkg/service/service.go +++ /dev/null @@ -1,94 +0,0 @@ -package service - -import ( - "context" - "errors" - - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/openmfp/account-operator/api/v1alpha1" -) - -type Servicer interface { - GetFirstLevelAccountForAccount(ctx context.Context, accountKey client.ObjectKey) (*v1alpha1.Account, error) - GetFirstLevelAccountForNamespace(ctx context.Context, namespace string) (*v1alpha1.Account, error) - - GetAccount(ctx context.Context, accountKey client.ObjectKey) (*v1alpha1.Account, error) - GetAccountForNamespace(ctx context.Context, namespace string) (*v1alpha1.Account, error) -} - -var _ Servicer = (*Service)(nil) - -type Service struct { - client client.Client - rootNamespace string -} - -func NewService(client client.Client, rootNamespace string) *Service { - return &Service{ - client: client, - rootNamespace: rootNamespace, - } -} - -func (s *Service) getAccountOwnerAndNamespaceForNamespace(ctx context.Context, namespace string) (string, string, error) { - var ns corev1.Namespace - err := s.client.Get(ctx, client.ObjectKey{Name: namespace}, &ns) - if err != nil { - return "", "", err - } - - if ns.Labels == nil { - return "", "", errors.New("namespace does not have a label and therefore no connected account") - } - - accountNamespace, ok := ns.Labels[v1alpha1.NamespaceAccountOwnerNamespaceLabel] - if !ok || accountNamespace == "" { - return "", "", errors.New("namespace does not have an account-owner-namespace label and therefore no connected account") - } - - accountName, ok := ns.Labels[v1alpha1.NamespaceAccountOwnerLabel] - if !ok || accountName == "" { - return "", "", errors.New("namespace does not have an account-owner label and therefore no connected account") - } - - return accountName, accountNamespace, nil -} - -func (s *Service) GetFirstLevelAccountForAccount(ctx context.Context, accountKey client.ObjectKey) (*v1alpha1.Account, error) { - return s.GetFirstLevelAccountForNamespace(ctx, accountKey.Namespace) -} - -func (s *Service) GetFirstLevelAccountForNamespace(ctx context.Context, namespace string) (*v1alpha1.Account, error) { - - accountName, accountNamespace, err := s.getAccountOwnerAndNamespaceForNamespace(ctx, namespace) - if err != nil { - return nil, err - } - - if s.rootNamespace != accountNamespace { - return s.GetFirstLevelAccountForNamespace(ctx, accountNamespace) - } - - var account v1alpha1.Account - err = s.client.Get(ctx, client.ObjectKey{Name: accountName, Namespace: accountNamespace}, &account) - return &account, err -} - -func (s *Service) GetAccount(ctx context.Context, accountKey client.ObjectKey) (*v1alpha1.Account, error) { - var account v1alpha1.Account - err := s.client.Get(ctx, accountKey, &account) - return &account, err -} - -func (s *Service) GetAccountForNamespace(ctx context.Context, namespace string) (*v1alpha1.Account, error) { - accountName, accountNamespace, err := s.getAccountOwnerAndNamespaceForNamespace(ctx, namespace) - if err != nil { - return nil, err - } - - var account v1alpha1.Account - err = s.client.Get(ctx, client.ObjectKey{Name: accountName, Namespace: accountNamespace}, &account) - return &account, err -} diff --git a/pkg/service/service_test.go b/pkg/service/service_test.go deleted file mode 100644 index 103b04f..0000000 --- a/pkg/service/service_test.go +++ /dev/null @@ -1,357 +0,0 @@ -package service_test - -import ( - "context" - "path/filepath" - "slices" - "testing" - - "github.com/stretchr/testify/suite" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - - "github.com/openmfp/account-operator/api/v1alpha1" - "github.com/openmfp/account-operator/pkg/service" -) - -type serviceTest struct { - suite.Suite - testEnv envtest.Environment - testClient client.Client -} - -func TestService(t *testing.T) { - suite.Run(t, new(serviceTest)) -} - -func (s *serviceTest) SetupSuite() { - - s.testEnv = envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, - ErrorIfCRDPathMissing: true, - } - cfg, err := s.testEnv.Start() - - s.Require().NoError(err) - - s.Require().NoError(v1alpha1.AddToScheme(scheme.Scheme)) - - s.testClient, err = client.New(cfg, client.Options{ - Scheme: scheme.Scheme, - }) - s.Require().NoError(err) - - err = s.testClient.Create(context.Background(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "root-namespace"}}) - s.Require().NoError(err) -} - -func (s *serviceTest) TearDownSuite() { - s.Require().NoError(s.testEnv.Stop()) -} - -func (s *serviceTest) TestGetAccount() { - tests := []struct { - name string - mockObjects []client.Object - objectKey client.ObjectKey - }{ - { - name: "", - mockObjects: []client.Object{ - &v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-account", - Namespace: "root-namespace", - }, - Spec: v1alpha1.AccountSpec{ - Type: v1alpha1.AccountTypeAccount, - }, - }, - }, - objectKey: types.NamespacedName{Namespace: "root-namespace", Name: "test-account"}, - }, - } - for _, test := range tests { - s.Run(test.name, func() { - svc := service.NewService(s.testClient, "root-namespace") - - for _, obj := range test.mockObjects { - err := s.testClient.Create(context.Background(), obj) - s.Require().NoError(err) - } - - account, err := svc.GetAccount(context.Background(), test.objectKey) - s.Require().NoError(err) - - s.Require().Equal(test.objectKey.Name, account.Name) - s.Require().Equal(test.objectKey.Namespace, account.Namespace) - - for _, obj := range test.mockObjects { - err := s.testClient.Delete(context.Background(), obj) - s.Require().NoError(err) - } - }) - } -} - -func (s *serviceTest) TestGetAccountForNamespace() { - tests := []struct { - name string - mockObjects []client.Object - namespace string - expectedAccount client.ObjectKey - expectError bool - }{ - { - name: "", - mockObjects: []client.Object{ - &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "account-for-namespace", - Labels: map[string]string{ - v1alpha1.NamespaceAccountOwnerNamespaceLabel: "root-namespace", - v1alpha1.NamespaceAccountOwnerLabel: "test-account", - }, - }, - }, - &v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-account", - Namespace: "root-namespace", - }, - Spec: v1alpha1.AccountSpec{ - Type: v1alpha1.AccountTypeAccount, - }, - }, - }, - namespace: "account-for-namespace", - expectedAccount: types.NamespacedName{ - Namespace: "root-namespace", - Name: "test-account", - }, - }, - { - name: "missing namespace", - mockObjects: []client.Object{ - &v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-account", - Namespace: "root-namespace", - }, - Spec: v1alpha1.AccountSpec{ - Type: v1alpha1.AccountTypeAccount, - }, - }, - }, - namespace: "account-for-namespace-4", - expectedAccount: types.NamespacedName{ - Namespace: "root-namespace", - Name: "test-account", - }, - expectError: true, - }, - { - name: "return an error due to one missing owner-namespace label", - mockObjects: []client.Object{ - &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "account-for-namespace-2", - Labels: map[string]string{ - v1alpha1.NamespaceAccountOwnerNamespaceLabel: "root-namespace", - }, - }, - }, - &v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-account", - Namespace: "root-namespace", - }, - Spec: v1alpha1.AccountSpec{ - Type: v1alpha1.AccountTypeAccount, - }, - }, - }, - namespace: "account-for-namespace-2", - expectedAccount: types.NamespacedName{ - Namespace: "root-namespace", - Name: "test-account", - }, - expectError: true, - }, - { - name: "return an error due to missing owner name label", - mockObjects: []client.Object{ - &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "account-for-namespace-3", - }, - }, - &v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-account", - Namespace: "root-namespace", - }, - Spec: v1alpha1.AccountSpec{ - Type: v1alpha1.AccountTypeAccount, - }, - }, - }, - namespace: "account-for-namespace-3", - expectedAccount: types.NamespacedName{ - Namespace: "root-namespace", - Name: "test-account", - }, - expectError: true, - }, - } - for _, test := range tests { - s.Run(test.name, func() { - svc := service.NewService(s.testClient, "root-namespace") - - for _, obj := range test.mockObjects { - err := s.testClient.Create(context.Background(), obj) - s.Require().NoError(err) - } - - defer func() { - for _, obj := range test.mockObjects { - err := s.testClient.Delete(context.Background(), obj) - s.Require().NoError(err) - } - }() - - account, err := svc.GetAccountForNamespace(context.Background(), test.namespace) - if test.expectError { - s.Require().Error(err) - return - } else { - s.Require().NoError(err) - } - - s.Require().Equal(test.expectedAccount.Name, account.Name) - s.Require().Equal(test.expectedAccount.Namespace, account.Namespace) - - }) - } -} - -func (s *serviceTest) TestGetFirstLevelAccount() { - tests := []struct { - name string - mockObjects []client.Object - expectedAccount client.ObjectKey - expectError bool - namespace string - }{ - //{ - // name: "", - // mockObjects: []client.Object{ - // &v1alpha1.Account{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "first-level-account", - // Namespace: "root-namespace", - // }, - // Spec: v1alpha1.AccountSpec{ - // Type: v1alpha1.AccountTypeOrg, - // }, - // Status: v1alpha1.AccountStatus{ - // Workspace: pointer.To("sub-namespace"), - // }, - // }, - // &corev1.Namespace{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "first-level-sub-namespace", - // Labels: map[string]string{ - // v1alpha1.NamespaceAccountOwnerNamespaceLabel: "root-namespace", - // v1alpha1.NamespaceAccountOwnerLabel: "first-level-account", - // }, - // }, - // }, - // &v1alpha1.Account{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "sub-account", - // Namespace: "first-level-sub-namespace", - // }, - // Spec: v1alpha1.AccountSpec{ - // Type: v1alpha1.AccountTypeOrg, - // }, - // }, - // }, - // namespace: "first-level-sub-namespace", - // expectedAccount: types.NamespacedName{ - // Namespace: "root-namespace", - // Name: "first-level-account", - // }, - //}, - //{ - // name: "invalid namespace", - // mockObjects: []client.Object{ - // &v1alpha1.Account{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "first-level-account", - // Namespace: "root-namespace", - // }, - // Spec: v1alpha1.AccountSpec{ - // Type: v1alpha1.AccountTypeOrg, - // }, - // Status: v1alpha1.AccountStatus{ - // Workspace: pointer.To("sub-namespace"), - // }, - // }, - // &corev1.Namespace{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "first-level-sub-namespace1", - // }, - // }, - // &v1alpha1.Account{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "sub-account", - // Namespace: "first-level-sub-namespace1", - // }, - // Spec: v1alpha1.AccountSpec{ - // Type: v1alpha1.AccountTypeOrg, - // }, - // }, - // }, - // namespace: "first-level-sub-namespace1", - // expectError: true, - //}, - } - for _, test := range tests { - s.Run(test.name, func() { - svc := service.NewService(s.testClient, "root-namespace") - - for _, obj := range test.mockObjects { - err := s.testClient.Create(context.Background(), obj) - s.Require().NoError(err) - } - - account, err := svc.GetFirstLevelAccountForNamespace(context.Background(), test.namespace) - if test.expectError { - s.Require().Error(err) - } else { - s.Require().NoError(err) - - s.Require().Equal(test.expectedAccount.Name, account.Name) - s.Require().Equal(test.expectedAccount.Namespace, account.Namespace) - - account, err = svc.GetFirstLevelAccountForAccount(context.Background(), types.NamespacedName{Namespace: test.namespace, Name: "sub-account"}) - s.Require().NoError(err) - - s.Require().Equal(test.expectedAccount.Name, account.Name) - s.Require().Equal(test.expectedAccount.Namespace, account.Namespace) - - slices.Reverse(test.mockObjects) - - for _, obj := range test.mockObjects { - err := s.testClient.Delete(context.Background(), obj) - s.Require().NoError(err) - } - } - }) - } -} diff --git a/pkg/subroutines/accountinfo.go b/pkg/subroutines/accountinfo.go index 8bf64ec..15dab55 100644 --- a/pkg/subroutines/accountinfo.go +++ b/pkg/subroutines/accountinfo.go @@ -3,6 +3,7 @@ package subroutines import ( "context" "fmt" + "strings" kcpcorev1alpha "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" kcptenancyv1alpha "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" @@ -25,22 +26,24 @@ import ( var _ lifecycle.Subroutine = (*AccountInfoSubroutine)(nil) const ( - AccountInfoSubroutineName = "AccountLocationSubroutine" + AccountInfoSubroutineName = "AccountInfoSubroutine" + DefaultAccountInfoName = "account" ) type AccountInfoSubroutine struct { - client client.Client + client client.Client + serverCA string } -func NewAccountInfoSubroutine(client client.Client) *AccountInfoSubroutine { - return &AccountInfoSubroutine{client: client} +func NewAccountInfoSubroutine(client client.Client, serverCA string) *AccountInfoSubroutine { + return &AccountInfoSubroutine{client: client, serverCA: serverCA} } func (r *AccountInfoSubroutine) GetName() string { return AccountInfoSubroutineName } -func (r *AccountInfoSubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.RuntimeObject) (ctrl.Result, errors.OperatorError) { +func (r *AccountInfoSubroutine) Finalize(_ context.Context, _ lifecycle.RuntimeObject) (ctrl.Result, errors.OperatorError) { return ctrl.Result{}, nil } @@ -54,41 +57,49 @@ func (r *AccountInfoSubroutine) Process(ctx context.Context, runtimeObj lifecycl log := logger.LoadLoggerFromContext(ctx) // select workspace for account - ws, err := r.retrieveWorkspace(ctx, instance, log) + accountWorkspace, err := r.retrieveWorkspace(ctx, instance, log) if err != nil { return ctrl.Result{}, errors.NewOperatorError(err, true, true) } + if accountWorkspace.Status.Phase != kcpcorev1alpha.LogicalClusterPhaseReady { + log.Info().Msg("workspace is not ready yet, retry") + return ctrl.Result{Requeue: true}, nil + } + // Prepare context to work in workspace - wsCtx := kontext.WithCluster(ctx, logicalcluster.Name(ws.Spec.Cluster)) + wsCtx := kontext.WithCluster(ctx, logicalcluster.Name(accountWorkspace.Spec.Cluster)) // Get FGA Store ID // For now this is hard coded, needs to be replaced with Store generation on Organization level storeId := cfg.FGA.StoreId // Retrieve logical cluster - parentPath, err := r.retrieveCurrentWorkspacePath(ctx, err, log) + currentWorkspacePath, currentWorkspaceUrl, err := r.retrieveCurrentWorkspacePath(accountWorkspace) if err != nil { return ctrl.Result{}, errors.NewOperatorError(err, true, true) } - wsPath := fmt.Sprintf("%s:%s", parentPath, ws.Name) - selfAccountLocation := v1alpha1.AccountLocation{Name: instance.Name, ClusterId: ws.Spec.Cluster, Type: instance.Spec.Type, Path: wsPath} + selfAccountLocation := v1alpha1.AccountLocation{Name: instance.Name, ClusterId: accountWorkspace.Spec.Cluster, Type: instance.Spec.Type, Path: currentWorkspacePath, URL: currentWorkspaceUrl} if instance.Spec.Type == v1alpha1.AccountTypeOrg { - accountInfo := &v1alpha1.AccountInfo{ObjectMeta: v1.ObjectMeta{Name: "account"}} + accountInfo := &v1alpha1.AccountInfo{ObjectMeta: v1.ObjectMeta{Name: DefaultAccountInfoName}} _, err = controllerutil.CreateOrUpdate(wsCtx, r.client, accountInfo, func() error { accountInfo.Spec.Account = selfAccountLocation accountInfo.Spec.ParentAccount = nil accountInfo.Spec.Organization = selfAccountLocation accountInfo.Spec.FGA.Store.Id = storeId + accountInfo.Spec.ClusterInfo.CA = r.serverCA return nil }) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(err, true, true) + } return ctrl.Result{}, nil } - parentAccountInfo, exists, err := r.retrieveAccountInfo(ctx, err, log) + parentAccountInfo, exists, err := r.retrieveAccountInfo(ctx, log) if err != nil { return ctrl.Result{}, errors.NewOperatorError(err, true, true) } @@ -97,7 +108,7 @@ func (r *AccountInfoSubroutine) Process(ctx context.Context, runtimeObj lifecycl return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("AccountInfo does not yet exist. Retry another time"), true, false) } - accountInfo := &v1alpha1.AccountInfo{ObjectMeta: v1.ObjectMeta{Name: "account"}} + accountInfo := &v1alpha1.AccountInfo{ObjectMeta: v1.ObjectMeta{Name: DefaultAccountInfoName}} _, err = controllerutil.CreateOrUpdate(wsCtx, r.client, accountInfo, func() error { accountInfo.Spec.Account = selfAccountLocation accountInfo.Spec.ParentAccount = &parentAccountInfo.Spec.Account @@ -105,12 +116,15 @@ func (r *AccountInfoSubroutine) Process(ctx context.Context, runtimeObj lifecycl accountInfo.Spec.FGA.Store.Id = storeId return nil }) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(err, true, true) + } return ctrl.Result{}, nil } -func (r *AccountInfoSubroutine) retrieveAccountInfo(ctx context.Context, err error, log *logger.Logger) (*v1alpha1.AccountInfo, bool, error) { +func (r *AccountInfoSubroutine) retrieveAccountInfo(ctx context.Context, log *logger.Logger) (*v1alpha1.AccountInfo, bool, error) { accountInfo := &v1alpha1.AccountInfo{} - err = r.client.Get(ctx, client.ObjectKey{Name: "account"}, accountInfo) + err := r.client.Get(ctx, client.ObjectKey{Name: "account"}, accountInfo) if err != nil { if kerrors.IsNotFound(err) { log.Info().Msg("accountInfo does not yet exist, retry") @@ -122,27 +136,31 @@ func (r *AccountInfoSubroutine) retrieveAccountInfo(ctx context.Context, err err return accountInfo, true, nil } -func (r *AccountInfoSubroutine) retrieveCurrentWorkspacePath(ctx context.Context, err error, log *logger.Logger) (string, error) { - lc := &kcpcorev1alpha.LogicalCluster{} - err = r.client.Get(ctx, client.ObjectKey{Name: "cluster"}, lc) - if err != nil { - log.Error().Err(err).Msg("logicalCluster does not yet exist") - return "", errors.Wrap(err, "logicalCluster does not yet exist") +func (r *AccountInfoSubroutine) retrieveCurrentWorkspacePath(ws *kcptenancyv1alpha.Workspace) (string, string, error) { + if ws.Spec.URL == "" { + return "", "", fmt.Errorf("workspace URL is empty") } - selfPath, ok := lc.ObjectMeta.Annotations["kcp.io/path"] - if !ok { - log.Error().Msg("logicalCluster does not have a path annotation") - return "", errors.New("logicalCluster does not have a path annotation") + + // Parse path from URL + split := strings.Split(ws.Spec.URL, "/") + if len(split) < 3 { + return "", "", fmt.Errorf("workspace URL is invalid") + } + + lastSegment := split[len(split)-1] + if lastSegment == "" || strings.Trim(lastSegment, " ") == "" { + return "", "", fmt.Errorf("workspace URL is empty") } - return selfPath, nil + return lastSegment, ws.Spec.URL, nil } func (r *AccountInfoSubroutine) retrieveWorkspace(ctx context.Context, instance *v1alpha1.Account, log *logger.Logger) (*kcptenancyv1alpha.Workspace, error) { ws := &kcptenancyv1alpha.Workspace{} err := r.client.Get(ctx, client.ObjectKey{Name: instance.Name}, ws) if err != nil { - log.Error().Msg("workspace does not exist") - return nil, err + const msg = "workspace does not exist" + log.Error().Msg(msg) + return nil, errors.Wrap(err, msg) } return ws, nil } diff --git a/pkg/subroutines/accountinfo_test.go b/pkg/subroutines/accountinfo_test.go index a0bb2c0..41c1217 100644 --- a/pkg/subroutines/accountinfo_test.go +++ b/pkg/subroutines/accountinfo_test.go @@ -5,7 +5,7 @@ import ( "fmt" "testing" - kcpcorev1alpha "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" kcptenancyv1alpha "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" openmfpcontext "github.com/openmfp/golang-commons/context" "github.com/openmfp/golang-commons/logger" @@ -44,7 +44,7 @@ func (suite *AccountInfoSubroutineTestSuite) SetupTest() { suite.clientMock = new(mocks.Client) // Initialize Tested Object(s) - suite.testObj = subroutines.NewAccountInfoSubroutine(suite.clientMock) + suite.testObj = subroutines.NewAccountInfoSubroutine(suite.clientMock, "some-ca") utilruntime.Must(v1alpha1.AddToScheme(scheme.Scheme)) utilruntime.Must(corev1.AddToScheme(scheme.Scheme)) @@ -75,16 +75,21 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_ForOrganization() Name: "account", }, Spec: v1alpha1.AccountInfoSpec{ + ClusterInfo: v1alpha1.ClusterInfo{ + CA: "some-ca", + }, Organization: v1alpha1.AccountLocation{ Name: "root-org", ClusterId: "some-cluster-id-root-org", Path: "root:openmfp:orgs:root-org", + URL: "https://example.com/root:openmfp:orgs:root-org", Type: "org", }, Account: v1alpha1.AccountLocation{ Name: "root-org", ClusterId: "some-cluster-id-root-org", Path: "root:openmfp:orgs:root-org", + URL: "https://example.com/root:openmfp:orgs:root-org", Type: "org", }, FGA: v1alpha1.FGAInfo{ @@ -95,16 +100,38 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_ForOrganization() }, } - suite.mockGetWorkspaceByName("root-org") - suite.mockGetLogicalClusterByName("root:openmfp:orgs", "some-cluster-id") + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseReady, "root:openmfp:orgs:root-org") suite.mockGetAccountInfoCallNotFound() suite.mockCreateAccountInfoCall(expectedAccountInfo) // When - _, err := suite.testObj.Process(suite.context, testAccount) + res, err := suite.testObj.Process(suite.context, testAccount) // Then suite.Nil(err) + suite.False(res.Requeue) + suite.clientMock.AssertExpectations(suite.T()) +} + +func (suite *AccountInfoSubroutineTestSuite) TestProcessing_ForOrganization_Workspace_Not_Ready() { + // Given + testAccount := &v1alpha1.Account{ + ObjectMeta: v1.ObjectMeta{ + Name: "root-org", + }, + Spec: v1alpha1.AccountSpec{ + Type: v1alpha1.AccountTypeOrg, + }, + } + + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseInitializing, "root:openmfp:orgs") + + // When + res, err := suite.testObj.Process(suite.context, testAccount) + + // Then + suite.Nil(err) + suite.True(res.Requeue) suite.clientMock.AssertExpectations(suite.T()) } @@ -126,6 +153,7 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_ForOrganization_No_W // Then suite.NotNil(err) + suite.Equal("workspace does not exist: \"\" not found", err.Err().Error()) suite.Error(err.Err()) suite.True(err.Retry()) suite.True(err.Sentry()) @@ -142,23 +170,21 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_No_Path() { Type: v1alpha1.AccountTypeOrg, }, } - - suite.mockGetWorkspaceByName("root-org") - suite.mockGetLogicalClusterByName("", "some-cluster-id") + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseReady, "") // When _, err := suite.testObj.Process(suite.context, testAccount) // Then suite.NotNil(err) - suite.Equal("LogicalCluster does not have a path annotation", err.Err().Error()) + suite.Equal("workspace URL is empty", err.Err().Error()) suite.Error(err.Err()) suite.True(err.Retry()) suite.True(err.Sentry()) suite.clientMock.AssertExpectations(suite.T()) } -func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_Path_Lookup_Failed() { +func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_Empty_Path() { // Given testAccount := &v1alpha1.Account{ ObjectMeta: v1.ObjectMeta{ @@ -168,16 +194,38 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_Path_Lookup_Faile Type: v1alpha1.AccountTypeOrg, }, } + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseReady, " ") - suite.mockGetWorkspaceByName("root-org") - suite.mockGetLogicalClusterNotFound() + // When + _, err := suite.testObj.Process(suite.context, testAccount) + + // Then + suite.NotNil(err) + suite.Equal("workspace URL is empty", err.Err().Error()) + suite.Error(err.Err()) + suite.True(err.Retry()) + suite.True(err.Sentry()) + suite.clientMock.AssertExpectations(suite.T()) +} + +func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_Invalid_Path() { + // Given + testAccount := &v1alpha1.Account{ + ObjectMeta: v1.ObjectMeta{ + Name: "root-org", + }, + Spec: v1alpha1.AccountSpec{ + Type: v1alpha1.AccountTypeOrg, + }, + } + suite.mockGetWorkspaceByWrongPath(kcpcorev1alpha1.LogicalClusterPhaseReady) // When _, err := suite.testObj.Process(suite.context, testAccount) // Then suite.NotNil(err) - suite.Equal("logicalCluster does not yet exist: \"\" not found", err.Err().Error()) + suite.Equal("workspace URL is invalid", err.Err().Error()) suite.Error(err.Err()) suite.True(err.Retry()) suite.True(err.Sentry()) @@ -204,17 +252,20 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_ForAccount() { ClusterId: "some-cluster-id-root-org", Path: "root:openmfp:orgs:root-org", Type: "org", + URL: "https://example.com/root:openmfp:orgs:root-org", }, Account: v1alpha1.AccountLocation{ Name: "example-account", ClusterId: "some-cluster-id-example-account", Path: "root:openmfp:orgs:root-org:example-account", Type: "account", + URL: "https://example.com/root:openmfp:orgs:root-org:example-account", }, ParentAccount: &v1alpha1.AccountLocation{ Name: "root-org", ClusterId: "some-cluster-id-root-org", Path: "root:openmfp:orgs:root-org", + URL: "https://example.com/root:openmfp:orgs:root-org", Type: "org", }, FGA: v1alpha1.FGAInfo{ @@ -225,8 +276,7 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_OK_ForAccount() { }, } - suite.mockGetWorkspaceByName("example-account") - suite.mockGetLogicalClusterByName("root:openmfp:orgs:root-org", "some-cluster-id-2") + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseReady, "root:openmfp:orgs:root-org:example-account") parentAccountInfoSpec := v1alpha1.AccountInfoSpec{ Organization: expectedAccountInfo.Spec.Organization, ParentAccount: nil, @@ -255,8 +305,7 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_ForAccount_No_Parent }, } - suite.mockGetWorkspaceByName("example-account") - suite.mockGetLogicalClusterByName("root:openmfp:orgs:root-org", "some-cluster-id-2") + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseReady, "root:openmfp:orgs:root-org") suite.mockGetAccountInfoCallNotFound() // When @@ -282,8 +331,7 @@ func (suite *AccountInfoSubroutineTestSuite) TestProcessing_ForAccount_Parent_Lo }, } - suite.mockGetWorkspaceByName("example-account") - suite.mockGetLogicalClusterByName("root:openmfp:orgs:root-org", "some-cluster-id-2") + suite.mockGetWorkspaceByName(kcpcorev1alpha1.LogicalClusterPhaseReady, "root:openmfp:orgs:root-org") suite.mockGetAccountInfoCallFailed() // When @@ -348,7 +396,26 @@ func (suite *AccountInfoSubroutineTestSuite) mockCreateAccountInfoCall(info v1al Return(nil) } -func (suite *AccountInfoSubroutineTestSuite) mockGetWorkspaceByName(name string) *mocks.Client_Get_Call { +func (suite *AccountInfoSubroutineTestSuite) mockGetWorkspaceByName(ready kcpcorev1alpha1.LogicalClusterPhaseType, path string) *mocks.Client_Get_Call { + return suite.clientMock.EXPECT(). + Get(mock.Anything, mock.Anything, mock.AnythingOfType("*v1alpha1.Workspace")). + Run(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) { + wsPath := "" + if path != "" { + wsPath = "https://example.com/" + path + } + actual, _ := obj.(*kcptenancyv1alpha.Workspace) + actual.Name = key.Name + actual.Spec = kcptenancyv1alpha.WorkspaceSpec{ + Cluster: "some-cluster-id-" + key.Name, + URL: wsPath, + } + actual.Status.Phase = ready + }). + Return(nil) +} + +func (suite *AccountInfoSubroutineTestSuite) mockGetWorkspaceByWrongPath(ready kcpcorev1alpha1.LogicalClusterPhaseType) *mocks.Client_Get_Call { return suite.clientMock.EXPECT(). Get(mock.Anything, mock.Anything, mock.AnythingOfType("*v1alpha1.Workspace")). Run(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) { @@ -356,7 +423,9 @@ func (suite *AccountInfoSubroutineTestSuite) mockGetWorkspaceByName(name string) actual.Name = key.Name actual.Spec = kcptenancyv1alpha.WorkspaceSpec{ Cluster: "some-cluster-id-" + key.Name, + URL: "asd", } + actual.Status.Phase = ready }). Return(nil) } @@ -377,26 +446,3 @@ func (suite *AccountInfoSubroutineTestSuite) mockGetAccountInfo(spec v1alpha1.Ac }). Return(nil) } - -func (suite *AccountInfoSubroutineTestSuite) mockGetLogicalClusterByName(path string, clusterid string) *mocks.Client_Get_Call { - return suite.clientMock.EXPECT(). - Get(mock.Anything, mock.Anything, mock.AnythingOfType("*v1alpha1.LogicalCluster")). - Run(func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) { - actual, _ := obj.(*kcpcorev1alpha.LogicalCluster) - actual.Name = key.Name - actual.ObjectMeta.Annotations = map[string]string{} - if path != "" { - actual.ObjectMeta.Annotations["kcp.io/path"] = path - } - if clusterid != "" { - actual.ObjectMeta.Annotations["kcp.io/cluster"] = clusterid - } - }). - Return(nil) -} - -func (suite *AccountInfoSubroutineTestSuite) mockGetLogicalClusterNotFound() *mocks.Client_Get_Call { - return suite.clientMock.EXPECT(). - Get(mock.Anything, mock.Anything, mock.AnythingOfType("*v1alpha1.LogicalCluster")). - Return(kerrors.NewNotFound(schema.GroupResource{}, "")) -} diff --git a/pkg/subroutines/fga.go b/pkg/subroutines/fga.go index 3662b5a..56d0091 100644 --- a/pkg/subroutines/fga.go +++ b/pkg/subroutines/fga.go @@ -6,7 +6,6 @@ import ( "regexp" "strings" - "github.com/kcp-dev/logicalcluster/v3" openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/openmfp/golang-commons/controller/lifecycle" "github.com/openmfp/golang-commons/errors" @@ -18,25 +17,20 @@ import ( "sigs.k8s.io/controller-runtime/pkg/kontext" "github.com/openmfp/account-operator/api/v1alpha1" - "github.com/openmfp/account-operator/pkg/service" ) type FGASubroutine struct { fgaClient openfgav1.OpenFGAServiceClient client client.Client - srv service.Servicer - rootNamespace string objectType string parentRelation string creatorRelation string } -func NewFGASubroutine(cl client.Client, fgaClient openfgav1.OpenFGAServiceClient, s service.Servicer, rootNamespace, creatorRelation, parentRealtion, objectType string) *FGASubroutine { +func NewFGASubroutine(cl client.Client, fgaClient openfgav1.OpenFGAServiceClient, creatorRelation, parentRealtion, objectType string) *FGASubroutine { return &FGASubroutine{ client: cl, fgaClient: fgaClient, - srv: s, - rootNamespace: rootNamespace, creatorRelation: creatorRelation, parentRelation: parentRealtion, objectType: objectType, @@ -54,26 +48,34 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim return ctrl.Result{}, nil } - storeId, err := e.getStoreId(ctx, account) + parentAccountInfo, err := e.getAccountInfo(ctx) if err != nil { log.Error().Err(err).Msg("Couldn't get Store Id") return ctrl.Result{}, errors.NewOperatorError(err, true, true) } + if parentAccountInfo.Spec.FGA.Store.Id == "" { + log.Error().Msg("FGA Store Id is empty") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("FGA Store Id is empty"), true, true) + } + + clusterId, ok := kontext.ClusterFrom(ctx) + if !ok { + log.Error().Msg("Couldn't get Cluster Id") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("couldn't get cluster id"), true, true) + } + writes := []*openfgav1.TupleKey{} - // Determine parent account to create parent relation - if account.GetNamespace() != e.rootNamespace { - parent, _, err := getParentAccount(ctx, e.client, account.GetNamespace()) - if err != nil { - log.Error().Err(err).Msg("Couldn't get parent account") - return ctrl.Result{}, errors.NewOperatorError(err, true, true) - } + // Parent Name + if account.Spec.Type != v1alpha1.AccountTypeOrg { + parentAccountName := parentAccountInfo.Spec.Account.Name + // Determine parent account to create parent relation writes = append(writes, &openfgav1.TupleKey{ - Object: fmt.Sprintf("%s:%s", e.objectType, account.GetName()), + Object: fmt.Sprintf("%s:%s/%s", e.objectType, clusterId, account.GetName()), Relation: e.parentRelation, - User: fmt.Sprintf("%s:%s", e.objectType, parent.GetName()), + User: fmt.Sprintf("%s:%s/%s", e.objectType, clusterId, parentAccountName), }) } @@ -86,22 +88,21 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim creator := formatUser(*account.Spec.Creator) writes = append(writes, &openfgav1.TupleKey{ - Object: fmt.Sprintf("role:%s/%s/owner", account.Spec.Type, account.Name), + Object: fmt.Sprintf("role:%s/%s/owner", clusterId, account.Name), Relation: "assignee", User: fmt.Sprintf("user:%s", creator), }) writes = append(writes, &openfgav1.TupleKey{ - Object: fmt.Sprintf("%s:%s", e.objectType, account.Name), + Object: fmt.Sprintf("%s:%s/%s", e.objectType, clusterId, account.Name), Relation: e.creatorRelation, - User: fmt.Sprintf("role:%s/%s/owner#assignee", account.Spec.Type, account.Name), + User: fmt.Sprintf("role:%s/%s/owner#assignee", clusterId, account.Name), }) } for _, writeTuple := range writes { - _, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{ - StoreId: storeId, + StoreId: parentAccountInfo.Spec.FGA.Store.Id, Writes: &openfgav1.WriteRequestWrites{ TupleKeys: []*openfgav1.TupleKey{writeTuple}, }, @@ -125,38 +126,44 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti account := runtimeObj.(*v1alpha1.Account) log := logger.LoadLoggerFromContext(ctx) - storeId, err := e.getStoreId(ctx, account) + parentAccountInfo, err := e.getAccountInfo(ctx) if err != nil { log.Error().Err(err).Msg("Couldn't get Store Id") return ctrl.Result{}, errors.NewOperatorError(err, true, true) } - deletes := []*openfgav1.TupleKeyWithoutCondition{} + if parentAccountInfo.Spec.FGA.Store.Id == "" { + log.Error().Msg("FGA Store Id is empty") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("FGA Store Id is empty"), true, true) + } - if account.GetNamespace() != e.rootNamespace { - parent, _, err := getParentAccount(ctx, e.client, account.GetNamespace()) - if err != nil { - log.Error().Err(err).Msg("Couldn't get parent account") - return ctrl.Result{}, errors.NewOperatorError(err, true, true) - } + clusterId, ok := kontext.ClusterFrom(ctx) + if !ok { + log.Error().Msg("Couldn't get Cluster Id") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("couldn't get cluster id"), true, true) + } + + deletes := []*openfgav1.TupleKeyWithoutCondition{} + if account.Spec.Type != v1alpha1.AccountTypeOrg { + parentAccountName := parentAccountInfo.Spec.Account.Name deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - Object: fmt.Sprintf("%s:%s", e.objectType, account.GetName()), + Object: fmt.Sprintf("%s:%s/%s", e.objectType, clusterId, account.GetName()), Relation: e.parentRelation, - User: fmt.Sprintf("%s:%s", e.objectType, parent.GetName()), + User: fmt.Sprintf("%s:%s/%s", e.objectType, clusterId, parentAccountName), }) } if account.Spec.Creator != nil { creator := formatUser(*account.Spec.Creator) deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - Object: fmt.Sprintf("role:%s/%s/owner", account.Spec.Type, account.Name), + Object: fmt.Sprintf("role:%s/%s/owner", clusterId, account.Name), Relation: "assignee", User: fmt.Sprintf("user:%s", creator), }) deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - Object: fmt.Sprintf("%s:%s", e.objectType, account.Name), + Object: fmt.Sprintf("%s:%s/%s", e.objectType, clusterId, account.Name), Relation: e.creatorRelation, User: fmt.Sprintf("role:%s/%s/owner#assignee", account.Spec.Type, account.Name), }) @@ -165,7 +172,7 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti for _, deleteTuple := range deletes { _, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{ - StoreId: storeId, + StoreId: parentAccountInfo.Spec.FGA.Store.Id, Deletes: &openfgav1.WriteRequestDeletes{ TupleKeys: []*openfgav1.TupleKeyWithoutCondition{deleteTuple}, }, @@ -186,37 +193,14 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti return ctrl.Result{}, nil } -func (e *FGASubroutine) getStoreId(ctx context.Context, account *v1alpha1.Account) (string, error) { - firstLevelAccountName := account.Name - - if e.rootNamespace != account.Namespace { - - lookupNamespace := account.Namespace - lookupCtx := ctx - for { - parent, newClusterContext, err := getParentAccount(lookupCtx, e.client, lookupNamespace) - if errors.Is(err, ErrNoParentAvailable) { - break - } - if err != nil { - return "", err - } - - if newClusterContext != nil { - lookupCtx = kontext.WithCluster(lookupCtx, logicalcluster.Name(*newClusterContext)) - } - - lookupNamespace = parent.GetNamespace() - firstLevelAccountName = parent.GetName() - } - } - - storeId, err := helpers.GetStoreIDForTenant(ctx, e.fgaClient, firstLevelAccountName) +func (e *FGASubroutine) getAccountInfo(ctx context.Context) (*v1alpha1.AccountInfo, error) { + // Get AccountInfo For Project + accountInfo := &v1alpha1.AccountInfo{} + err := e.client.Get(ctx, client.ObjectKey{Name: DefaultAccountInfoName}, accountInfo) if err != nil { - return "", err + return nil, err } - - return storeId, nil + return accountInfo, nil } func (e *FGASubroutine) GetName() string { return "CreatorSubroutine" } diff --git a/pkg/subroutines/fga_test.go b/pkg/subroutines/fga_test.go index 20ef3d6..f8f747b 100644 --- a/pkg/subroutines/fga_test.go +++ b/pkg/subroutines/fga_test.go @@ -4,10 +4,9 @@ import ( "context" "testing" - corev1 "k8s.io/api/core/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/kontext" openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/stretchr/testify/assert" @@ -18,7 +17,6 @@ import ( "k8s.io/utils/ptr" "github.com/openmfp/account-operator/api/v1alpha1" - corev1alpha1 "github.com/openmfp/account-operator/api/v1alpha1" "github.com/openmfp/account-operator/pkg/subroutines" "github.com/openmfp/account-operator/pkg/subroutines/mocks" ) @@ -41,65 +39,16 @@ func newFgaError(c openfgav1.ErrorCode, m string) *fgaError { } func TestCreatorSubroutine_GetName(t *testing.T) { - routine := subroutines.NewFGASubroutine(nil, nil, nil, "", "", "", "") + routine := subroutines.NewFGASubroutine(nil, nil, "", "", "") assert.Equal(t, "CreatorSubroutine", routine.GetName()) } func TestCreatorSubroutine_Finalizers(t *testing.T) { - routine := subroutines.NewFGASubroutine(nil, nil, nil, "", "", "", "") + routine := subroutines.NewFGASubroutine(nil, nil, "", "", "") assert.Equal(t, []string{"account.core.openmfp.org/fga"}, routine.Finalizers()) } -func getStoreMocks(openFGAServiceClientMock *mocks.OpenFGAServiceClient, clientMock *mocks.Client) { - - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - - *account = v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "first-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, - }, - }, - } - - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - return nil - }).Once() - - openFGAServiceClientMock.EXPECT().ListStores(context.Background(), mock.Anything).Return(&openfgav1.ListStoresResponse{Stores: []*openfgav1.Store{{Id: "1", Name: "tenant-first-level"}}}, nil).Maybe() -} - func TestCreatorSubroutine_Process(t *testing.T) { - namespace := "test-openmfp-namespace" creator := "test-creator" testCases := []struct { @@ -132,43 +81,30 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) + account := o.(*v1alpha1.AccountInfo) - *ns = corev1.Namespace{ + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - - *account = v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT().ListStores(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, }, { @@ -181,19 +117,6 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) }, }, @@ -207,45 +130,33 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). Return(nil, assert.AnError) @@ -261,45 +172,33 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). Return(nil, newFgaError(openfgav1.ErrorCode_write_failed_due_to_invalid_input, "error")) @@ -314,45 +213,33 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). Return(&openfgav1.WriteResponse{}, nil) @@ -370,45 +257,33 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). Return(&openfgav1.WriteResponse{}, nil) @@ -434,38 +309,28 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } @@ -486,44 +351,33 @@ func TestCreatorSubroutine_Process(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -543,21 +397,21 @@ func TestCreatorSubroutine_Process(t *testing.T) { test.setupMocks(openFGAClient, accountClient, clientMock) } - routine := subroutines.NewFGASubroutine(clientMock, openFGAClient, accountClient, namespace, "owner", "parent", "account") - ctx := context.Background() + routine := subroutines.NewFGASubroutine(clientMock, openFGAClient, "owner", "parent", "account") + ctx := kontext.WithCluster(context.Background(), "abcdefghi") _, err := routine.Process(ctx, test.account) if test.expectedError { assert.NotNil(t, err) } else { assert.Nil(t, err) } + clientMock.AssertExpectations(t) }) } } func TestCreatorSubroutine_Finalize(t *testing.T) { - namespace := "test-openmfp-namespace" creator := "test-creator" testCases := []struct { @@ -577,43 +431,30 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) + account := o.(*v1alpha1.AccountInfo) - *ns = corev1.Namespace{ + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - - *account = v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT().ListStores(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, }, { @@ -626,19 +467,6 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) }, }, @@ -652,46 +480,36 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - getStoreMocks(openFGAServiceClientMock, clientMock) + account := o.(*v1alpha1.AccountInfo) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - - *account = v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError) + openFGAServiceClientMock.EXPECT(). + Write(mock.Anything, mock.Anything). + Return(nil, assert.AnError) }, }, @@ -704,45 +522,33 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) - openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). Return(nil, newFgaError(openfgav1.ErrorCode_write_failed_due_to_invalid_input, "error")) @@ -757,44 +563,33 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -813,44 +608,33 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { }, }, setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { - getStoreMocks(openFGAServiceClientMock, clientMock) - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - ns := o.(*corev1.Namespace) - *ns = corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - corev1alpha1.NamespaceAccountOwnerLabel: "first-level", - corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", - }, - }, - } - return nil - }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - account := o.(*v1alpha1.Account) - *account = v1alpha1.Account{ + account := o.(*v1alpha1.AccountInfo) + + *account = v1alpha1.AccountInfo{ ObjectMeta: metav1.ObjectMeta{ - Name: "fist-level", - Namespace: "first-level", - }, - Spec: v1alpha1.AccountSpec{ - Extensions: []v1alpha1.Extension{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "AccountExtension", - APIVersion: "core.openmfp.org/v1alpha1", - }, - SpecGoTemplate: apiextensionsv1.JSON{}, - }, + Name: "root-org", + }, + Spec: v1alpha1.AccountInfoSpec{ + Organization: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, }, + Account: v1alpha1.AccountLocation{ + Name: "root-org", + Path: "root:openmfp:org:root-org", + URL: "http://example.com/clusters/root:openmfp:org:root-org", + Type: v1alpha1.AccountTypeOrg, + }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "123123"}}, }, } return nil }).Once() - clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -870,8 +654,8 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { test.setupMocks(openFGAClient, accountClient, k8sClient) } - routine := subroutines.NewFGASubroutine(k8sClient, openFGAClient, accountClient, namespace, "owner", "parent", "account") - ctx := context.Background() + routine := subroutines.NewFGASubroutine(k8sClient, openFGAClient, "owner", "parent", "account") + ctx := kontext.WithCluster(context.Background(), "abcdefghi") _, err := routine.Finalize(ctx, test.account) if test.expectedError { assert.NotNil(t, err) diff --git a/pkg/testing/kcpenvtest/server.go b/pkg/testing/kcpenvtest/server.go index 51b07ef..e40d836 100644 --- a/pkg/testing/kcpenvtest/server.go +++ b/pkg/testing/kcpenvtest/server.go @@ -13,6 +13,7 @@ import ( "github.com/openmfp/golang-commons/logger" "github.com/otiai10/copy" + "github.com/rs/zerolog/log" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -30,7 +31,6 @@ import ( const ( kcpEnvStartTimeout = "KCP_SERVER_START_TIMEOUT" kcpEnvStopTimeout = "KCP_SERVER_STOP_TIMEOUT" - defaulKCPServerTimeout = 20 * time.Minute defaultKCPServerTimeout = 20 * time.Minute kcpAdminKubeconfigPath = ".kcp/admin.kubeconfig" kcpRootNamespaceServerUrl = "https://localhost:6443/clusters/root" @@ -62,6 +62,7 @@ type Environment struct { func NewEnvironment(apiExportEndpointSliceName string, providerWorkspaceName string, pathToRoot string, relativeAssetDirectory string, relativeSetupDirectory string, log *logger.Logger) *Environment { kcpBinary := filepath.Join(relativeAssetDirectory, "kcp") kcpServ := NewKCPServer(pathToRoot, kcpBinary, pathToRoot, log) + //kcpServ.Out = os.Stdout //kcpServ.Err = os.Stderr return &Environment{ @@ -87,8 +88,8 @@ func (te *Environment) Start(useExsiting bool) (*rest.Config, string, error) { if err := te.defaultTimeouts(); err != nil { return nil, "", fmt.Errorf("failed to default controlplane timeouts: %w", err) } - //te.kcpServer.StartTimeout = te.ControlPlaneStartTimeout - //te.kcpServer.StopTimeout = te.ControlPlaneStopTimeout + te.kcpServer.StartTimeout = te.ControlPlaneStartTimeout + te.kcpServer.StopTimeout = te.ControlPlaneStopTimeout te.log.Info().Msg("starting control plane") if err := te.kcpServer.Start(); err != nil { @@ -184,7 +185,7 @@ func (te *Environment) waitForDefaultNamespace() error { func (te *Environment) waitForWorkspace(client client.Client, name string, log *logger.Logger) error { // It shouldn't take longer than 5s for the default namespace to be brought up in etcd - return wait.PollUntilContextTimeout(context.TODO(), time.Millisecond*500, time.Second*15, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(context.TODO(), time.Millisecond*500, time.Second*15, true, func(ctx context.Context) (bool, error) { ws := &kcptenancyv1alpha.Workspace{} if err := client.Get(ctx, types.NamespacedName{Name: name}, ws); err != nil { return false, nil //nolint:nilerr @@ -193,6 +194,11 @@ func (te *Environment) waitForWorkspace(client client.Client, name string, log * log.Info().Str("workspace", name).Bool("ready", ready).Msg("waiting for workspace to be ready") return ready, nil }) + + if err != nil { + return fmt.Errorf("workspace %s did not become ready: %w", name, err) + } + return err } func (te *Environment) defaultTimeouts() error { @@ -204,7 +210,7 @@ func (te *Environment) defaultTimeouts() error { return err } } else { - te.kcpServer.StartTimeout = defaulKCPServerTimeout + te.ControlPlaneStartTimeout = defaultKCPServerTimeout } } @@ -373,6 +379,7 @@ func (te *Environment) ApplyYAML(pathToRootConfig string, config *rest.Config, p } } } + log.Info().Msg("finished applying setup") return nil } diff --git a/test/setup/01-openmfp-system/apiexport-core.openmfp.org.yaml b/test/setup/01-openmfp-system/apiexport-core.openmfp.org.yaml index 62d2c46..e8d5af0 100644 --- a/test/setup/01-openmfp-system/apiexport-core.openmfp.org.yaml +++ b/test/setup/01-openmfp-system/apiexport-core.openmfp.org.yaml @@ -5,8 +5,8 @@ metadata: name: core.openmfp.org spec: latestResourceSchemas: - - v250220-cdcd05c.accounts.core.openmfp.org - - v250224-cafdd19.accountinfos.core.openmfp.org + - v250225-f1af48a.accountinfos.core.openmfp.org + - v250225-f1af48a.accounts.core.openmfp.org permissionClaims: - all: true resource: namespaces diff --git a/test/setup/01-openmfp-system/apiresourceschema-AccountInfos.core.openmfp.org.yaml b/test/setup/01-openmfp-system/apiresourceschema-accountinfos.core.openmfp.org.yaml similarity index 86% rename from test/setup/01-openmfp-system/apiresourceschema-AccountInfos.core.openmfp.org.yaml rename to test/setup/01-openmfp-system/apiresourceschema-accountinfos.core.openmfp.org.yaml index 2f40013..b1ccfa4 100644 --- a/test/setup/01-openmfp-system/apiresourceschema-AccountInfos.core.openmfp.org.yaml +++ b/test/setup/01-openmfp-system/apiresourceschema-accountinfos.core.openmfp.org.yaml @@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1 kind: APIResourceSchema metadata: creationTimestamp: null - name: v250224-cafdd19.accountinfos.core.openmfp.org + name: v250225-f1af48a.accountinfos.core.openmfp.org spec: group: core.openmfp.org names: @@ -10,7 +10,7 @@ spec: listKind: AccountInfoList plural: accountinfos singular: accountinfo - scope: Namespaced + scope: Cluster versions: - name: v1alpha1 schema: @@ -46,11 +46,21 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url + type: object + clusterInfo: + properties: + ca: + type: string + required: + - ca type: object fga: properties: @@ -74,11 +84,14 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url type: object parentAccount: properties: @@ -90,17 +103,20 @@ spec: type: string type: type: string + url: + type: string required: - clusterId - name - path - type + - url type: object required: - account + - clusterInfo - fga - organization - - parentAccount type: object status: description: AccountInfoStatus defines the observed state of Account diff --git a/test/setup/01-openmfp-system/apiresourceschema-accounts.core.openmfp.org.yaml b/test/setup/01-openmfp-system/apiresourceschema-accounts.core.openmfp.org.yaml index a8e1516..7bb9b1f 100644 --- a/test/setup/01-openmfp-system/apiresourceschema-accounts.core.openmfp.org.yaml +++ b/test/setup/01-openmfp-system/apiresourceschema-accounts.core.openmfp.org.yaml @@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1 kind: APIResourceSchema metadata: creationTimestamp: null - name: v250220-cdcd05c.accounts.core.openmfp.org + name: v250225-f1af48a.accounts.core.openmfp.org spec: group: core.openmfp.org names: