Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ Kro is Kubernetes native and integrates seamlessly with existing tools to preser
| [Examples][kro-examples] | Example resources |
| [Contributions](./CONTRIBUTING.md) | How to get involved |

[kro-instance-scope]: https://kro.run/docs/concepts/resource-group-definitions

### Instance scope (new)

ResourceGraphDefinitions can now choose the scope of the generated instance CRD via `spec.schema.scope`:
- `Namespaced` (default) — preserves current behavior.
- `Cluster` — generates a cluster-scoped instance CRD when your graph needs to be cluster-level.

The field is immutable after creation, matching Kubernetes CRD scope rules. See the concepts docs for details.

[kro-overview]: https://kro.run/docs/overview
[kro-installation]: https://kro.run/docs/getting-started/Installation
[kro-getting-started]: https://kro.run/docs/getting-started/deploy-a-resource-graph-definition
Expand Down
16 changes: 16 additions & 0 deletions api/v1alpha1/resourcegraphdefinition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ type Schema struct {
// +kubebuilder:default="kro.run"
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="group is immutable"
Group string `json:"group,omitempty"`
// Scope determines whether the generated instance CRD is Namespaced or Cluster scoped.
// Default is Namespaced to preserve current behavior. This field is immutable.
//
// +kubebuilder:validation:Optional
// +kubebuilder:default="Namespaced"
// +kubebuilder:validation:Enum=Namespaced;Cluster
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="scope is immutable"
Scope ResourceScope `json:"scope,omitempty"`
// Spec defines the schema for the instance's spec section using SimpleSchema syntax.
// This becomes the OpenAPI schema for instances of the generated CRD.
// Use SimpleSchema's concise syntax to define fields, types, defaults, and validations.
Expand Down Expand Up @@ -219,6 +227,14 @@ type Dependency struct {
ID string `json:"id,omitempty"`
}

// ResourceScope defines the scope of the generated instance CRD.
type ResourceScope string

const (
ScopeNamespaced ResourceScope = "Namespaced"
ScopeCluster ResourceScope = "Cluster"
)

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="APIVERSION",type=string,priority=0,JSONPath=`.spec.schema.apiVersion`
Expand Down
67 changes: 67 additions & 0 deletions docs/design/proposals/resource-descriptor-scope-resolution.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Use ResourceDescriptor for External Reference Scope Resolution

## Problem statement

When reconciling instances, the controller needs to interact with external references (resources not managed by the instance but referenced by it). To do this, it needs to know the GroupVersionResource (GVR) and the scope (namespaced or cluster-scoped) of the external resource.

Previously, the controller relied on `restMapper.RESTMapping` at runtime to resolve the GVK to GVR and determine the scope. This introduces a runtime dependency on the discovery client and can lead to errors if the REST mapper is not up-to-date or if the discovery fails. It also adds unnecessary latency to the reconciliation loop.

## Proposal

### Overview

We propose to use the `ResourceDescriptor` interface, which is populated during the ResourceGraphDefinition (RGD) compilation phase, to determine the GVR and scope of external references. The `ResourceDescriptor` already contains this information, as it is resolved when the RGD is processed.

### Design details

The `readExternalRef` function in `pkg/controller/instance/controller_reconcile.go` will be updated to:

1. Retrieve the `ResourceDescriptor` for the given resource ID from the runtime.
2. Use `descriptor.GetGroupVersionResource()` to get the GVR.
3. Use `descriptor.IsNamespaced()` to determine if the resource is namespaced.
4. Construct the dynamic client using this information.

```go
func (igr *instanceGraphReconciler) readExternalRef(ctx context.Context, resourceID string, resource *unstructured.Unstructured) (*unstructured.Unstructured, error) {
descriptor := igr.runtime.ResourceDescriptor(resourceID)
gvr := descriptor.GetGroupVersionResource()

var dynResource dynamic.ResourceInterface
if descriptor.IsNamespaced() {
namespace := igr.getResourceNamespace(resourceID)
dynResource = igr.client.Resource(gvr).Namespace(namespace)
} else {
dynResource = igr.client.Resource(gvr)
}
// ...
}
```

## Benefits

* **Performance**: Removes the need for a REST mapper lookup during every reconciliation of an external reference.
* **Reliability**: Relies on the static analysis performed during RGD compilation, which is consistent with how other resources are handled.
* **Consistency**: Aligns the handling of external references with managed resources, which already use `ResourceDescriptor`.

## Tradeoffs

* **Static Definition**: This assumes that the scope of a resource does not change between RGD compilation and runtime. In Kubernetes, changing the scope of a CRD is a breaking change and requires re-creation, so this assumption holds true for practical purposes.

## Scoping

### What is in scope for this proposal?

* Modifying `pkg/controller/instance/controller_reconcile.go`.
* Updating `readExternalRef` implementation.

### What is not in scope?

* Changes to RGD compilation logic (the information is already there).
* Changes to other parts of the controller.

## Testing strategy

### Test plan

* **Unit Tests**: Existing unit tests for the instance controller should pass.
* **Manual Verification**: Verify that external references are correctly resolved and read by the controller.
39 changes: 22 additions & 17 deletions pkg/controller/instance/controller_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,14 @@ func (igr *instanceGraphReconciler) setManaged(

igr.instanceLabeler.ApplyLabels(instancePatch)

updated, err := igr.client.Resource(igr.gvr).
Namespace(obj.GetNamespace()).
Apply(ctx, instancePatch.GetName(), instancePatch,
metav1.ApplyOptions{FieldManager: FieldManagerForLabeler, Force: true})
instanceClient := igr.client.Resource(igr.gvr)
var namespacedClient dynamic.ResourceInterface = instanceClient
if ns := obj.GetNamespace(); ns != "" {
namespacedClient = instanceClient.Namespace(ns)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

namespacedClient name is misleading IMO It could be cluster-scoped or namespaced client depending on the resource

I would suggest defining a getGVRClient used here, in the setUnmanaged and in the getResourceClient function to ensure namespacing is well-managed everywhere


updated, err := namespacedClient.Apply(ctx, instancePatch.GetName(), instancePatch,
metav1.ApplyOptions{FieldManager: FieldManagerForLabeler, Force: true})
if err != nil {
return nil, fmt.Errorf("failed to update managed state: %w", err)
}
Expand Down Expand Up @@ -552,10 +556,14 @@ func (igr *instanceGraphReconciler) setUnmanaged(
return nil, fmt.Errorf("failed to remove finalizer: %w", err)
}

updated, err := igr.client.Resource(igr.gvr).
Namespace(obj.GetNamespace()).
Apply(ctx, instancePatch.GetName(), instancePatch,
metav1.ApplyOptions{FieldManager: FieldManagerForLabeler, Force: true})
instanceClient := igr.client.Resource(igr.gvr)
var namespacedClient dynamic.ResourceInterface = instanceClient
if ns := obj.GetNamespace(); ns != "" {
namespacedClient = instanceClient.Namespace(ns)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runtime, has already information about the resource scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Just to confirm — are you suggesting that instead of checking obj.GetNamespace() here, I should pull the scope from the runtime (via RESTMapper / GVK mapping) and choose the correct client accordingly?

If runtime scope is authoritative in this context, I can update the implementation to rely on it and remove this namespaced check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already know the scope at RGD compile time, we can store that information in https://github.com/kubernetes-sigs/kro/blob/main/pkg/graph/graph.go#L30-L31 - which has a namespaced bool https://github.com/kubernetes-sigs/kro/blob/main/pkg/graph/resource.go#L60-L63.

Later we can just retrieve from runtime using https://github.com/kubernetes-sigs/kro/blob/main/pkg/runtime/interfaces.go#L113-L115


updated, err := namespacedClient.Apply(ctx, instancePatch.GetName(), instancePatch,
metav1.ApplyOptions{FieldManager: FieldManagerForLabeler, Force: true})
if err != nil {
return nil, fmt.Errorf("failed to update unmanaged state: %w", err)
}
Expand All @@ -571,26 +579,23 @@ func (igr *instanceGraphReconciler) delayedRequeue(err error) error {
// readExternalRef fetches an external reference from the cluster.
// External references are resources that exist outside of this instance's control.
func (igr *instanceGraphReconciler) readExternalRef(ctx context.Context, resourceID string, resource *unstructured.Unstructured) (*unstructured.Unstructured, error) {
gvk := resource.GroupVersionKind()
restMapping, err := igr.restMapper.RESTMapping(gvk.GroupKind(), gvk.Version)
if err != nil {
return nil, fmt.Errorf("failed to get REST mapping for %v: %w", gvk, err)
}
descriptor := igr.runtime.ResourceDescriptor(resourceID)
gvr := descriptor.GetGroupVersionResource()

var dynResource dynamic.ResourceInterface
if restMapping.Scope.Name() == meta.RESTScopeNameNamespace {
if descriptor.IsNamespaced() {
namespace := igr.getResourceNamespace(resourceID)
dynResource = igr.client.Resource(restMapping.Resource).Namespace(namespace)
dynResource = igr.client.Resource(gvr).Namespace(namespace)
} else {
dynResource = igr.client.Resource(restMapping.Resource)
dynResource = igr.client.Resource(gvr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this change related to allowing cluster-scoped RGDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @tjamet I have added a KREP document (
docs/design/proposals/resource-descriptor-scope-resolution.md
) in the latest commit that details the motivation and design for this change.

}

clusterObj, err := dynResource.Get(ctx, resource.GetName(), metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("failed to get external ref %s/%s: %w", resource.GetNamespace(), resource.GetName(), err)
}

igr.log.V(2).Info("read external ref", "gvk", gvk, "namespace", resource.GetNamespace(), "name", resource.GetName())
igr.log.V(2).Info("read external ref", "gvr", gvr, "namespace", resource.GetNamespace(), "name", resource.GetName())
return clusterObj, nil
}

Expand Down
7 changes: 6 additions & 1 deletion pkg/graph/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,11 @@ func (b *Builder) buildInstanceResource(
rgDefinition *v1alpha1.Schema,
resources map[string]*Resource,
) (*Resource, error) {
scope := extv1.NamespaceScoped
if rgDefinition.Scope == v1alpha1.ScopeCluster {
scope = extv1.ClusterScoped
}

// The instance resource is the resource users will create in their cluster,
// to request the creation of the resources defined in the resource graph definition.
//
Expand All @@ -491,7 +496,7 @@ func (b *Builder) buildInstanceResource(

// Synthesize the CRD for the instance resource.
overrideStatusFields := true
instanceCRD := crd.SynthesizeCRD(group, apiVersion, kind, *instanceSpecSchema, *instanceStatusSchema, overrideStatusFields, rgDefinition.AdditionalPrinterColumns)
instanceCRD := crd.SynthesizeCRD(group, apiVersion, kind, scope, *instanceSpecSchema, *instanceStatusSchema, overrideStatusFields, rgDefinition.AdditionalPrinterColumns)

instanceSchemaExt := instanceCRD.Spec.Versions[0].Schema.OpenAPIV3Schema
instanceSchema, err := schema.ConvertJSONSchemaPropsToSpecSchema(instanceSchemaExt)
Expand Down
8 changes: 4 additions & 4 deletions pkg/graph/crd/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (

// SynthesizeCRD generates a CustomResourceDefinition for a given API version and kind
// with the provided spec and status schemas~
func SynthesizeCRD(group, apiVersion, kind string, spec, status extv1.JSONSchemaProps, statusFieldsOverride bool, additionalPrinterColumns []extv1.CustomResourceColumnDefinition) *extv1.CustomResourceDefinition {
return newCRD(group, apiVersion, kind, newCRDSchema(spec, status, statusFieldsOverride), additionalPrinterColumns)
func SynthesizeCRD(group, apiVersion, kind string, scope extv1.ResourceScope, spec, status extv1.JSONSchemaProps, statusFieldsOverride bool, additionalPrinterColumns []extv1.CustomResourceColumnDefinition) *extv1.CustomResourceDefinition {
return newCRD(group, apiVersion, kind, scope, newCRDSchema(spec, status, statusFieldsOverride), additionalPrinterColumns)
}

func newCRD(group, apiVersion, kind string, schema *extv1.JSONSchemaProps, additionalPrinterColumns []extv1.CustomResourceColumnDefinition) *extv1.CustomResourceDefinition {
func newCRD(group, apiVersion, kind string, scope extv1.ResourceScope, schema *extv1.JSONSchemaProps, additionalPrinterColumns []extv1.CustomResourceColumnDefinition) *extv1.CustomResourceDefinition {
pluralKind := flect.Pluralize(strings.ToLower(kind))
return &extv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -44,7 +44,7 @@ func newCRD(group, apiVersion, kind string, schema *extv1.JSONSchemaProps, addit
Plural: pluralKind,
Singular: strings.ToLower(kind),
},
Scope: extv1.NamespaceScoped,
Scope: scope,
Versions: []extv1.CustomResourceDefinitionVersion{
{
Name: apiVersion,
Expand Down
16 changes: 13 additions & 3 deletions pkg/graph/crd/crd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestSynthesizeCRD(t *testing.T) {
statusFieldsOverride bool
expectedName string
expectedGroup string
scope extv1.ResourceScope
}{
{
name: "standard group and kind",
Expand All @@ -44,6 +45,7 @@ func TestSynthesizeCRD(t *testing.T) {
statusFieldsOverride: true,
expectedName: "widgets.kro.com",
expectedGroup: "kro.com",
scope: extv1.NamespaceScoped,
},
{
name: "mixes case kind",
Expand All @@ -55,17 +57,19 @@ func TestSynthesizeCRD(t *testing.T) {
statusFieldsOverride: true,
expectedName: "databases.kro.com",
expectedGroup: "kro.com",
scope: extv1.ClusterScoped,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
crd := SynthesizeCRD(tt.group, tt.apiVersion, tt.kind, tt.spec, tt.status, tt.statusFieldsOverride, nil)
crd := SynthesizeCRD(tt.group, tt.apiVersion, tt.kind, tt.scope, tt.spec, tt.status, tt.statusFieldsOverride, nil)

assert.Equal(t, tt.expectedName, crd.Name)
assert.Equal(t, tt.expectedGroup, crd.Spec.Group)
assert.Equal(t, tt.kind, crd.Spec.Names.Kind)
assert.Equal(t, tt.kind+"List", crd.Spec.Names.ListKind)
assert.Equal(t, tt.scope, crd.Spec.Scope)

require.Len(t, crd.Spec.Versions, 1)
version := crd.Spec.Versions[0]
Expand Down Expand Up @@ -96,6 +100,7 @@ func TestNewCRD(t *testing.T) {
expectedPlural string
expectedSingular string
expectedPrinterColumns []extv1.CustomResourceColumnDefinition
scope extv1.ResourceScope
}{
{
name: "basic example",
Expand All @@ -107,6 +112,7 @@ func TestNewCRD(t *testing.T) {
expectedPlural: "tests",
expectedSingular: "test",
expectedPrinterColumns: defaultAdditionalPrinterColumns,
scope: extv1.NamespaceScoped,
},
{
name: "uppercase kind",
Expand All @@ -118,6 +124,7 @@ func TestNewCRD(t *testing.T) {
expectedPlural: "configs",
expectedSingular: "config",
expectedPrinterColumns: defaultAdditionalPrinterColumns,
scope: extv1.NamespaceScoped,
},
{
name: "mixed case kind",
Expand All @@ -129,6 +136,7 @@ func TestNewCRD(t *testing.T) {
expectedPlural: "webhooks",
expectedSingular: "webhook",
expectedPrinterColumns: defaultAdditionalPrinterColumns,
scope: extv1.NamespaceScoped,
},
{
name: "non nil empty printer columns",
Expand All @@ -141,6 +149,7 @@ func TestNewCRD(t *testing.T) {
expectedPlural: "webhooks",
expectedSingular: "webhook",
expectedPrinterColumns: defaultAdditionalPrinterColumns,
scope: extv1.NamespaceScoped,
},
{
name: "custom printer columns",
Expand Down Expand Up @@ -175,13 +184,14 @@ func TestNewCRD(t *testing.T) {
JSONPath: ".spec.image",
},
},
scope: extv1.ClusterScoped,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
schema := &extv1.JSONSchemaProps{Type: "object"}
crd := newCRD(tt.group, tt.apiVersion, tt.kind, schema, tt.printerColumns)
crd := newCRD(tt.group, tt.apiVersion, tt.kind, tt.scope, schema, tt.printerColumns)

assert.Equal(t, tt.expectedName, crd.Name)
assert.Equal(t, tt.group, crd.Spec.Group)
Expand All @@ -190,7 +200,7 @@ func TestNewCRD(t *testing.T) {
assert.Equal(t, tt.expectedPlural, crd.Spec.Names.Plural)
assert.Equal(t, tt.expectedSingular, crd.Spec.Names.Singular)

assert.Equal(t, extv1.NamespaceScoped, crd.Spec.Scope)
assert.Equal(t, tt.scope, crd.Spec.Scope)

require.Len(t, crd.Spec.Versions, 1)
assert.Equal(t, tt.apiVersion, crd.Spec.Versions[0].Name)
Expand Down