Skip to content

Commit 4f0596f

Browse files
committed
address reviews
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt>
1 parent 65d1922 commit 4f0596f

11 files changed

Lines changed: 38 additions & 41 deletions

File tree

backend/options/options.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (options *Options) AddFlags(fs *pflag.FlagSet) {
154154
// TODO(mjudeikis): remove deprecated flag in future release
155155
fs.StringVar(&options.Isolation, "cluster-scoped-isolation", options.Isolation, "How cluster scoped service objects are isolated between multiple consumers on the provider side. Among the choices, \"prefixed\" prepends the name of the cluster namespace to an object's name; \"namespaced\" maps a consumer side object into a namespaced object inside the corresponding cluster namespace; \"none\" is used for the case of a dedicated provider where isolation is not necessary.")
156156
_ = fs.MarkDeprecated("cluster-scoped-isolation", "use --isolation instead")
157-
fs.StringVar(&options.Isolation, "isolation", options.Isolation, "Deprecated: use --cluster-scoped-isolation instead. How cluster scoped service objects are isolated between multiple consumers on the provider side. Among the choices, \"prefixed\" prepends the name of the cluster namespace to an object's name; \"namespaced\" maps a consumer side object into a namespaced object inside the corresponding cluster namespace; \"none\" is used for the case of a dedicated provider where isolation is not necessary.")
157+
fs.StringVar(&options.Isolation, "isolation", options.Isolation, "Deprecated: use --isolation instead. How cluster scoped service objects are isolated between multiple consumers on the provider side. Among the choices, \"prefixed\" prepends the name of the cluster namespace to an object's name; \"namespaced\" maps a consumer side object into a namespaced object inside the corresponding cluster namespace; \"none\" is used for the case of a dedicated provider where isolation is not necessary.")
158158
fs.StringVar(&options.ExternalAddress, "external-address", options.ExternalAddress, "The external address for the service provider cluster, including https:// and port. If not specified, service account's hosts are used.")
159159
fs.StringVar(&options.ExternalCAFile, "external-ca-file", options.ExternalCAFile, "The external CA file for the service provider cluster. If not specified, service account's CA is used.")
160160
fs.StringVar(&options.TLSExternalServerName, "external-server-name", options.TLSExternalServerName, "The external (TLS) server name used by consumers to talk to the service provider cluster. This can be useful to select the right certificate via SNI.")

cli/pkg/kubectl/dev/plugin/create.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,32 +70,25 @@ type DevOptions struct {
7070
KindNetwork string
7171
}
7272

73-
// assetVersion is the version of the kube-bind backend assets used in dev mode
74-
var assetVersion = ""
75-
7673
// fallbackAssetVersion is used when unable to fetch the latest version
77-
var fallbackAssetVersion = "0.6.0"
74+
const fallbackAssetVersion = "0.6.0"
7875

79-
// GitHubRelease represents a GitHub release response
80-
type GitHubRelease struct {
76+
// gitHubRelease represents a GitHub release response
77+
type gitHubRelease struct {
8178
TagName string `json:"tag_name"`
8279
}
8380

8481
// NewDevOptions creates a new DevOptions
8582
func NewDevOptions(streams genericclioptions.IOStreams) *DevOptions {
8683
opts := base.NewOptions(streams)
87-
// Initialize assetVersion with fallback if not set
88-
if assetVersion == "" {
89-
assetVersion = fallbackAssetVersion
90-
}
9184
return &DevOptions{
9285
Options: opts,
9386
Logs: logs.NewOptions(),
9487
Streams: streams,
9588
ProviderClusterName: "kind-provider",
9689
ConsumerClusterName: "kind-consumer",
9790
ChartPath: "oci://ghcr.io/kube-bind/charts/backend",
98-
ChartVersion: assetVersion,
91+
ChartVersion: fallbackAssetVersion,
9992
}
10093
}
10194

@@ -110,14 +103,15 @@ func (o *DevOptions) AddCmdFlags(cmd *cobra.Command) {
110103
cmd.Flags().StringVar(&o.ChartPath, "chart-path", o.ChartPath, "Helm chart path or OCI registry URL")
111104
cmd.Flags().StringVar(&o.ChartVersion, "chart-version", o.ChartVersion, "Helm chart version")
112105
cmd.Flags().StringVar(&o.Image, "image", "ghcr.io/kube-bind/backend", "kube-bind backend image to use in dev mode")
113-
cmd.Flags().StringVar(&o.Tag, "tag", "v"+assetVersion, "kube-bind backend image tag to use in dev mode")
106+
cmd.Flags().StringVar(&o.Tag, "tag", "", "kube-bind backend image tag to use in dev mode")
114107
cmd.Flags().StringVar(&o.KindNetwork, "kind-network", "kube-bind-dev", "kind network to use in dev mode")
115108
}
116109

117110
// Complete completes the options
118111
func (o *DevOptions) Complete(args []string) error {
119-
// Only fetch the latest version if assetVersion is not set
120-
if assetVersion == "" {
112+
// Only fetch the latest version if tag is not set
113+
var assetVersion string
114+
if o.Tag == "" {
121115
version, err := fetchLatestRelease()
122116
if err != nil {
123117
// Log the error but continue with fallback version
@@ -164,7 +158,7 @@ func fetchLatestRelease() (string, error) {
164158
return "", fmt.Errorf("failed to read response body: %w", err)
165159
}
166160

167-
var release GitHubRelease
161+
var release gitHubRelease
168162
if err := json.Unmarshal(body, &release); err != nil {
169163
return "", fmt.Errorf("failed to parse release data: %w", err)
170164
}
@@ -173,7 +167,6 @@ func fetchLatestRelease() (string, error) {
173167
return "", fmt.Errorf("no tag name in release data")
174168
}
175169

176-
// Remove 'v' prefix if present
177170
version := strings.TrimPrefix(release.TagName, "v")
178171
return version, nil
179172
}

contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ spec:
6262
crd: {}
6363
- group: kube-bind.io
6464
name: apiserviceexports
65-
schema: v251230-e36591a1.apiserviceexports.kube-bind.io
65+
schema: v260119-2e5a3e93.apiserviceexports.kube-bind.io
6666
storage:
6767
crd: {}
6868
- group: kube-bind.io

contrib/kcp/deploy/resources/apiresourceschema-apiserviceexports.kube-bind.io.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
apiVersion: apis.kcp.io/v1alpha1
22
kind: APIResourceSchema
33
metadata:
4-
name: v251230-e36591a1.apiserviceexports.kube-bind.io
4+
name: v260119-2e5a3e93.apiserviceexports.kube-bind.io
55
spec:
66
conversion:
77
strategy: None
@@ -466,7 +466,8 @@ spec:
466466
description: |-
467467
ClusterScopedIsolation specifies how cluster scoped service objects are isolated between multiple consumers on the provider side.
468468
It can be "Prefixed", "Namespaced", or "None".
469-
Deprecated: use Isolation instead.
469+
470+
Deprecated: ClusterScopedIsolation is deprecated, use Isolation instead.
470471
enum:
471472
- Prefixed
472473
- Namespaced
@@ -491,9 +492,8 @@ spec:
491492
- message: informerScope is immutable
492493
rule: self == oldSelf
493494
isolation:
494-
description: |-
495-
Isolation specifies how service objects are isolated between multiple consumers on the provider side.
496-
It can be "Prefixed", "Namespaced", or "None".
495+
description: Isolation specifies how service objects are isolated between
496+
multiple consumers on the provider side.
497497
enum:
498498
- Prefixed
499499
- Namespaced

deploy/charts/backend/crds/kube-bind.io_apiserviceexports.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,8 @@ spec:
469469
description: |-
470470
ClusterScopedIsolation specifies how cluster scoped service objects are isolated between multiple consumers on the provider side.
471471
It can be "Prefixed", "Namespaced", or "None".
472-
Deprecated: use Isolation instead.
472+
473+
Deprecated: ClusterScopedIsolation is deprecated, use Isolation instead.
473474
enum:
474475
- Prefixed
475476
- Namespaced
@@ -494,9 +495,8 @@ spec:
494495
- message: informerScope is immutable
495496
rule: self == oldSelf
496497
isolation:
497-
description: |-
498-
Isolation specifies how service objects are isolated between multiple consumers on the provider side.
499-
It can be "Prefixed", "Namespaced", or "None".
498+
description: Isolation specifies how service objects are isolated
499+
between multiple consumers on the provider side.
500500
enum:
501501
- Prefixed
502502
- Namespaced

deploy/crd/kube-bind.io_apiserviceexports.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,8 @@ spec:
470470
description: |-
471471
ClusterScopedIsolation specifies how cluster scoped service objects are isolated between multiple consumers on the provider side.
472472
It can be "Prefixed", "Namespaced", or "None".
473-
Deprecated: use Isolation instead.
473+
474+
Deprecated: ClusterScopedIsolation is deprecated, use Isolation instead.
474475
enum:
475476
- Prefixed
476477
- Namespaced
@@ -495,9 +496,8 @@ spec:
495496
- message: informerScope is immutable
496497
rule: self == oldSelf
497498
isolation:
498-
description: |-
499-
Isolation specifies how service objects are isolated between multiple consumers on the provider side.
500-
It can be "Prefixed", "Namespaced", or "None".
499+
description: Isolation specifies how service objects are isolated
500+
between multiple consumers on the provider side.
501501
enum:
502502
- Prefixed
503503
- Namespaced

docs/content/developers/architecture.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
This document provides detailed architecture diagrams and explanations for how kube-bind handles resource synchronization between consumer and provider clusters, with a focus on isolation strategies and namespace mapping.
44

5+
This is development-focused documentation intended to help contributors understand the internal workings of kube-bind.
6+
If you are looking for user-focused documentation, please refer to the [Synchronization User Guide](../usage/synchronization.md).
7+
58
## Cluster-Scoped Resources
69

710
Cluster-scoped resources (like Sheriffs in our examples) can use different isolation strategies to control how they are placed on the provider cluster and how their associated secrets are isolated.

pkg/konnector/controllers/cluster/serviceexport/serviceexport_reconcile.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,8 @@ func (r *reconciler) ensureControllerForSchema(ctx context.Context, export *kube
280280
isolationStrategy = isolation.NewNamespaced(r.providerNamespace)
281281
default:
282282
// Default to None isolation strategy if no valid isolation strategy is specified
283-
logger.V(4).Info("Using default None isolation strategy", "export", export.Name)
283+
// This should never happen due to validation, but we add this as a safety net.
284+
logger.V(2).Info("Using default None isolation strategy", "export", export.Name)
284285
isolationStrategy = isolation.NewNone(r.providerNamespace, providerNamespaceUID)
285286
}
286287

pkg/resources/resources.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func IsClaimedWithReference(
208208

209209
if !isReferenceAllowed(ref, apiServiceExport) {
210210
logger.Info("reference not allowed, resource is not part of the contract",
211-
"isolation", apiServiceExport.Spec.ClusterScopedIsolation,
211+
"isolation", apiServiceExport.Spec.Isolation,
212212
"referenceGroup", ref.Group,
213213
"referenceResource", ref.Resource,
214214
)
@@ -254,7 +254,7 @@ func IsClaimedWithReference(
254254

255255
if consumerSide {
256256
logger.Info("checking claim on consumer side")
257-
result := IsClaimed(logger, claim.Selector, copy, potentiallyReferencedResources, apiServiceExport.Spec.ClusterScopedIsolation)
257+
result := IsClaimed(logger, claim.Selector, copy, potentiallyReferencedResources, apiServiceExport.Spec.Isolation)
258258
logger.Info("IsClaimed result", "result", result)
259259
return result
260260
}
@@ -280,14 +280,14 @@ func IsClaimedWithReference(
280280
copy.SetNamespace(sn.Name)
281281
}
282282

283-
result := IsClaimed(logger, claim.Selector, copy, potentiallyReferencedResources, apiServiceExport.Spec.ClusterScopedIsolation)
283+
result := IsClaimed(logger, claim.Selector, copy, potentiallyReferencedResources, apiServiceExport.Spec.Isolation)
284284
logger.V(4).Info("IsClaimed result (provider side)", "result", result)
285285
return result
286286
}
287287

288288
func isReferenceAllowed(ref kubebindv1alpha2.SelectorReference, apiServiceExport *kubebindv1alpha2.APIServiceExport) bool {
289289
// If isolation is None, everything should be allowed, as both ends are owned.
290-
if apiServiceExport.Spec.ClusterScopedIsolation == kubebindv1alpha2.IsolationNone {
290+
if apiServiceExport.Spec.Isolation == kubebindv1alpha2.IsolationNone {
291291
return true
292292
}
293293
for _, resource := range apiServiceExport.Spec.Resources {

pkg/resources/resources_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,8 +1129,8 @@ func TestReferenceSelector_IsolationMode(t *testing.T) {
11291129
t.Run(tt.name, func(t *testing.T) {
11301130
export := &kubebindv1alpha2.APIServiceExport{
11311131
Spec: kubebindv1alpha2.APIServiceExportSpec{
1132-
ClusterScopedIsolation: tt.isolation,
1133-
Resources: tt.exportedResources,
1132+
Isolation: tt.isolation,
1133+
Resources: tt.exportedResources,
11341134
},
11351135
}
11361136
got := isReferenceAllowed(tt.reference, export)

0 commit comments

Comments
 (0)