Skip to content

Commit dbc2741

Browse files
fix: updated api binding finalization (#168)
* fix: updated api binding finalization On-behalf-of: SAP [email protected] * updated tests On-behalf-of: SAP [email protected] * linter fixes On-behalf-of: SAP [email protected] * fixed typo On-behalf-of: SAP [email protected] * fixed continue error On-behalf-of: SAP [email protected] * fix: changed finalizer name and added organization check On-behalf-of: SAP [email protected] * refactored tests On-behalf-of: SAP [email protected] * fixed linter issue On-behalf-of: SAP [email protected] * chore: refactored tests On-behalf-of: SAP [email protected] * feat: introduced integration tests On-behalf-of: SAP [email protected] * Update internal/subroutine/authorization_model_generation_integration_test.go Co-authored-by: Aaron Schweig <[email protected]> * Update internal/subroutine/authorization_model_generation_integration_test.go Co-authored-by: Aaron Schweig <[email protected]> * Update internal/subroutine/authorization_model_generation_integration_test.go Co-authored-by: Aaron Schweig <[email protected]> * chore: removed e-mail On-behalf-of: SAP [email protected] * feat: introduced isolated integration tests On-behalf-of: SAP [email protected] * updated naming On-behalf-of: SAP [email protected] * updated task file On-behalf-of: SAP [email protected] * fixed path to kcp binary On-behalf-of: SAP [email protected] * updated kcp version in task file On-behalf-of: SAP [email protected] * fixed linter errors On-behalf-of: SAP [email protected] * fixed imports On-behalf-of: SAP [email protected] * removed previos intetration tests On-behalf-of: SAP [email protected] * chore: removed package variables On-behalf-of: SAP [email protected] * feat: added api binding controller in tests On-behalf-of: SAP [email protected] * fixed linter error On-behalf-of: SAP [email protected] --------- Co-authored-by: Aaron Schweig <[email protected]>
1 parent eb0145a commit dbc2741

22 files changed

+2160
-433
lines changed

Taskfile.yaml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ vars:
77
ENVTEST_VERSION: release-0.19
88
CRD_DIRECTORY: config/crd/bases
99
KCP_APIGEN_VERSION: v0.21.0
10-
KCP_VERSION: 0.26.1
10+
KCP_VERSION: 0.28.3
1111
GOLANGCI_LINT_VERSION: v2.5.0
1212
GOARCH:
1313
sh: go env GOARCH
@@ -76,15 +76,17 @@ tasks:
7676
GOPRIVATE: github.com/platform-mesh
7777
KUBEBUILDER_ASSETS:
7878
sh: $(pwd)/{{.LOCAL_BIN}}/setup-envtest use {{.ENVTEST_K8S_VERSION}} --bin-dir $(pwd)/{{.LOCAL_BIN}} -p path
79+
TEST_KCP_ASSETS:
80+
sh: echo $(pwd)/{{.LOCAL_BIN}}
7981
GO111MODULE: on
8082
cmds:
8183
- go test -count=1 ./... {{.ADDITIONAL_COMMAND_ARGS}}
8284
test:
83-
deps: [setup:envtest]
85+
deps: [setup:envtest, setup:kcp]
8486
cmds:
8587
- task: envtest
8688
cover:
87-
deps: [setup:envtest]
89+
deps: [setup:envtest, setup:kcp]
8890
cmds:
8991
- task: envtest
9092
vars:

cmd/operator.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ func logicalClusterClientFromKey(mgr ctrl.Manager, log *logger.Logger) NewLogica
5959

6060
parsed.Path = fmt.Sprintf("/clusters/%s", clusterKey)
6161

62-
log.Info().Msg(fmt.Sprintf("HOST from logical cluster client from key -- %s", parsed.String()))
63-
6462
cfg.Host = parsed.String()
6563

6664
return client.New(cfg, client.Options{

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ require (
7070
github.com/json-iterator/go v1.1.12 // indirect
7171
github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250728122101-adbf20db3e51 // indirect
7272
github.com/mailru/easyjson v0.9.0 // indirect
73+
github.com/martinlindhe/base36 v1.1.1 // indirect
7374
github.com/mattn/go-colorable v0.1.14 // indirect
7475
github.com/mattn/go-isatty v0.0.20 // indirect
7576
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect

go.sum

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0
130130
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
131131
github.com/mailru/easyjson v0.9.0 h1:PrnmzHw7262yW8sTBwxi1PdJA3Iw/EKBa8psRf7d9a4=
132132
github.com/mailru/easyjson v0.9.0/go.mod h1:1+xMtQp2MRNVL/V1bOzuP3aP8VNwRW55fQUto+XFtTU=
133+
github.com/martinlindhe/base36 v1.1.1 h1:1F1MZ5MGghBXDZ2KJ3QfxmiydlWOGB8HCEtkap5NkVg=
134+
github.com/martinlindhe/base36 v1.1.1/go.mod h1:vMS8PaZ5e/jV9LwFKlm0YLnXl/hpOihiBxKkIoc3g08=
133135
github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
134136
github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
135137
github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
@@ -208,6 +210,7 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
208210
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
209211
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
210212
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
213+
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
211214
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
212215
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
213216
github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8=

internal/controller/apibinding_controller.go

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,19 @@ package controller
22

33
import (
44
"context"
5+
"net/url"
6+
"strings"
57

68
kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
9+
"github.com/kcp-dev/logicalcluster/v3"
710
platformeshconfig "github.com/platform-mesh/golang-commons/config"
811
"github.com/platform-mesh/golang-commons/controller/lifecycle/builder"
912
lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine"
1013
"github.com/platform-mesh/golang-commons/logger"
14+
"github.com/rs/zerolog/log"
15+
"k8s.io/client-go/rest"
1116
ctrl "sigs.k8s.io/controller-runtime"
17+
"sigs.k8s.io/controller-runtime/pkg/client"
1218
"sigs.k8s.io/controller-runtime/pkg/predicate"
1319

1420
"github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster"
@@ -18,11 +24,46 @@ import (
1824
mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile"
1925
)
2026

27+
func getAllClient(mcMgr mcmanager.Manager) (client.Client, error) {
28+
allCfg := rest.CopyConfig(mcMgr.GetLocalManager().GetConfig())
29+
30+
parsed, err := url.Parse(allCfg.Host)
31+
if err != nil {
32+
log.Error().Err(err).Msg("unable to parse host from config")
33+
return nil, err
34+
}
35+
36+
parts := strings.Split(parsed.Path, "clusters")
37+
38+
parsed.Path, err = url.JoinPath(parts[0], "clusters", logicalcluster.Wildcard.String())
39+
if err != nil {
40+
log.Error().Err(err).Msg("unable to join path")
41+
return nil, err
42+
}
43+
44+
allCfg.Host = parsed.String()
45+
46+
log.Info().Str("host", allCfg.Host).Msg("using host")
47+
48+
allClient, err := client.New(allCfg, client.Options{
49+
Scheme: mcMgr.GetLocalManager().GetScheme(),
50+
})
51+
if err != nil {
52+
return nil, err
53+
}
54+
return allClient, nil
55+
}
56+
2157
func NewAPIBindingReconciler(logger *logger.Logger, mcMgr mcmanager.Manager) *APIBindingReconciler {
58+
allclient, err := getAllClient(mcMgr)
59+
if err != nil {
60+
log.Fatal().Err(err).Msg("unable to create new client")
61+
}
62+
2263
return &APIBindingReconciler{
2364
log: logger,
2465
mclifecycle: builder.NewBuilder("apibinding", "apibinding-controller", []lifecyclesubroutine.Subroutine{
25-
subroutine.NewAuthorizationModelGenerationSubroutine(mcMgr),
66+
subroutine.NewAuthorizationModelGenerationSubroutine(mcMgr, allclient),
2667
}, logger).
2768
BuildMultiCluster(mcMgr),
2869
}

internal/controller/store_controller.go

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@ package controller
22

33
import (
44
"context"
5-
"net/url"
6-
"strings"
75

8-
"github.com/kcp-dev/logicalcluster/v3"
96
"github.com/platform-mesh/golang-commons/controller/lifecycle/builder"
107
"k8s.io/apimachinery/pkg/types"
118
"k8s.io/client-go/discovery"
@@ -37,28 +34,7 @@ type StoreReconciler struct {
3734
}
3835

3936
func NewStoreReconciler(log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager) *StoreReconciler {
40-
41-
allCfg := rest.CopyConfig(mcMgr.GetLocalManager().GetConfig())
42-
43-
parsed, err := url.Parse(allCfg.Host)
44-
if err != nil {
45-
log.Fatal().Err(err).Msg("unable to parse host from config")
46-
}
47-
48-
parts := strings.Split(parsed.Path, "clusters")
49-
50-
parsed.Path, err = url.JoinPath(parts[0], "clusters", logicalcluster.Wildcard.String())
51-
if err != nil {
52-
log.Fatal().Err(err).Msg("unable to join path")
53-
}
54-
55-
allCfg.Host = parsed.String()
56-
57-
log.Info().Str("host", allCfg.Host).Msg("using host")
58-
59-
allClient, err := client.New(allCfg, client.Options{
60-
Scheme: mcMgr.GetLocalManager().GetScheme(),
61-
})
37+
allClient, err := getAllClient(mcMgr)
6238
if err != nil {
6339
log.Fatal().Err(err).Msg("unable to create new client")
6440
}

internal/subroutine/authorization_model_generation.go

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,25 @@ import (
1919
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2020
"k8s.io/apimachinery/pkg/types"
2121
ctrl "sigs.k8s.io/controller-runtime"
22+
"sigs.k8s.io/controller-runtime/pkg/client"
2223
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2324
mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
2425

2526
securityv1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1"
2627
)
2728

28-
func NewAuthorizationModelGenerationSubroutine(mcMgr mcmanager.Manager) *AuthorizationModelGenerationSubroutine {
29+
func NewAuthorizationModelGenerationSubroutine(mcMgr mcmanager.Manager, allClient client.Client) *AuthorizationModelGenerationSubroutine {
2930
return &AuthorizationModelGenerationSubroutine{
30-
mgr: mcMgr,
31+
mgr: mcMgr,
32+
allClient: allClient,
3133
}
3234
}
3335

3436
var _ lifecyclesubroutine.Subroutine = &AuthorizationModelGenerationSubroutine{}
3537

3638
type AuthorizationModelGenerationSubroutine struct {
37-
mgr mcmanager.Manager
39+
mgr mcmanager.Manager
40+
allClient client.Client
3841
}
3942

4043
var modelTpl = template.Must(template.New("model").Parse(`module {{ .Name }}
@@ -86,19 +89,19 @@ func (a *AuthorizationModelGenerationSubroutine) Finalize(ctx context.Context, i
8689

8790
bindingToDelete := instance.(*kcpv1alpha1.APIBinding)
8891

89-
cluster, err := a.mgr.ClusterFromContext(ctx)
92+
bindingCluster, err := a.mgr.ClusterFromContext(ctx)
9093
if err != nil {
9194
return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get cluster from context: %w", err), true, false)
9295
}
9396

9497
var bindings kcpv1alpha1.APIBindingList
95-
err = cluster.GetClient().List(ctx, &bindings)
98+
err = a.allClient.List(ctx, &bindings)
9699
if err != nil {
97100
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
98101
}
99102

100103
var toDeleteAccountInfo accountv1alpha1.AccountInfo
101-
err = cluster.GetClient().Get(ctx, types.NamespacedName{Name: "account"}, &toDeleteAccountInfo)
104+
err = bindingCluster.GetClient().Get(ctx, types.NamespacedName{Name: "account"}, &toDeleteAccountInfo)
102105
if err != nil {
103106
log.Error().Err(err).Msg("unable to get account info for binding deletion")
104107
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
@@ -110,16 +113,16 @@ func (a *AuthorizationModelGenerationSubroutine) Finalize(ctx context.Context, i
110113
continue
111114
}
112115

113-
bindingCluster, err := a.mgr.GetCluster(ctx, string(logicalcluster.From(&binding)))
116+
bindingWsCluster, err := a.mgr.GetCluster(ctx, string(logicalcluster.From(&binding)))
114117
if err != nil {
115118
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
116119
}
117120

118121
var accountInfo accountv1alpha1.AccountInfo
119-
err = bindingCluster.GetClient().Get(ctx, types.NamespacedName{Name: "account"}, &accountInfo)
122+
err = bindingWsCluster.GetClient().Get(ctx, types.NamespacedName{Name: "account"}, &accountInfo)
120123
if kerrors.IsNotFound(err) || meta.IsNoMatchError(err) {
121-
// If the accountinfo does not exist, we can skip the model generation.
122-
return ctrl.Result{}, nil
124+
// If the accountinfo does not exist, skip this binding and continue with others
125+
continue
123126
}
124127
if err != nil {
125128
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
@@ -140,26 +143,52 @@ func (a *AuthorizationModelGenerationSubroutine) Finalize(ctx context.Context, i
140143
return ctrl.Result{}, nil
141144
}
142145

143-
err = cluster.GetClient().Delete(ctx, &securityv1alpha1.AuthorizationModel{
144-
ObjectMeta: metav1.ObjectMeta{
145-
Name: fmt.Sprintf("%s-%s", bindingToDelete.Spec.Reference.Export.Name, bindingToDelete.Spec.Reference.Export.Path),
146-
},
147-
})
146+
apiExportCluster, err := a.mgr.GetCluster(ctx, bindingToDelete.Status.APIExportClusterName)
148147
if err != nil {
149-
if kerrors.IsNotFound(err) {
150-
// If the model does not exist, we can skip the deletion.
151-
return ctrl.Result{}, nil
152-
}
148+
return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get cluster %q: %w", bindingToDelete.Status.APIExportClusterName, err), true, false)
149+
}
150+
apiExportClient := apiExportCluster.GetClient()
151+
152+
var apiExport kcpv1alpha1.APIExport
153+
err = apiExportClient.Get(ctx, types.NamespacedName{Name: bindingToDelete.Spec.Reference.Export.Name}, &apiExport)
154+
if err != nil {
155+
log.Error().Err(err).Msg("failed to get apiexport for binding deletion")
153156
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
154157
}
155158

159+
for _, latestResourceSchema := range apiExport.Spec.LatestResourceSchemas {
160+
var resourceSchema kcpv1alpha1.APIResourceSchema
161+
err := apiExportClient.Get(ctx, types.NamespacedName{Name: latestResourceSchema}, &resourceSchema)
162+
if err != nil {
163+
log.Error().Err(err).Msg("failed to get resource schema for binding deletion")
164+
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
165+
}
166+
167+
authModelName := fmt.Sprintf("%s-%s", resourceSchema.Spec.Names.Plural, toDeleteAccountInfo.Spec.Organization.Name)
168+
err = apiExportClient.Delete(ctx, &securityv1alpha1.AuthorizationModel{
169+
ObjectMeta: metav1.ObjectMeta{
170+
Name: authModelName,
171+
},
172+
})
173+
if err != nil {
174+
if kerrors.IsNotFound(err) {
175+
// If the model does not exist, we can skip the deletion.
176+
log.Info().Msg(fmt.Sprintf("authorization model %s does not exist", authModelName))
177+
continue
178+
}
179+
log.Error().Err(err).Msg("failed to delete authorization model")
180+
return ctrl.Result{}, errors.NewOperatorError(err, true, true)
181+
}
182+
log.Info().Msg(fmt.Sprintf("authorization model %s has been deleted", authModelName))
183+
}
184+
156185
return ctrl.Result{}, nil
157186

158187
}
159188

160189
// Finalizers implements lifecycle.Subroutine.
161190
func (a *AuthorizationModelGenerationSubroutine) Finalizers(_ lifecyclecontrollerruntime.RuntimeObject) []string {
162-
return []string{}
191+
return []string{"core.platform-mesh.io/apibinding-finalizer"}
163192
}
164193

165194
// GetName implements lifecycle.Subroutine.

0 commit comments

Comments
 (0)