Part3: Make backend universal and support kcp #276
Conversation
3f7c4bb to
390e348
Compare
390e348 to
df413f1
Compare
df413f1 to
b650304
Compare
| return nil, err | ||
| } | ||
| return binding, nil | ||
| createClusterRoleBinding: func(ctx context.Context, client client.Client, binding *rbacv1.ClusterRoleBinding) error { |
There was a problem hiding this comment.
Do all of these still offer a real value, now that they are one-liners?
There was a problem hiding this comment.
It just gives consistency in mocking. If one of these are more complex than one-line it makes sense to have all of them same so its just consistent.
We can always refactor these out if we see need. Now - it just change for sake of change :)
| } | ||
|
|
||
| func (s *Server) Start(ctx context.Context) error { | ||
| fakeBatteries := sets.New("") |
There was a problem hiding this comment.
Are the fake batteries meant to include a single, empty string?
There was a problem hiding this comment.
yap. Because I reuse kcp machinery/lib for it. it just placeholder to be able to reuse lib.
| }, | ||
| } | ||
|
|
||
| binding.Spec.PermissionClaims = []apisv1alpha2.AcceptablePermissionClaim{ |
There was a problem hiding this comment.
In case you were just looking for a 2nd opinion: I would consider using a loop here to construct the claims. For my taste it would probably save enough code to make it worth it. But that's just my 2 cents. Take it or leave it.
There was a problem hiding this comment.
This was lift and shift. Lets do this in follow-up
#286
| @@ -0,0 +1,32 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| # Copyright 2021 The Kube Bind Authors. | |||
There was a problem hiding this comment.
In theory this was written in 2021 :D
b650304 to
71cd9ba
Compare
71cd9ba to
cce3a0c
Compare
xrstf
left a comment
There was a problem hiding this comment.
I'll fix the empty string feature gate later.
Summary
This PR start making backend a first-class citizen on the project, which means that with the move to multicluster-runtime we can now make it runnable for any cluster.
Removes kcp SDK and moves
kcprelated assets to/kcpMakes separate package
backendfor new/old backend.Removes usage of generated client (still left in konnector) #280
Commit history is bit shady. I tried to split no-op to individual separate commit so one can just review second commit only.
Fixes: #267
Will need kubernetes-sigs/multicluster-runtime#62 as now its go mod replaced
Will need version of ^ in kcp apiexport providers too as its too go mod replaces
What Type of PR Is This?
Related Issue(s)
Fixes #
Release Notes