Skip to content

Commit

Permalink
fix: Fixing ParentAccount lookup and adjusting tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@nexus49
nexus49 committed Jan 18, 2025
1 parent 68c1cfb commit 436638a
Show file tree
Hide file tree
Showing 4 changed files with 588 additions and 142 deletions.
4 changes: 2 additions & 2 deletions internal/controller/account_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ func NewAccountReconciler(log *logger.Logger, mgr ctrl.Manager, cfg config.Confi

srv := service.NewService(mgr.GetClient(), cfg.Subroutines.FGA.RootNamespace)

cl := openfgav1.NewOpenFGAServiceClient(conn)
fgaClient := openfgav1.NewOpenFGAServiceClient(conn)

subs = append(subs, subroutines.NewFGASubroutine(cl, 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, srv, cfg.Subroutines.FGA.RootNamespace, 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(),
Expand Down
64 changes: 41 additions & 23 deletions pkg/subroutines/extensions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"strings"
"text/template"

Expand Down Expand Up @@ -196,41 +197,58 @@ func collectExtensions(ctx context.Context, cl client.Client, lookupNamespace st
}

func getParentAccount(ctx context.Context, cl client.Client, ns string) (*v1alpha1.Account, *string, error) {
if _, ok := kontext.ClusterFrom(ctx); ok {
return getParentAccountWithKcp(ctx, cl)
} else {
return getParentAccountByNs(ctx, cl, ns)
}
}

if cluster, ok := kontext.ClusterFrom(ctx); ok && !cluster.Empty() {
func getParentAccountWithKcp(ctx context.Context, cl client.Client) (*v1alpha1.Account, *string, error) {

wsCtx := kontext.WithCluster(ctx, logicalcluster.Name(""))
list := &tenancyv1alpha1.WorkspaceList{}
cluster, ok := kontext.ClusterFrom(ctx)
if !ok {
return nil, nil, fmt.Errorf("no cluster context found, this is a configuration error")
}

err := cl.List(wsCtx, list)
if err != nil {
return nil, nil, err
}
if cluster.Empty() {
return nil, nil, fmt.Errorf("no cluster in context is empty")
}

for _, ws := range list.Items {
if ws.Spec.Cluster != cluster.String() {
continue
}
wsCtx := kontext.WithCluster(ctx, "")
list := &tenancyv1alpha1.WorkspaceList{}

err := cl.List(wsCtx, list)
if err != nil {
return nil, nil, err
}

clusterName := ws.Annotations[logicalcluster.AnnotationKey]
for _, ws := range list.Items {
if ws.Spec.Cluster != cluster.String() {
continue
}

parentCtx := kontext.WithCluster(ctx, logicalcluster.Name(clusterName))
clusterName := ws.Annotations[logicalcluster.AnnotationKey]

parentAccount := v1alpha1.Account{}
err = cl.Get(parentCtx, types.NamespacedName{
Name: ws.Annotations[v1alpha1.NamespaceAccountOwnerLabel],
Namespace: ns,
}, &parentAccount)
if err != nil {
return nil, nil, err
}
parentCtx := kontext.WithCluster(ctx, logicalcluster.Name(clusterName))

return &parentAccount, &clusterName, nil
parentAccount := v1alpha1.Account{}
err = cl.Get(parentCtx, types.NamespacedName{
Name: ws.Annotations[v1alpha1.NamespaceAccountOwnerLabel],
Namespace: ws.Annotations[v1alpha1.NamespaceAccountOwnerNamespaceLabel],
}, &parentAccount)
if err != nil {
return nil, nil, err
}

return nil, nil, ErrNoParentAvailable
return &parentAccount, &clusterName, nil
}

return nil, nil, ErrNoParentAvailable
}

func getParentAccountByNs(ctx context.Context, cl client.Client, ns string) (*v1alpha1.Account, *string, error) {

var namespace v1.Namespace
err := cl.Get(ctx, types.NamespacedName{Name: ns}, &namespace)
if kerrors.IsNotFound(err) {
Expand Down
21 changes: 12 additions & 9 deletions pkg/subroutines/fga.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,19 @@ import (
)

type FGASubroutine struct {
client openfgav1.OpenFGAServiceClient
k8sClient client.Client
fgaClient openfgav1.OpenFGAServiceClient
client client.Client
srv service.Servicer
rootNamespace string
objectType string
parentRelation string
creatorRelation string
}

func NewFGASubroutine(cl openfgav1.OpenFGAServiceClient, s service.Servicer, rootNamespace, creatorRelation, parentRealtion, objectType string) *FGASubroutine {
func NewFGASubroutine(cl client.Client, fgaClient openfgav1.OpenFGAServiceClient, s service.Servicer, rootNamespace, creatorRelation, parentRealtion, objectType string) *FGASubroutine {
return &FGASubroutine{
client: cl,
fgaClient: fgaClient,
srv: s,
rootNamespace: rootNamespace,
creatorRelation: creatorRelation,
Expand Down Expand Up @@ -61,8 +62,9 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim

writes := []*openfgav1.TupleKey{}

// Determine parent account to create parent relation
if account.GetNamespace() != e.rootNamespace {
parent, err := e.srv.GetAccountForNamespace(ctx, account.GetNamespace())
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)
Expand All @@ -75,6 +77,7 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim
})
}

// Assign creator to the account
if account.Spec.Creator != nil {
if valid := validateCreator(*account.Spec.Creator); !valid {
log.Error().Err(err).Str("creator", *account.Spec.Creator).Msg("creator string is in the protected service account prefix range")
Expand All @@ -97,7 +100,7 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim

for _, writeTuple := range writes {

_, err = e.client.Write(ctx, &openfgav1.WriteRequest{
_, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{
StoreId: storeId,
Writes: &openfgav1.WriteRequestWrites{
TupleKeys: []*openfgav1.TupleKey{writeTuple},
Expand Down Expand Up @@ -131,7 +134,7 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti
deletes := []*openfgav1.TupleKeyWithoutCondition{}

if account.GetNamespace() != e.rootNamespace {
parent, err := e.srv.GetAccountForNamespace(ctx, account.GetNamespace())
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)
Expand Down Expand Up @@ -161,7 +164,7 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti

for _, deleteTuple := range deletes {

_, err = e.client.Write(ctx, &openfgav1.WriteRequest{
_, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{
StoreId: storeId,
Deletes: &openfgav1.WriteRequestDeletes{
TupleKeys: []*openfgav1.TupleKeyWithoutCondition{deleteTuple},
Expand Down Expand Up @@ -191,7 +194,7 @@ func (e *FGASubroutine) getStoreId(ctx context.Context, account *v1alpha1.Accoun
lookupNamespace := account.Namespace
lookupCtx := ctx
for {
parent, newClusterContext, err := getParentAccount(lookupCtx, e.k8sClient, lookupNamespace)
parent, newClusterContext, err := getParentAccount(lookupCtx, e.client, lookupNamespace)
if errors.Is(err, ErrNoParentAvailable) {
break
}
Expand All @@ -208,7 +211,7 @@ func (e *FGASubroutine) getStoreId(ctx context.Context, account *v1alpha1.Accoun
}
}

storeId, err := helpers.GetStoreIDForTenant(ctx, e.client, firstLevelAccountName)
storeId, err := helpers.GetStoreIDForTenant(ctx, e.fgaClient, firstLevelAccountName)
if err != nil {
return "", err
}
Expand Down
Loading

0 comments on commit 436638a

Please sign in to comment.