Skip to content

Commit be86cd9

Browse files
committed
Fix backend scoping
1 parent c31ff72 commit be86cd9

16 files changed

Lines changed: 209 additions & 35 deletions

backend/controllers/clusterbinding/clusterbinding_reconcile.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,15 +172,18 @@ func (r *reconciler) ensureRBACClusterRole(ctx context.Context, client client.Cl
172172
Resources: []string{schema.Spec.Names.Plural},
173173
Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"},
174174
},
175-
rbacv1.PolicyRule{
176-
APIGroups: []string{kubebindv1alpha2.GroupName},
177-
Resources: []string{"boundschemas"},
178-
Verbs: []string{"get", "list", "watch"},
179-
},
180175
)
181176
}
182177
}
183178

179+
expected.Rules = append(expected.Rules,
180+
rbacv1.PolicyRule{
181+
APIGroups: []string{kubebindv1alpha2.GroupName},
182+
Resources: []string{"boundschemas"},
183+
Verbs: []string{"get", "list", "watch"},
184+
},
185+
)
186+
184187
if role == nil {
185188
if err := r.createClusterRole(ctx, client, expected); err != nil {
186189
return fmt.Errorf("failed to create ClusterRole %s: %w", expected.Name, err)
@@ -202,7 +205,6 @@ func (r *reconciler) ensureRBACClusterRoleBinding(ctx context.Context, client cl
202205
if err != nil && !errors.IsNotFound(err) {
203206
return fmt.Errorf("failed to get ClusterRoleBinding %s: %w", name, err)
204207
}
205-
206208
if r.scope != kubebindv1alpha2.ClusterScope {
207209
if err := r.deleteClusterRoleBinding(ctx, client, name); err != nil && !errors.IsNotFound(err) {
208210
return fmt.Errorf("failed to delete ClusterRoleBinding %s: %w", name, err)

backend/controllers/serviceexportrequest/serviceexportrequest_controller.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,17 @@ func (r *APIServiceExportRequestReconciler) Reconcile(ctx context.Context, req m
215215
// Run the reconciliation logic
216216
if err := r.reconciler.reconcile(ctx, client, cache, apiServiceExportRequest); err != nil {
217217
logger.Error(err, "Failed to reconcile APIServiceExportRequest")
218-
return ctrl.Result{}, err
218+
// we still need to update status to reflect error conditions.
219+
if !reflect.DeepEqual(original.Status, apiServiceExportRequest.Status) {
220+
err := client.Status().Update(ctx, apiServiceExportRequest)
221+
if err != nil {
222+
logger.Error(err, "Failed to update APIServiceExportRequest status after reconciliation error")
223+
return ctrl.Result{}, fmt.Errorf("failed to update APIServiceExportRequest status after reconciliation error: %w", err)
224+
}
225+
}
226+
return ctrl.Result{
227+
Requeue: false,
228+
}, err
219229
}
220230

221231
// Update status if it has changed

backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,12 @@ type reconciler struct {
5454

5555
func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
5656
if err := r.ensureBoundSchemas(ctx, cl, cache, req); err != nil {
57+
conditions.SetSummary(req)
5758
return err
5859
}
5960

6061
if err := r.ensureExports(ctx, cl, cache, req); err != nil {
62+
conditions.SetSummary(req)
6163
return err
6264
}
6365

@@ -119,7 +121,26 @@ func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, c
119121
klog.FromContext(ctx).Error(nil, "Skipping invalid schema: missing group or names.plural", "ns", item.GetNamespace(), "name", item.GetName())
120122
continue
121123
}
124+
scope := apiextensionsv1.ResourceScope(spec["scope"].(string))
125+
122126
if g == res.Group && p == res.Resource {
127+
// Important: This checks if the resource are right-scoped. If consumer is namespaced, we can't allow this.
128+
// We terminate early to prevent triggering other controllers.
129+
if r.informerScope.String() != string(scope) && r.informerScope != kubebindv1alpha2.ClusterScope {
130+
conditions.MarkFalse(
131+
req,
132+
kubebindv1alpha2.APIServiceExportRequestConditionExportsReady,
133+
"APIServiceExportRequestInvalid",
134+
conditionsapi.ConditionSeverityError,
135+
"APIServiceExportRequest %s is invalid: resource %s/%s has scope %q which is incompatible with backend informer scope %q",
136+
req.Name, g, p, scope, r.informerScope,
137+
)
138+
req.Status.Phase = kubebindv1alpha2.APIServiceExportRequestPhaseFailed
139+
req.Status.TerminalMessage = conditions.GetMessage(req, kubebindv1alpha2.APIServiceExportRequestConditionExportsReady)
140+
// We can't proceed with this request.
141+
return nil
142+
}
143+
123144
boundSchema, err := helpers.UnstructuredToBoundSchema(item)
124145
if err != nil {
125146
return err

backend/http/handler.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,11 @@ func (h *handler) handleResources(w http.ResponseWriter, r *http.Request) {
388388
scope = "-"
389389
}
390390

391+
// TODO(mjudeikis): This parsing is very brittle. This is fixed in follow-up permissionClaims PR.
392+
if !strings.EqualFold(h.scope.String(), scope.(string)) && h.scope != kubebindv1alpha2.ClusterScope {
393+
continue
394+
}
395+
391396
group := item.UnstructuredContent()["spec"].(map[string]interface{})["group"]
392397
if group == nil {
393398
group = "-"

backend/kubernetes/manager.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -192,24 +192,3 @@ func (m *Manager) ListDynamicResources(ctx context.Context, cluster string, gvk
192192

193193
return list, nil
194194
}
195-
196-
/* func (m *Manager) CreateBoundSchema(ctx context.Context, cluster string, name string, u *unstructured.Unstructured) error {
197-
cl, err := m.manager.GetCluster(ctx, cluster)
198-
if err != nil {
199-
return err
200-
}
201-
c := cl.GetClient()
202-
203-
boundSchema := &kubebindv1alpha2.BoundSchema{}
204-
err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), boundSchema)
205-
if err != nil {
206-
return err
207-
}
208-
209-
boundSchema.ResourceVersion = ""
210-
boundSchema.Name = name
211-
boundSchema.Spec.InformerScope = m.scope
212-
213-
return c.Create(ctx, boundSchema)
214-
}
215-
*/

kcp/README.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ kubectl kcp bind apiexport root:kube-bind:kube-bind.io --accept-permission-claim
7373

7474
7. Create CRD:
7575
```bash
76-
kubectl create -f kcp/deploy/examples/apiexport.yaml
77-
kubectl create -f kcp/deploy/examples/apiresourceschema.yaml
76+
kubectl apply -f kcp/deploy/examples/apiexport.yaml
77+
kubectl create -f kcp/deploy/examples/apiresourceschema-cowboys.yaml
78+
kubectl create -f kcp/deploy/examples/apiresourceschema-sheriffs.yaml
7879
# recursive bind
7980
kubectl kcp bind apiexport root:provider:cowboys-stable
8081
```
@@ -84,7 +85,7 @@ kubectl kcp bind apiexport root:provider:cowboys-stable
8485
```bash
8586
kubectl get logicalcluster
8687
# NAME PHASE URL AGE
87-
# cluster Ready https://192.168.2.166:6443/clusters/2xh2v3gzjhn4tmve
88+
# cluster Ready https://192.168.2.166:6443/clusters/2d0i12ky2qn7e4js
8889
```
8990

9091
9. Now we gonna initiate consumer:
@@ -98,13 +99,13 @@ kubectl ws create consumer --enter
9899
10. Bind the thing:
99100

100101
```bash
101-
./bin/kubectl-bind http://127.0.0.1:8080/clusters/2vgrh380y0cq38du/exports --dry-run -o yaml > apiserviceexport.yaml
102+
./bin/kubectl-bind http://127.0.0.1:8080/clusters/2d0i12ky2qn7e4js/exports --dry-run -o yaml > apiserviceexport.yaml
102103

103104
# Extract secret for binding process. Note that secret name is not the same as output from command above. Check secret
104105
# name by running `kubectl get secret -n kube-bind`
105-
kubectl get secret kubeconfig-wvvsb -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
106+
kubectl get secret kubeconfig-49z2s -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
106107

107-
./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f apiserviceexport.yaml --skip-konnector --remote-namespace kube-bind-m5zx4
108+
./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f kcp/deploy/examples/apiserviceexport-cluster.yaml --skip-konnector --remote-namespace kube-bind-jjgms
108109

109110
export KUBECONFIG=.kcp/consumer.kubeconfig
110111
go run ./cmd/konnector/ --lease-namespace default
@@ -131,9 +132,9 @@ go run ./cmd/konnector/ --lease-namespace default --server-address :8091
131132
Create objects:
132133
```
133134
kubectl apply -f kcp/deploy/examples/cowboy.yaml
135+
kubectl apply -f kcp/deploy/examples/sheriff.yaml
134136
```
135137
136-
137138
## Debug
138139
139140
```bash

kcp/deploy/examples/apiexport.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,10 @@ spec:
77
- group: wildwest.dev
88
name: cowboys
99
schema: today.cowboys.wildwest.dev
10+
storage:
11+
crd: {}
12+
- group: wildwest.dev
13+
name: sheriffs
14+
schema: today.sheriffs.wildwest.dev
1015
storage:
1116
crd: {}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
apiVersion: apis.kcp.io/v1alpha1
2+
kind: APIResourceSchema
3+
metadata:
4+
name: today.cowboys.wildwest.dev
5+
labels:
6+
kube-bind.io/exported: "true"
7+
spec:
8+
group: wildwest.dev
9+
names:
10+
kind: Cowboy
11+
listKind: CowboyList
12+
plural: cowboys
13+
singular: cowboy
14+
scope: Namespaced
15+
versions:
16+
- name: v1alpha1
17+
schema:
18+
description: Cowboy is part of the wild west
19+
properties:
20+
apiVersion:
21+
description: 'APIVersion defines the versioned schema of this representation
22+
of an object. Servers should convert recognized schemas to the latest
23+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
24+
type: string
25+
kind:
26+
description: 'Kind is a string value representing the REST resource this
27+
object represents. Servers may infer this from the endpoint the client
28+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
29+
type: string
30+
metadata:
31+
type: object
32+
spec:
33+
description: CowboySpec holds the desired state of the Cowboy.
34+
properties:
35+
intent:
36+
type: string
37+
type: object
38+
status:
39+
description: CowboyStatus communicates the observed state of the Cowboy.
40+
properties:
41+
result:
42+
type: string
43+
type: object
44+
type: object
45+
served: true
46+
storage: true
47+
subresources:
48+
status: {}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
apiVersion: apis.kcp.io/v1alpha1
2+
kind: APIResourceSchema
3+
metadata:
4+
name: today.sheriffs.wildwest.dev
5+
labels:
6+
kube-bind.io/exported: "true"
7+
spec:
8+
group: wildwest.dev
9+
names:
10+
kind: Sheriff
11+
listKind: SheriffList
12+
plural: sheriffs
13+
singular: sheriff
14+
scope: Cluster
15+
versions:
16+
- name: v1alpha1
17+
schema:
18+
description: Sheriff is part of the wild west
19+
properties:
20+
apiVersion:
21+
description: 'APIVersion defines the versioned schema of this representation
22+
of an object. Servers should convert recognized schemas to the latest
23+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
24+
type: string
25+
kind:
26+
description: 'Kind is a string value representing the REST resource this
27+
object represents. Servers may infer this from the endpoint the client
28+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
29+
type: string
30+
metadata:
31+
type: object
32+
spec:
33+
description: SheriffSpec holds the desired state of the Sheriff.
34+
properties:
35+
intent:
36+
type: string
37+
type: object
38+
status:
39+
description: SheriffStatus communicates the observed state of the Sheriff.
40+
properties:
41+
result:
42+
type: string
43+
type: object
44+
type: object
45+
served: true
46+
storage: true
47+
subresources:
48+
status: {}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
apiVersion: kube-bind.io/v1alpha2
2+
kind: APIServiceExportRequest
3+
metadata:
4+
name: sheriffs.wildwest.dev
5+
spec:
6+
resources:
7+
- group: wildwest.dev
8+
resource: sheriffs
9+
versions:
10+
- v1alpha1
11+
status: {}

0 commit comments

Comments
 (0)