diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java index 8611950e04..c9c1d770c5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java @@ -33,6 +33,7 @@ public ReconcileResult reconcile(P primary, Context

context) { if (maybeActual.isEmpty()) { if (creatable) { var desired = desired(primary, context); + throwIfNull(desired, primary, "Desired"); logForOperation("Creating", primary, desired); var createdResource = handleCreate(desired, primary, context); return ReconcileResult.resourceCreated(createdResource); @@ -43,6 +44,7 @@ public ReconcileResult reconcile(P primary, Context

context) { final var match = updater.match(actual, primary, context); if (!match.matched()) { final var desired = match.computedDesired().orElse(desired(primary, context)); + throwIfNull(desired, primary, "Desired"); logForOperation("Updating", primary, desired); var updatedResource = handleUpdate(actual, desired, primary, context); return ReconcileResult.resourceUpdated(updatedResource); @@ -59,6 +61,13 @@ public ReconcileResult reconcile(P primary, Context

context) { return ReconcileResult.noOperation(maybeActual.orElse(null)); } + private void throwIfNull(R desired, P primary, String descriptor) { + if (desired == null) { + throw new DependentResourceException( + descriptor + " cannot be null. Primary ID: " + ResourceID.fromResource(primary)); + } + } + private void logForOperation(String operation, P primary, R desired) { final var desiredDesc = desired instanceof HasMetadata ? "'" + ((HasMetadata) desired).getMetadata().getName() + "' " @@ -71,6 +80,7 @@ private void logForOperation(String operation, P primary, R desired) { protected R handleCreate(R desired, P primary, Context

context) { ResourceID resourceID = ResourceID.fromResource(primary); R created = creator.create(desired, primary, context); + throwIfNull(created, primary, "Created resource"); onCreated(resourceID, created); return created; } @@ -98,6 +108,7 @@ protected R handleCreate(R desired, P primary, Context

context) { protected R handleUpdate(R actual, R desired, P primary, Context

context) { ResourceID resourceID = ResourceID.fromResource(primary); R updated = updater.update(actual, desired, primary, context); + throwIfNull(updated, primary, "Updated resource"); onUpdated(resourceID, updated, actual); return updated; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceException.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceException.java new file mode 100644 index 0000000000..a69eb1e0a8 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceException.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import io.javaoperatorsdk.operator.OperatorException; + +public class DependentResourceException extends OperatorException { + + public DependentResourceException() {} + + public DependentResourceException(String message) { + super(message); + } + + public DependentResourceException(Throwable cause) { + super(cause); + } + + public DependentResourceException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResourceTest.java new file mode 100644 index 0000000000..4edd3dfb4c --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResourceTest.java @@ -0,0 +1,136 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import java.util.Optional; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class AbstractDependentResourceTest { + + + @Test + void throwsExceptionIfDesiredIsNullOnCreate() { + TestDependentResource testDependentResource = new TestDependentResource(); + testDependentResource.setSecondary(null); + testDependentResource.setDesired(null); + + assertThrows(DependentResourceException.class, + () -> testDependentResource.reconcile(new TestCustomResource(), null)); + + } + + @Test + void throwsExceptionIfDesiredIsNullOnUpdate() { + TestDependentResource testDependentResource = new TestDependentResource(); + testDependentResource.setSecondary(configMap()); + testDependentResource.setDesired(null); + + assertThrows(DependentResourceException.class, + () -> testDependentResource.reconcile(new TestCustomResource(), null)); + } + + @Test + void throwsExceptionIfCreateReturnsNull() { + TestDependentResource testDependentResource = new TestDependentResource(); + testDependentResource.setSecondary(null); + testDependentResource.setDesired(configMap()); + + assertThrows(DependentResourceException.class, + () -> testDependentResource.reconcile(new TestCustomResource(), null)); + } + + @Test + void throwsExceptionIfUpdateReturnsNull() { + TestDependentResource testDependentResource = new TestDependentResource(); + testDependentResource.setSecondary(configMap()); + testDependentResource.setDesired(configMap()); + + assertThrows(DependentResourceException.class, + () -> testDependentResource.reconcile(new TestCustomResource(), null)); + } + + private ConfigMap configMap() { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder() + .withName("test") + .withNamespace("default") + .build()); + return configMap; + } + + private static class TestDependentResource + extends AbstractDependentResource + implements Creator, Updater { + + private ConfigMap secondary; + private ConfigMap desired; + + @Override + public Class resourceType() { + return ConfigMap.class; + } + + @Override + public Optional getSecondaryResource(TestCustomResource primary) { + return Optional.ofNullable(secondary); + } + + @Override + protected void onCreated(ResourceID primaryResourceId, ConfigMap created) {} + + @Override + protected void onUpdated(ResourceID primaryResourceId, ConfigMap updated, ConfigMap actual) {} + + @Override + protected ConfigMap desired(TestCustomResource primary, Context context) { + return desired; + } + + public ConfigMap getSecondary() { + return secondary; + } + + public TestDependentResource setSecondary(ConfigMap secondary) { + this.secondary = secondary; + return this; + } + + public ConfigMap getDesired() { + return desired; + } + + public TestDependentResource setDesired(ConfigMap desired) { + this.desired = desired; + return this; + } + + @Override + public ConfigMap create(ConfigMap desired, TestCustomResource primary, + Context context) { + return null; + } + + @Override + public ConfigMap update(ConfigMap actual, ConfigMap desired, TestCustomResource primary, + Context context) { + return null; + } + + @Override + public Matcher.Result match(ConfigMap actualResource, TestCustomResource primary, + Context context) { + var result = mock(Matcher.Result.class); + when(result.matched()).thenReturn(false); + return result; + } + } +}