Skip to content

Commit

Permalink
Add support for external references.
Browse files Browse the repository at this point in the history
- Add ReadOnly flag to resource. True indicates an external resource we want to read (no create/update/delete)
- Reuse template  to describe the external resource (gvkn). Reason: we could templatize the name part of the external resource
- Use as much of existing resource reconcile flow as possible. Reason: we can optionally support ReadyWhen and IncludeWhen rules for the external resource
- Rename WantToCreateResource() to ReadyToProcessResource() to reflect the fact that resources will not be created in case readOnly is set to true
- mark readonly resource as skipped on deletion
  • Loading branch information
barney-s committed Feb 5, 2025
1 parent b9f0419 commit 354e597
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 17 deletions.
5 changes: 4 additions & 1 deletion api/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,11 @@ type Validation struct {
type Resource struct {
// +kubebuilder:validation:Required
ID string `json:"id,omitempty"`
// +kubebuilder:validation:Required
// +kubebuilder:validation:Optional
Template runtime.RawExtension `json:"template,omitempty"`
// ReadOnly indicates an external resource we want to read and use in the Graph
// +kubebuilder:validation:Optional
ReadOnly bool `json:"readOnly,omitempty"`
// +kubebuilder:validation:Optional
ReadyWhen []string `json:"readyWhen,omitempty"`
// +kubebuilder:validation:Optional
Expand Down
5 changes: 4 additions & 1 deletion config/crd/bases/kro.run_resourcegraphdefinitions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ spec:
items:
type: string
type: array
readOnly:
description: ReadOnly indicates an external resource we want
to read and use in the Graph
type: boolean
readyWhen:
items:
type: string
Expand All @@ -88,7 +92,6 @@ spec:
x-kubernetes-preserve-unknown-fields: true
required:
- id
- template
type: object
type: array
schema:
Expand Down
5 changes: 4 additions & 1 deletion helm/crds/kro.run_resourcegraphdefinitions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ spec:
items:
type: string
type: array
readOnly:
description: ReadOnly indicates an external resource we want
to read and use in the Graph
type: boolean
readyWhen:
items:
type: string
Expand All @@ -88,7 +92,6 @@ spec:
x-kubernetes-preserve-unknown-fields: true
required:
- id
- template
type: object
type: array
schema:
Expand Down
24 changes: 21 additions & 3 deletions pkg/controller/instance/controller_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ func (igr *instanceGraphReconciler) reconcileResource(ctx context.Context, resou
resourceState := &ResourceState{State: "IN_PROGRESS"}
igr.state.ResourceStates[resourceID] = resourceState

// Check if resource should be created
if want, err := igr.runtime.WantToCreateResource(resourceID); err != nil || !want {
log.V(1).Info("Skipping resource creation", "reason", err)
// Check if resource should be processed (create or get)
if want, err := igr.runtime.ReadyToProcessResource(resourceID); err != nil || !want {
log.V(1).Info("Skipping resource processing", "reason", err)
resourceState.State = "SKIPPED"
igr.runtime.IgnoreResource(resourceID)
return nil
Expand Down Expand Up @@ -177,6 +177,12 @@ func (igr *instanceGraphReconciler) handleResourceReconciliation(
observed, err := rc.Get(ctx, resource.GetName(), metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
// For read-only resources, we don't create
if igr.runtime.ResourceDescriptor(resourceID).IsReadOnly() {
resourceState.State = "WAITING_FOR_READONLY"
resourceState.Err = fmt.Errorf("read-only resource not found: %w", err)
return igr.delayedRequeue(resourceState.Err)
}
return igr.handleResourceCreation(ctx, rc, resource, resourceID, resourceState)
}
resourceState.State = "ERROR"
Expand All @@ -196,6 +202,12 @@ func (igr *instanceGraphReconciler) handleResourceReconciliation(
}

resourceState.State = "SYNCED"

// For read-only resources, don't perform updates
if igr.runtime.ResourceDescriptor(resourceID).IsReadOnly() {
return nil
}

return igr.updateResource(ctx, rc, resource, observed, resourceID, resourceState)
}

Expand Down Expand Up @@ -354,6 +366,12 @@ func (igr *instanceGraphReconciler) deleteResourcesInOrder(ctx context.Context)
continue
}

// Skip deletion for read-only resources
if igr.runtime.ResourceDescriptor(resourceID).IsReadOnly() {
igr.state.ResourceStates[resourceID].State = "SKIPPED"
continue
}

if err := igr.deleteResource(ctx, resourceID); err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/graph/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ func (b *Builder) buildRGResource(rgResource *v1alpha1.Resource, namespacedResou
includeWhenExpressions: includeWhen,
namespaced: isNamespaced,
order: order,
readOnly: rgResource.ReadOnly,
}, nil
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/graph/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ type Resource struct {
// order reflects the original order in which the resources were specified,
// and lets us keep the client-specified ordering where the dependencies allow.
order int
// readOnly indicates if the resource should only be read and not created/updated
readOnly bool
}

// GetDependencies returns the dependencies of the resource.
Expand Down Expand Up @@ -164,6 +166,11 @@ func (r *Resource) IsNamespaced() bool {
return r.namespaced
}

// IsReadOnly returns whether the resource is read-only
func (r *Resource) IsReadOnly() bool {
return r.readOnly
}

// DeepCopy returns a deep copy of the resource.
func (r *Resource) DeepCopy() *Resource {
return &Resource{
Expand All @@ -177,5 +184,6 @@ func (r *Resource) DeepCopy() *Resource {
readyWhenExpressions: slices.Clone(r.readyWhenExpressions),
includeWhenExpressions: slices.Clone(r.includeWhenExpressions),
namespaced: r.namespaced,
readOnly: r.readOnly,
}
}
8 changes: 6 additions & 2 deletions pkg/runtime/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ type Interface interface {
// IsResourceReady returns true if the resource is ready, and false otherwise.
IsResourceReady(resourceID string) (bool, string, error)

// WantToCreateResource returns true if all the condition expressions return true
// ReadyToProcessResource returns true if all the condition expressions return true
// if not it will add itself to the ignored resources
WantToCreateResource(resourceID string) (bool, error)
ReadyToProcessResource(resourceID string) (bool, error)

// IgnoreResource ignores resource that has a condition expressison that evaluated
// to false
Expand Down Expand Up @@ -116,6 +116,10 @@ type ResourceDescriptor interface {
// IsNamespaced returns true if the resource is namespaced, and false if it's
// cluster-scoped.
IsNamespaced() bool

// IsReadOnly returns true if the resource is marked as read only
// This is used for external references
IsReadOnly() bool
}

// Resource extends `ResourceDescriptor` to include the actual resource data.
Expand Down
4 changes: 2 additions & 2 deletions pkg/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,9 @@ func (rt *ResourceGraphDefinitionRuntime) areDependenciesIgnored(resourceID stri
return false
}

// WantToCreateResource returns true if all the condition expressions return true
// ReadyToProcessResource returns true if all the condition expressions return true
// if not it will add itself to the ignored resources
func (rt *ResourceGraphDefinitionRuntime) WantToCreateResource(resourceID string) (bool, error) {
func (rt *ResourceGraphDefinitionRuntime) ReadyToProcessResource(resourceID string) (bool, error) {
if rt.areDependenciesIgnored(resourceID) {
return false, nil
}
Expand Down
31 changes: 24 additions & 7 deletions pkg/runtime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2228,7 +2228,11 @@ func Test_IsResourceReady(t *testing.T) {
{
name: "multiple expressions all true",
resource: newTestResource(
withReadyExpressions([]string{"test.status.ready", "test.status.healthy && test.status.count > 10", "test.status.count > 5"}),
withReadyExpressions([]string{
"test.status.ready",
"test.status.healthy && test.status.count > 10",
"test.status.count > 5",
}),
),
resolvedObject: map[string]interface{}{
"status": map[string]interface{}{
Expand Down Expand Up @@ -2280,7 +2284,7 @@ func Test_IsResourceReady(t *testing.T) {
})
}
}
func Test_WantToCreateResource(t *testing.T) {
func Test_ReadyToProcessResource(t *testing.T) {
tests := []struct {
name string
resource Resource
Expand Down Expand Up @@ -2368,25 +2372,25 @@ func Test_WantToCreateResource(t *testing.T) {
},
}

got, err := rt.WantToCreateResource("test")
got, err := rt.ReadyToProcessResource("test")
if tt.wantErr {
if err == nil {
t.Error("WantToCreateResource() expected error, got none")
t.Error("ReadyToProcessResource() expected error, got none")
}
return
}
if tt.wantSkip {
if err == nil || !strings.Contains(err.Error(), "Skipping resource creation due to condition") {
t.Errorf("WantToCreateResource() expected skip message, got %v", err)
t.Errorf("ReadyToProcessResource() expected skip message, got %v", err)
}
return
}
if err != nil {
t.Errorf("WantToCreateResource() unexpected error = %v", err)
t.Errorf("ReadyToProcessResource() unexpected error = %v", err)
return
}
if got != tt.want {
t.Errorf("WantToCreateResource() = %v, want %v", got, tt.want)
t.Errorf("ReadyToProcessResource() = %v, want %v", got, tt.want)
}
})
}
Expand Down Expand Up @@ -2613,6 +2617,7 @@ type mockResource struct {
conditions []string
topLevelFields []string
namespaced bool
readOnly bool
obj *unstructured.Unstructured
}

Expand Down Expand Up @@ -2656,6 +2661,10 @@ func (m *mockResource) Unstructured() *unstructured.Unstructured {
return m.obj
}

func (m *mockResource) IsReadOnly() bool {
return m.readOnly
}

type mockResourceOption func(*mockResource)

/* func withGVR(group, version, resource string) mockResourceOption {
Expand Down Expand Up @@ -2686,6 +2695,14 @@ func withReadyExpressions(exprs []string) mockResourceOption {
}
}

/*
func withReadOnly(ro bool) mockResourceOption {
return func(m *mockResource) {
m.readOnly = ro
}
}
*/

func withConditions(conditions []string) mockResourceOption {
return func(m *mockResource) {
m.conditions = conditions
Expand Down

0 comments on commit 354e597

Please sign in to comment.