Skip to content

Commit 9ed1827

Browse files
cnvergencecnvergence
authored
Merge pull request #455 from cnvergence/add-owner-refs-bound-schema
feat: add OwnerReferences to BoundSchema
2 parents ef17d6e + 9ea0ae3 commit 9ed1827

3 files changed

Lines changed: 88 additions & 33 deletions

File tree

‎backend/controllers/serviceexportrequest/serviceexportrequest_controller.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ func NewAPIServiceExportRequestReconciler(
105105
createBoundSchema: func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error {
106106
return cl.Create(ctx, schema)
107107
},
108+
updateBoundSchema: func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error {
109+
return cl.Update(ctx, schema)
110+
},
108111
deleteServiceExportRequest: func(ctx context.Context, cl client.Client, ns, name string) error {
109112
return cl.Delete(ctx, &kubebindv1alpha2.APIServiceExportRequest{
110113
ObjectMeta: metav1.ObjectMeta{

‎backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go‎

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/klog/v2"
3232
"sigs.k8s.io/controller-runtime/pkg/cache"
3333
"sigs.k8s.io/controller-runtime/pkg/client"
34+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3435

3536
"github.com/kube-bind/kube-bind/backend/kubernetes/resources"
3637
kubebindv1alpha2 "github.com/kube-bind/kube-bind/sdk/apis/kubebind/v1alpha2"
@@ -46,16 +47,21 @@ type reconciler struct {
4647

4748
getBoundSchema func(ctx context.Context, cl client.Client, namespace, name string) (*kubebindv1alpha2.BoundSchema, error)
4849
createBoundSchema func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error
50+
updateBoundSchema func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error
4951

5052
getServiceExport func(ctx context.Context, cache cache.Cache, ns, name string) (*kubebindv1alpha2.APIServiceExport, error)
5153
createServiceExport func(ctx context.Context, cl client.Client, resource *kubebindv1alpha2.APIServiceExport) error
5254
deleteServiceExportRequest func(ctx context.Context, cl client.Client, namespace, name string) error
5355
}
5456

5557
func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
58+
export, err := r.getServiceExport(ctx, cache, req.Namespace, req.Name)
59+
if err != nil && !apierrors.IsNotFound(err) {
60+
return fmt.Errorf("failed to get APIServiceExport: %w", err)
61+
}
5662
// We must ensure schemas are created in form of boundSchemas first for the validation.
5763
// Worst case scenario if validation fails, we will reuse schemas for same consumer once issues are fixed.
58-
if err := r.ensureBoundSchemas(ctx, cl, cache, req); err != nil {
64+
if err := r.ensureBoundSchemas(ctx, cl, export, req); err != nil {
5965
conditions.SetSummary(req)
6066
return fmt.Errorf("failed to ensure bound schemas: %w", err)
6167
}
@@ -65,7 +71,7 @@ func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cach
6571
return fmt.Errorf("failed to validate APIServiceExportRequest: %w", err)
6672
}
6773

68-
if err := r.ensureExports(ctx, cl, cache, req); err != nil {
74+
if err := r.ensureExports(ctx, cl, export, req); err != nil {
6975
conditions.SetSummary(req)
7076
return fmt.Errorf("failed to ensure exports: %w", err)
7177
}
@@ -75,10 +81,6 @@ func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cach
7581
return fmt.Errorf("failed to ensure APIServiceNamespaces: %w", err)
7682
}
7783

78-
// TODO(mjudeikis): we could potentially add finallizer to APIServiceExport above or "adopt" boundschemas
79-
// with owner references once export is created.
80-
// https://github.com/kube-bind/kube-bind/issues/297
81-
8284
conditions.SetSummary(req)
8385

8486
return nil
@@ -134,13 +136,12 @@ func (r *reconciler) getExportedSchemas(ctx context.Context, cl client.Client) (
134136
return boundSchemas, nil
135137
}
136138

137-
func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, _ cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
139+
func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, export *kubebindv1alpha2.APIServiceExport, req *kubebindv1alpha2.APIServiceExportRequest) error {
138140
exportedSchemas, err := r.getExportedSchemas(ctx, cl)
139141
if err != nil {
140142
return err
141143
}
142144

143-
// Ensure all bound schemas exist
144145
for _, res := range req.Spec.Resources {
145146
if len(res.Versions) == 0 {
146147
continue
@@ -153,35 +154,53 @@ func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, _
153154
boundSchema.Spec.InformerScope = r.informerScope
154155
boundSchema.ResourceVersion = ""
155156

156-
obj, err := r.getBoundSchema(ctx, cl, boundSchema.Namespace, boundSchema.Name)
157-
if err != nil && !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
157+
if err := r.createOrUpdateBoundSchema(ctx, cl, export, boundSchema); err != nil {
158158
return err
159159
}
160+
}
161+
}
162+
}
160163

161-
// TODO(mjudeikis): https://github.com/kube-bind/kube-bind/issues/297
162-
if obj != nil {
163-
continue
164-
}
164+
return nil
165+
}
165166

166-
// If namespaced isolation is configured for cluster-scoped objects,
167-
// we need to rewrite the BoundSchema's scope accordingly. For all
168-
// other isolation strategies, as well as for namespaced schemas,
169-
// no changes are necessary.
170-
if boundSchema.Spec.Scope == apiextensionsv1.NamespaceScoped && r.isolation == kubebindv1alpha2.IsolationNamespaced {
171-
boundSchema.Spec.Scope = apiextensionsv1.ClusterScoped
172-
}
167+
func (r *reconciler) createOrUpdateBoundSchema(ctx context.Context, cl client.Client, export *kubebindv1alpha2.APIServiceExport, desired *kubebindv1alpha2.BoundSchema) error {
168+
logger := klog.FromContext(ctx)
173169

174-
if err := r.createBoundSchema(ctx, cl, boundSchema); err != nil {
175-
return err
176-
}
177-
}
170+
existing, err := r.getBoundSchema(ctx, cl, desired.Namespace, desired.Name)
171+
if err != nil && !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
172+
return err
173+
}
174+
175+
if existing != nil {
176+
if export == nil {
177+
return nil
178+
}
179+
if err := controllerutil.SetControllerReference(export, existing, cl.Scheme()); err != nil {
180+
return fmt.Errorf("failed to set owner reference on BoundSchema %s: %w", desired.Name, err)
178181
}
182+
if err := r.updateBoundSchema(ctx, cl, existing); err != nil {
183+
return fmt.Errorf("failed to update BoundSchema %s with owner reference: %w", desired.Name, err)
184+
}
185+
logger.V(6).Info("Updated owner reference on existing BoundSchema",
186+
"boundSchema", desired.Name,
187+
"export", export.Name,
188+
"namespace", desired.Namespace)
189+
return nil
179190
}
180191

181-
return nil
192+
// If namespaced isolation is configured for cluster-scoped objects,
193+
// we need to rewrite the BoundSchema's scope accordingly. For all
194+
// other isolation strategies, as well as for namespaced schemas,
195+
// no changes are necessary.
196+
if desired.Spec.Scope == apiextensionsv1.NamespaceScoped && r.isolation == kubebindv1alpha2.IsolationNamespaced {
197+
desired.Spec.Scope = apiextensionsv1.ClusterScoped
198+
}
199+
200+
return r.createBoundSchema(ctx, cl, desired)
182201
}
183202

184-
func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
203+
func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, existingExport *kubebindv1alpha2.APIServiceExport, req *kubebindv1alpha2.APIServiceExportRequest) error {
185204
logger := klog.FromContext(ctx)
186205

187206
var schemas []*kubebindv1alpha2.BoundSchema
@@ -209,12 +228,7 @@ func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, cache
209228
schemas = append(schemas, boundSchema)
210229
}
211230

212-
if _, err := r.getServiceExport(ctx, cache, req.Namespace, req.Name); err != nil {
213-
if !apierrors.IsNotFound(err) {
214-
return err
215-
}
216-
} else {
217-
// already exists; nothing to do
231+
if existingExport != nil {
218232
conditions.MarkTrue(req, kubebindv1alpha2.APIServiceExportRequestConditionExportsReady)
219233
return nil
220234
}

‎test/e2e/bind/happy-case_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,44 @@ func testHappyCase(
448448
require.NotEqual(t, consumer.providerContractNamespace, "unknown")
449449
},
450450
},
451+
{
452+
name: "verify BoundSchemas have owner references to APIServiceExport",
453+
step: func(t *testing.T) {
454+
t.Logf("Verifying BoundSchemas have owner references to APIServiceExport")
455+
export, err := providerBindClient.KubeBindV1alpha2().APIServiceExports(consumer.providerContractNamespace).Get(ctx, "test-binding", metav1.GetOptions{})
456+
if errors.IsNotFound(err) && consumer.name != "consumer1" {
457+
t.Skip("APIServiceExport already deleted by another consumer")
458+
}
459+
require.NoError(t, err, "APIServiceExport should exist")
460+
461+
boundSchemas, err := providerBindClient.KubeBindV1alpha2().BoundSchemas(consumer.providerContractNamespace).List(ctx, metav1.ListOptions{})
462+
require.NoError(t, err, "should be able to list BoundSchemas")
463+
require.NotEmpty(t, boundSchemas.Items, "should have at least one BoundSchema")
464+
465+
// Verify each BoundSchema has an owner reference to the APIServiceExport
466+
for _, boundSchema := range boundSchemas.Items {
467+
t.Logf("Checking owner reference for BoundSchema %s", boundSchema.Name)
468+
469+
var hasOwnerRef bool
470+
for _, ownerRef := range boundSchema.OwnerReferences {
471+
if ownerRef.Kind == "APIServiceExport" &&
472+
ownerRef.Name == export.Name &&
473+
ownerRef.UID == export.UID &&
474+
ownerRef.Controller != nil &&
475+
*ownerRef.Controller {
476+
hasOwnerRef = true
477+
break
478+
}
479+
}
480+
481+
require.True(t, hasOwnerRef,
482+
"BoundSchema %s should have controller owner reference to APIServiceExport %s",
483+
boundSchema.Name, export.Name)
484+
}
485+
486+
t.Logf("All BoundSchemas have proper owner references")
487+
},
488+
},
451489
// Request included namespace, so we check it first
452490
{
453491
name: "verify provider side namespace pre-seeding and RBAC management",

0 commit comments

Comments
 (0)