diff --git a/README.md b/README.md index 1b37e8e..64476e5 100644 --- a/README.md +++ b/README.md @@ -417,7 +417,7 @@ func (r *DeploymentResource) Mutate(current client.Object) error { // 1. Apply core desired state to the current object. // This ensures that the base fields are always correct before features apply their changes. - r.applyCoreDesiredState(current) + r.applyCoreDesiredState(currentDeployment) // 2. Apply feature mutations via a restricted mutator interface // We've applied all desired fields from the core object and can now continue working @@ -810,7 +810,7 @@ func (r *DeploymentResource) Mutate(current client.Object) error { // 1. Apply core desired state to the current object. // This ensures that the base fields are always correct before features apply their changes. - r.applyCoreDesiredState(current) + r.applyCoreDesiredState(currentDeployment) // 2. Apply feature mutations via a restricted mutator interface // We've applied all desired fields from the core object and can now continue working diff --git a/examples/component-architecture-basics/resources/deployment_builder.go b/examples/component-architecture-basics/resources/deployment_builder.go index ac17863..3a2a04e 100644 --- a/examples/component-architecture-basics/resources/deployment_builder.go +++ b/examples/component-architecture-basics/resources/deployment_builder.go @@ -21,7 +21,7 @@ type DeploymentBuilder struct { func NewDeploymentBuilder(deployment *appsv1.Deployment) *DeploymentBuilder { return &DeploymentBuilder{ res: &DeploymentResource{ - desired: deployment, + deployment: deployment, }, } } @@ -105,14 +105,14 @@ func (b *DeploymentBuilder) WithDataExtractor( // Build finalizes and returns the configured DeploymentResource. // It returns an error if the desired is nil, or if it lacks a name or namespace. func (b *DeploymentBuilder) Build() (*DeploymentResource, error) { - if b.res.desired == nil { - return nil, errors.New("desired cannot be nil") + if b.res.deployment == nil { + return nil, errors.New("deployment cannot be nil") } - if b.res.desired.Name == "" { - return nil, errors.New("desired name cannot be empty") + if b.res.deployment.Name == "" { + return nil, errors.New("deployment name cannot be empty") } - if b.res.desired.Namespace == "" { - return nil, errors.New("desired namespace cannot be empty") + if b.res.deployment.Namespace == "" { + return nil, errors.New("deployment namespace cannot be empty") } return b.res, nil } diff --git a/examples/component-architecture-basics/resources/deployment_resource.go b/examples/component-architecture-basics/resources/deployment_resource.go index 589b015..61f409d 100644 --- a/examples/component-architecture-basics/resources/deployment_resource.go +++ b/examples/component-architecture-basics/resources/deployment_resource.go @@ -2,6 +2,7 @@ package resources import ( "fmt" + "maps" "github.com/sourcehawk/operator-component-framework/pkg/feature" appsv1 "k8s.io/api/apps/v1" @@ -18,8 +19,9 @@ import ( // This abstraction exists to decouple the reconciliation logic from the specific // Kubernetes type, allowing the framework to handle lifecycle and status aggregation. type DeploymentResource struct { - // desired is the underlying Kubernetes object. - desired *appsv1.Deployment + // deployment becomes the underlying Kubernetes object after a reconcile and is + // the desired core object before a reconcile (more concretely before a successful return from Mutate). + deployment *appsv1.Deployment // mutations is a list of feature-gated changes to apply to the resource. mutations []feature.Mutation[*DeploymentResourceMutator] @@ -43,12 +45,13 @@ type DeploymentResource struct { // Identity returns a unique identifier for the resource, used by the framework for logging and tracking. func (r *DeploymentResource) Identity() string { - return fmt.Sprintf("apps/v1/Deployment/%s", r.desired.Name) + return fmt.Sprintf("apps/v1/Deployment/%s", r.deployment.Name) } -// DesiredDefaultObject returns the underlying Kubernetes object. It implements the Resource interface. -func (r *DeploymentResource) DesiredDefaultObject() (client.Object, error) { - return r.desired.DeepCopy(), nil +// Object returns a copy of the underlying Kubernetes object after reconcile or the unchanged desired object +// before a reconcile happens. It implements the Resource interface. +func (r *DeploymentResource) Object() (client.Object, error) { + return r.deployment.DeepCopy(), nil } // Mutate applies the desired state to the resource, including Feature Mutations. @@ -90,7 +93,7 @@ func (r *DeploymentResource) Mutate(current client.Object) error { // 4. Update internal desired state with the mutated current object. // This ensures that subsequent calls to ConvergingStatus and ExtractData // use the fully mutated state, including status. - r.desired = currentDeployment.DeepCopy() + r.deployment = currentDeployment.DeepCopy() return nil } @@ -106,43 +109,39 @@ func (r *DeploymentResource) applyCoreDesiredState(current *appsv1.Deployment) { if current.Labels == nil { current.Labels = make(map[string]string) } - for k, v := range r.desired.Labels { - current.Labels[k] = v - } + maps.Copy(current.Labels, r.deployment.Labels) if current.Annotations == nil { current.Annotations = make(map[string]string) } - for k, v := range r.desired.Annotations { - current.Annotations[k] = v - } + maps.Copy(current.Annotations, r.deployment.Annotations) // Apply Spec fields - current.Spec.Replicas = r.desired.Spec.Replicas - current.Spec.Selector = r.desired.Spec.Selector - current.Spec.Template.Labels = r.desired.Spec.Template.Labels - current.Spec.Template.Annotations = r.desired.Spec.Template.Annotations + current.Spec.Replicas = r.deployment.Spec.Replicas + current.Spec.Selector = r.deployment.Spec.Selector + current.Spec.Template.Labels = r.deployment.Spec.Template.Labels + current.Spec.Template.Annotations = r.deployment.Spec.Template.Annotations // For containers, we might want a more sophisticated merge, but for this example // we just ensure the core containers exist with their base settings. // This is where the "core" part of the resource definition is enforced. - current.Spec.Template.Spec.Containers = make([]corev1.Container, len(r.desired.Spec.Template.Spec.Containers)) - copy(current.Spec.Template.Spec.Containers, r.desired.Spec.Template.Spec.Containers) + current.Spec.Template.Spec.Containers = make([]corev1.Container, len(r.deployment.Spec.Template.Spec.Containers)) + copy(current.Spec.Template.Spec.Containers, r.deployment.Spec.Template.Spec.Containers) } // ConvergingStatus reports the progress of the resource toward its desired state. // It implements the Alive interface, allowing the Component to aggregate status. func (r *DeploymentResource) ConvergingStatus(op component.ConvergingOperation) (component.ConvergingStatusWithReason, error) { if r.convergeStatusHandler != nil { - return r.convergeStatusHandler(op, r.desired) + return r.convergeStatusHandler(op, r.deployment) } desiredReplicas := int32(1) - if r.desired.Spec.Replicas != nil { - desiredReplicas = *r.desired.Spec.Replicas + if r.deployment.Spec.Replicas != nil { + desiredReplicas = *r.deployment.Spec.Replicas } - if r.desired.Status.ReadyReplicas == desiredReplicas { + if r.deployment.Status.ReadyReplicas == desiredReplicas { return component.ConvergingStatusWithReason{ Status: component.ConvergingStatusReady, Reason: "All replicas are ready", @@ -163,7 +162,7 @@ func (r *DeploymentResource) ConvergingStatus(op component.ConvergingOperation) Status: status, Reason: fmt.Sprintf( "Waiting for replicas: %d/%d ready", - r.desired.Status.ReadyReplicas, + r.deployment.Status.ReadyReplicas, desiredReplicas, ), }, nil @@ -173,10 +172,10 @@ func (r *DeploymentResource) ConvergingStatus(op component.ConvergingOperation) // It's part of the Alive interface used for sophisticated status reporting. func (r *DeploymentResource) GraceStatus() (component.GraceStatusWithReason, error) { if r.graceStatusHandler != nil { - return r.graceStatusHandler(r.desired) + return r.graceStatusHandler(r.deployment) } - if r.desired.Status.ReadyReplicas > 0 { + if r.deployment.Status.ReadyReplicas > 0 { return component.GraceStatusWithReason{ Status: component.GraceStatusDegraded, Reason: "Deployment partially available", @@ -192,13 +191,13 @@ func (r *DeploymentResource) GraceStatus() (component.GraceStatusWithReason, err // IsSuspended checks if the resource is currently in a suspended state (e.g., scaled to 0). // It implements the Suspendable interface. func (r *DeploymentResource) IsSuspended() bool { - return r.desired.Spec.Replicas != nil && *r.desired.Spec.Replicas == 0 + return r.deployment.Spec.Replicas != nil && *r.deployment.Spec.Replicas == 0 } // DeleteOnSuspend determines if the resource should be deleted when the component is suspended. func (r *DeploymentResource) DeleteOnSuspend() bool { if r.suspendDeletionDecisionHandler != nil { - return r.suspendDeletionDecisionHandler(r.desired) + return r.suspendDeletionDecisionHandler(r.deployment) } return false } @@ -227,10 +226,10 @@ func (r *DeploymentResource) Suspend() error { // SuspensionStatus reports the progress of the resource toward a suspended state. func (r *DeploymentResource) SuspensionStatus() (component.SuspensionStatusWithReason, error) { if r.suspendStatusHandler != nil { - return r.suspendStatusHandler(r.desired) + return r.suspendStatusHandler(r.deployment) } - if r.desired.Status.Replicas == 0 { + if r.deployment.Status.Replicas == 0 { return component.SuspensionStatusWithReason{ Status: component.SuspensionStatusSuspended, Reason: "Deployment scaled to zero", @@ -241,7 +240,7 @@ func (r *DeploymentResource) SuspensionStatus() (component.SuspensionStatusWithR Status: component.SuspensionStatusSuspending, Reason: fmt.Sprintf( "Waiting for replicas to scale down, %d replicas still running.", - r.desired.Status.Replicas, + r.deployment.Status.Replicas, ), }, nil } @@ -250,7 +249,7 @@ func (r *DeploymentResource) SuspensionStatus() (component.SuspensionStatusWithR // It implements the DataExtractable interface. func (r *DeploymentResource) ExtractData() error { // We enure no data mutations are applied by extractors - deploymentCopy := r.desired.DeepCopy() + deploymentCopy := r.deployment.DeepCopy() for _, extractor := range r.dataExtractors { if extractor == nil { diff --git a/pkg/component/component_test.go b/pkg/component/component_test.go index 44a86db..9a74cd4 100644 --- a/pkg/component/component_test.go +++ b/pkg/component/component_test.go @@ -97,7 +97,7 @@ var _ = Describe("Component Reconciler", func() { Data: map[string]string{"foo": "bar"}, } res := &MockResource{} - res.On("DesiredDefaultObject").Return(cm, nil) + res.On("Object").Return(cm, nil) res.On("Identity").Return("ConfigMap/test-cm") res.On("Mutate", mock.Anything).Return(nil) @@ -130,7 +130,7 @@ var _ = Describe("Component Reconciler", func() { }, } res := &MockAliveResource{} - res.On("DesiredDefaultObject").Return(cm, nil) + res.On("Object").Return(cm, nil) res.On("Identity").Return("ConfigMap/test-alive-cm") res.On("Mutate", mock.Anything).Return(nil) res.On("ConvergingStatus", ConvergingOperationCreated).Return(ConvergingStatusWithReason{ @@ -164,7 +164,7 @@ var _ = Describe("Component Reconciler", func() { Expect(k8sClient.Create(ctx, cm)).To(Succeed()) res := &MockAliveResource{} - res.On("DesiredDefaultObject").Return(cm, nil) + res.On("Object").Return(cm, nil) res.On("Identity").Return("ConfigMap/test-readonly-cm") res.On("ConvergingStatus", ConvergingOperationNone).Return(ConvergingStatusWithReason{ Status: ConvergingStatusReady, @@ -191,7 +191,7 @@ var _ = Describe("Component Reconciler", func() { ObjectMeta: metav1.ObjectMeta{Name: "create-cm", Namespace: namespace}, } res1 := &MockAliveResource{} - res1.On("DesiredDefaultObject").Return(cm1, nil) + res1.On("Object").Return(cm1, nil) res1.On("Identity").Return("ConfigMap/create-cm") res1.On("Mutate", mock.Anything).Return(nil) res1.On("ConvergingStatus", ConvergingOperationCreated).Return(ConvergingStatusWithReason{ @@ -205,7 +205,7 @@ var _ = Describe("Component Reconciler", func() { Expect(k8sClient.Create(ctx, cm2)).To(Succeed()) res2 := &MockAliveResource{} - res2.On("DesiredDefaultObject").Return(cm2, nil) + res2.On("Object").Return(cm2, nil) res2.On("Identity").Return("ConfigMap/read-cm") res2.On("ConvergingStatus", ConvergingOperationNone).Return(ConvergingStatusWithReason{ Status: ConvergingStatusCreating, // Dominant status (False/Creating) @@ -259,13 +259,13 @@ var _ = Describe("Component Reconciler", func() { Data: map[string]string{"foo": "bar"}, } createRes := &MockResource{} - createRes.On("DesiredDefaultObject").Return(cm, nil) + createRes.On("Object").Return(cm, nil) createRes.On("Identity").Return("ConfigMap/should-not-be-created") createRes.On("Mutate", mock.Anything).Return(nil) // Set up suspendable resource suspendRes := &MockSuspendableResource{} - suspendRes.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + suspendRes.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "suspend-me", Namespace: namespace}, }, nil) suspendRes.On("Identity").Return("suspend-me") @@ -309,7 +309,7 @@ var _ = Describe("Component Reconciler", func() { Expect(k8sClient.Create(ctx, cm)).To(Succeed()) res := &MockSuspendableResource{} - res.On("DesiredDefaultObject").Return(cm, nil) + res.On("Object").Return(cm, nil) res.On("Identity").Return("ConfigMap/test-suspended-cm") res.On("Suspend").Return(nil) res.On("Mutate", mock.Anything).Return(nil) @@ -352,7 +352,7 @@ var _ = Describe("Component Reconciler", func() { Expect(k8sClient.Create(ctx, cm)).To(Succeed()) res := &MockResource{} - res.On("DesiredDefaultObject").Return(cm, nil) + res.On("Object").Return(cm, nil) res.On("Identity").Return("ConfigMap/to-be-deleted") comp.deleteResources = []Resource{res} @@ -375,7 +375,7 @@ var _ = Describe("Component Reconciler", func() { It("should update status to Error when reconciliation fails", func() { // Given res := &MockResource{} - res.On("DesiredDefaultObject").Return(nil, fmt.Errorf("reconciliation error")) + res.On("Object").Return(nil, fmt.Errorf("reconciliation error")) res.On("Identity").Return("failing-resource") comp.createResources = []Resource{res} @@ -397,7 +397,7 @@ var _ = Describe("Component Reconciler", func() { It("should handle errors during read-only resource retrieval", func() { // Given res := &MockResource{} - res.On("DesiredDefaultObject").Return(nil, fmt.Errorf("read error")) + res.On("Object").Return(nil, fmt.Errorf("read error")) res.On("Identity").Return("failing-read-resource") comp.readResources = []Resource{res} @@ -418,7 +418,7 @@ var _ = Describe("Component Reconciler", func() { It("should handle errors during resource deletion in normal flow", func() { // Given res := &MockResource{} - res.On("DesiredDefaultObject").Return(nil, fmt.Errorf("delete object error")) + res.On("Object").Return(nil, fmt.Errorf("delete object error")) res.On("Identity").Return("failing-delete-resource") comp.deleteResources = []Resource{res} @@ -463,7 +463,7 @@ var _ = Describe("Component Reconciler", func() { comp.suspended = true // A resource that suspends successfully susRes := &MockSuspendableResource{} - susRes.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + susRes.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "suspend-ok", Namespace: namespace}, }, nil) susRes.On("Identity").Return("suspend-ok") @@ -474,7 +474,7 @@ var _ = Describe("Component Reconciler", func() { // A resource that fails deletion delRes := &MockResource{} - delRes.On("DesiredDefaultObject").Return(nil, fmt.Errorf("suspend-delete error")) + delRes.On("Object").Return(nil, fmt.Errorf("suspend-delete error")) delRes.On("Identity").Return("failing-suspend-delete-resource") comp.createResources = []Resource{susRes} @@ -498,7 +498,7 @@ var _ = Describe("Component Reconciler", func() { It("should execute extraction during normal reconcile flow", func() { // Given res := &MockExtractableResource{} - res.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + res.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: namespace}, }, nil) res.On("Identity").Return("ConfigMap/test-cm") @@ -525,7 +525,7 @@ var _ = Describe("Component Reconciler", func() { // Let's create a combined mock or just use MockExtractableResource and see if it's called. // Reconcile checks c.suspended FIRST. - res.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + res.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: namespace}, }, nil) res.On("Identity").Return("ConfigMap/test-cm") @@ -549,7 +549,7 @@ var _ = Describe("Component Reconciler", func() { It("should propagate extraction errors and set status to Error", func() { // Given res := &MockExtractableResource{} - res.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + res.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: namespace}, }, nil) res.On("Identity").Return("ConfigMap/test-cm") @@ -573,7 +573,7 @@ var _ = Describe("Component Reconciler", func() { It("should handle multiple resources with and without DataExtractable", func() { // Given res1 := &MockExtractableResource{} - res1.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + res1.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "res1", Namespace: namespace}, }, nil) res1.On("Identity").Return("ConfigMap/res1") @@ -581,7 +581,7 @@ var _ = Describe("Component Reconciler", func() { res1.On("ExtractData").Return(nil) res2 := &MockResource{} - res2.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + res2.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "res2", Namespace: namespace}, }, nil) res2.On("Identity").Return("ConfigMap/res2") @@ -609,7 +609,7 @@ var _ = Describe("Component Reconciler", func() { return nil }, } - res.On("DesiredDefaultObject").Return(&corev1.ConfigMap{ + res.On("Object").Return(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "ext-cm", Namespace: namespace}, }, nil) res.On("Identity").Return("ConfigMap/ext-cm") diff --git a/pkg/component/create.go b/pkg/component/create.go index 0ff659d..27b1415 100644 --- a/pkg/component/create.go +++ b/pkg/component/create.go @@ -25,7 +25,7 @@ import ( // - If the object doesn't exist, MutateFn will be called, and it will be created. // - If the object exists, CreateOrUpdate first populates the object with the server state, // then calls MutateFn. -// - If the object did not exist, the object remains the one returned by DesiredDefaultObject(), +// - If the object did not exist, the object remains the one returned by Object(), // typically just with name/namespace/type metadata set. // // Returns a slice of convergingResults for all processed Alive resources, or an error if @@ -37,7 +37,7 @@ func createOrUpdateResources( for _, resource := range resources { // Create or update resources - obj, err := resource.DesiredDefaultObject() + obj, err := resource.Object() if err != nil { return nil, fmt.Errorf( "failed to retrieve object for resource %s: %w", resource.Identity(), err, @@ -80,7 +80,7 @@ func createOrUpdateResources( // 1. Mutations: Calls Mutate() to apply all fields to the object. // The `obj` passed here is the same object instance that `ctrl.CreateOrUpdate` works with. // If the object exists, it is pre-populated with server state; if not, it is the original -// object from `resource.DesiredDefaultObject()`. +// object from `resource.Object()`. // 2. Ownership: Ensures the object has a controller reference pointing to the owner CRD. func mutateResource(resource Resource, obj client.Object, owner client.Object, scheme *runtime.Scheme) error { if err := resource.Mutate(obj); err != nil { diff --git a/pkg/component/create_test.go b/pkg/component/create_test.go index 7f18efe..f5eaa2b 100644 --- a/pkg/component/create_test.go +++ b/pkg/component/create_test.go @@ -39,7 +39,7 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } resource := &MockResource{} - resource.On("DesiredDefaultObject").Return(resourceObject, nil) + resource.On("Object").Return(resourceObject, nil) resource.On("Mutate", mock.Anything).Return(nil) // When @@ -66,7 +66,7 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } resource1 := &MockResource{} - resource1.On("DesiredDefaultObject").Return(resourceObject1, nil) + resource1.On("Object").Return(resourceObject1, nil) resource1.On("Mutate", mock.Anything).Return(nil) resourceObject2 := &corev1.ConfigMap{ @@ -76,7 +76,7 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } resource2 := &MockResource{} - resource2.On("DesiredDefaultObject").Return(resourceObject2, nil) + resource2.On("Object").Return(resourceObject2, nil) resource2.On("Mutate", mock.Anything).Return(nil) // When @@ -106,7 +106,7 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } regularResource := &MockResource{} - regularResource.On("DesiredDefaultObject").Return(regularResourceObject, nil) + regularResource.On("Object").Return(regularResourceObject, nil) regularResource.On("Mutate", mock.Anything).Return(nil) aliveResourceObject := &corev1.ConfigMap{ @@ -116,7 +116,7 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } aliveResource := &MockAliveResource{} - aliveResource.On("DesiredDefaultObject").Return(aliveResourceObject, nil) + aliveResource.On("Object").Return(aliveResourceObject, nil) aliveResource.On("Mutate", mock.Anything).Return(nil) aliveResource.On("ConvergingStatus", mock.Anything).Return(ConvergingStatusWithReason{ Status: ConvergingStatusReady, @@ -145,12 +145,12 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } resource1 := &MockResource{} - resource1.On("DesiredDefaultObject").Return(resourceObject1, nil) + resource1.On("Object").Return(resourceObject1, nil) resource1.On("Mutate", mock.Anything).Return(nil) resource2 := &MockResource{} resource2.On("Identity").Return("v1/ConfigMap/failed-resource") - resource2.On("DesiredDefaultObject").Return(nil, fmt.Errorf("resource 2 error")) + resource2.On("Object").Return(nil, fmt.Errorf("resource 2 error")) resource3 := &MockResource{} // Should not be processed @@ -170,7 +170,7 @@ func TestCreateOrUpdateResources(t *testing.T) { resource1.AssertExpectations(t) resource2.AssertExpectations(t) // Explicitly verify subsequent resources are not touched - resource3.AssertNotCalled(t, "DesiredDefaultObject") + resource3.AssertNotCalled(t, "Object") }) t.Run("should successfully update a resource", func(t *testing.T) { @@ -189,7 +189,7 @@ func TestCreateOrUpdateResources(t *testing.T) { updatedResourceObject.Data["foo"] = "baz" resource := &MockResource{} - resource.On("DesiredDefaultObject").Return(configMap, nil) + resource.On("Object").Return(configMap, nil) resource.On("Mutate", mock.Anything).Run(func(args mock.Arguments) { obj := args.Get(0).(*corev1.ConfigMap) obj.Data["foo"] = "baz" @@ -218,7 +218,7 @@ func TestCreateOrUpdateResources(t *testing.T) { }, } resource := &MockAliveResource{} - resource.On("DesiredDefaultObject").Return(resourceObject, nil) + resource.On("Object").Return(resourceObject, nil) resource.On("Mutate", mock.Anything).Return(nil) resource.On("ConvergingStatus", mock.Anything).Return(ConvergingStatusWithReason{ Status: ConvergingStatusReady, @@ -236,11 +236,11 @@ func TestCreateOrUpdateResources(t *testing.T) { resource.AssertExpectations(t) }) - t.Run("should return error if resource.DesiredDefaultObject() fails", func(t *testing.T) { + t.Run("should return error if resource.Object() fails", func(t *testing.T) { // Given resource := &MockResource{} resource.On("Identity").Return("v1/ConfigMap/failed-resource") - resource.On("DesiredDefaultObject").Return(nil, fmt.Errorf("object error")) + resource.On("Object").Return(nil, fmt.Errorf("object error")) // When _, err := createOrUpdateResources(ctx, reconcileContext, []Resource{resource}) @@ -260,7 +260,7 @@ func TestCreateOrUpdateResources(t *testing.T) { } resource := &MockResource{} resource.On("Identity").Return("v1/ConfigMap/error-cm") - resource.On("DesiredDefaultObject").Return(resourceObject, nil) + resource.On("Object").Return(resourceObject, nil) resource.On("Mutate", mock.Anything).Return(fmt.Errorf("mutation failed")) // When diff --git a/pkg/component/delete.go b/pkg/component/delete.go index 5911c8e..32c23d9 100644 --- a/pkg/component/delete.go +++ b/pkg/component/delete.go @@ -32,7 +32,7 @@ func deleteResources( var errs []error for _, resource := range resources { - object, err := resource.DesiredDefaultObject() + object, err := resource.Object() if err != nil { errs = append(errs, fmt.Errorf( "failed to get resource %s's underlying object on deletion: %w", diff --git a/pkg/component/delete_test.go b/pkg/component/delete_test.go index bf52d40..4b4c460 100644 --- a/pkg/component/delete_test.go +++ b/pkg/component/delete_test.go @@ -42,7 +42,7 @@ func TestDeleteResources(t *testing.T) { require.NoError(t, err) resource := &MockResource{} - resource.On("DesiredDefaultObject").Return(resourceObject, nil) + resource.On("Object").Return(resourceObject, nil) resource.On("Identity").Return("v1/ConfigMap/test-cm") // When @@ -70,7 +70,7 @@ func TestDeleteResources(t *testing.T) { // Resource is NOT created in fakeClient resource := &MockResource{} - resource.On("DesiredDefaultObject").Return(resourceObject, nil) + resource.On("Object").Return(resourceObject, nil) // When err := deleteResources(ctx, reconcileContext, []Resource{resource}) @@ -80,10 +80,10 @@ func TestDeleteResources(t *testing.T) { resource.AssertExpectations(t) }) - t.Run("should collect errors from Resource.DesiredDefaultObject() and continue", func(t *testing.T) { + t.Run("should collect errors from Resource.Object() and continue", func(t *testing.T) { // Given resource1 := &MockResource{} - resource1.On("DesiredDefaultObject").Return(nil, errors.New("failed to get object")) + resource1.On("Object").Return(nil, errors.New("failed to get object")) resource1.On("Identity").Return("v1/ConfigMap/failed-resource") resourceObject2 := &corev1.ConfigMap{ @@ -96,7 +96,7 @@ func TestDeleteResources(t *testing.T) { require.NoError(t, err) resource2 := &MockResource{} - resource2.On("DesiredDefaultObject").Return(resourceObject2, nil) + resource2.On("Object").Return(resourceObject2, nil) resource2.On("Identity").Return("v1/ConfigMap/test-cm-2") // When @@ -132,7 +132,7 @@ func TestDeleteResources(t *testing.T) { mockClient.On("Delete", ctx, resourceObject1, mock.Anything).Return(errors.New("forbidden")) resource1 := &MockResource{} - resource1.On("DesiredDefaultObject").Return(resourceObject1, nil) + resource1.On("Object").Return(resourceObject1, nil) resource1.On("Identity").Return("v1/ConfigMap/test-cm-1") resourceObject2 := &corev1.ConfigMap{ @@ -144,7 +144,7 @@ func TestDeleteResources(t *testing.T) { mockClient.On("Delete", ctx, resourceObject2, mock.Anything).Return(nil) resource2 := &MockResource{} - resource2.On("DesiredDefaultObject").Return(resourceObject2, nil) + resource2.On("Object").Return(resourceObject2, nil) resource2.On("Identity").Return("v1/ConfigMap/test-cm-2") // When diff --git a/pkg/component/read.go b/pkg/component/read.go index e50363c..ecd59d9 100644 --- a/pkg/component/read.go +++ b/pkg/component/read.go @@ -20,7 +20,7 @@ func readResources( for _, resource := range resources { // Get readonly resources - object, err := resource.DesiredDefaultObject() + object, err := resource.Object() if err != nil { return nil, fmt.Errorf( "failed to retrieve read-only object from resource %s: %w", diff --git a/pkg/component/read_test.go b/pkg/component/read_test.go index 8ce1a7a..2415e29 100644 --- a/pkg/component/read_test.go +++ b/pkg/component/read_test.go @@ -49,11 +49,11 @@ func TestReadResources(t *testing.T) { // Resource 1: not Alive resource1 := &MockResource{} - resource1.On("DesiredDefaultObject").Return(cm, nil) + resource1.On("Object").Return(cm, nil) // Resource 2: Alive resource2 := &MockAliveResource{} - resource2.On("DesiredDefaultObject").Return(secret, nil) + resource2.On("Object").Return(secret, nil) resource2.On("ConvergingStatus", ConvergingOperationNone).Return(ConvergingStatusWithReason{ Status: ConvergingStatusReady, Reason: "Secret is ready", @@ -72,10 +72,10 @@ func TestReadResources(t *testing.T) { resource2.AssertExpectations(t) }) - t.Run("should return error if resource.DesiredDefaultObject() fails", func(t *testing.T) { + t.Run("should return error if resource.Object() fails", func(t *testing.T) { // Given resource := &MockResource{} - resource.On("DesiredDefaultObject").Return(nil, errors.New("failed to get object")) + resource.On("Object").Return(nil, errors.New("failed to get object")) resource.On("Identity").Return("v1/ConfigMap/failed-resource") // When @@ -98,7 +98,7 @@ func TestReadResources(t *testing.T) { }, } resource := &MockResource{} - resource.On("DesiredDefaultObject").Return(cm, nil) + resource.On("Object").Return(cm, nil) resource.On("Identity").Return("v1/ConfigMap/missing-cm") // When @@ -124,7 +124,7 @@ func TestReadResources(t *testing.T) { require.NoError(t, err) resource := &MockAliveResource{} - resource.On("DesiredDefaultObject").Return(cm, nil) + resource.On("Object").Return(cm, nil) resource.On("Identity").Return("v1/ConfigMap/test-cm-alive-fail") resource.On("ConvergingStatus", ConvergingOperationNone).Return(ConvergingStatusWithReason{}, errors.New("failed status")) diff --git a/pkg/component/resource.go b/pkg/component/resource.go index 222a7d1..56d5394 100644 --- a/pkg/component/resource.go +++ b/pkg/component/resource.go @@ -21,15 +21,18 @@ type Resource interface { // // The mutations are applied every time the component is reconciled. Mutate(current client.Object) error - // DesiredDefaultObject returns a copy of the **desired** core resource object. - // This object represents the baseline state of the resource before any feature-driven + // Object returns a copy of the managed resource object. + // + // Before reconciliation this object represents the baseline state of the resource before any feature-driven // mutations or side effects are applied. // - // It is used as the starting point for reconciliation: - // - When a resource is created, this object (plus mutations from Mutate) defines the initial state. - // - When a resource is updated, this object provides the "core" desired fields that the - // Resource implementation must apply to the current server state during Mutate. - DesiredDefaultObject() (client.Object, error) + // After reconciliation this object represents what is applied on the kubernetes api after mutations and side + // effects have been applied. + // + // Note that this method is mainly exposed for the Component reconciler and implementers should generally refrain + // from accessing this and gathering required data from applied kubernetes objects by implementing idioms provided + // by the module. E.g. data extractors provided by the DataExtractable interface. + Object() (client.Object, error) // Identity returns a unique identifier for the resource in the format //. Identity() string } diff --git a/pkg/component/suspend.go b/pkg/component/suspend.go index 8412d6c..e647969 100644 --- a/pkg/component/suspend.go +++ b/pkg/component/suspend.go @@ -92,7 +92,7 @@ func suspendResource( } // Get the object if possible - object, err := resource.DesiredDefaultObject() + object, err := resource.Object() if err != nil { return SuspensionStatusWithReason{}, fmt.Errorf("failed to get object on suspension: %w", err) } diff --git a/pkg/component/suspend_test.go b/pkg/component/suspend_test.go index 7b7731e..387e373 100644 --- a/pkg/component/suspend_test.go +++ b/pkg/component/suspend_test.go @@ -51,7 +51,7 @@ func setupMockResource(name string, status SuspensionStatus, reason string, dele obj := &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}} res.On("Suspend").Return(nil) - res.On("DesiredDefaultObject").Return(obj, nil) + res.On("Object").Return(obj, nil) res.On("Identity").Return(name) res.On("SuspensionStatus").Return(SuspensionStatusWithReason{Status: status, Reason: reason}, nil) res.On("DeleteOnSuspend").Return(deleteOnSuspend) @@ -292,12 +292,12 @@ func TestSuspendResource(t *testing.T) { assert.Contains(t, err.Error(), "suspend fail") }) - t.Run("should return error if resource.DesiredDefaultObject() fails", func(t *testing.T) { + t.Run("should return error if resource.Object() fails", func(t *testing.T) { cli := &MockClient{} rec := setupReconcileContext(scheme, nil, cli) res := &MockSuspendableResource{} res.On("Suspend").Return(nil) - res.On("DesiredDefaultObject").Return(nil, errors.New("object fail")) + res.On("Object").Return(nil, errors.New("object fail")) _, err := suspendResource(ctx, rec, res, res) require.Error(t, err) diff --git a/pkg/component/zz_mock_resource_test.go b/pkg/component/zz_mock_resource_test.go index d121104..57e8cea 100644 --- a/pkg/component/zz_mock_resource_test.go +++ b/pkg/component/zz_mock_resource_test.go @@ -16,7 +16,7 @@ func (m *MockResource) Mutate(current client.Object) error { return args.Error(0) } -func (m *MockResource) DesiredDefaultObject() (client.Object, error) { +func (m *MockResource) Object() (client.Object, error) { args := m.Called() obj := args.Get(0) if obj == nil {