diff --git a/operator/src/main/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconciler.java b/operator/src/main/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconciler.java index 0542247..44551ca 100644 --- a/operator/src/main/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconciler.java +++ b/operator/src/main/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconciler.java @@ -43,6 +43,8 @@ import jakarta.inject.Inject; import org.jboss.logging.Logger; +import java.util.Map; + import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_ALL_NAMESPACES; /** @@ -95,7 +97,8 @@ protected UpdateControl doReconcile(ConfigurationPlan resourc } String backendSecretId = null; - String organization = resource.getMetadata().getAnnotations().get(ReshaprAnnotations.ORGANIZATION); + Map annotations = resource.getMetadata().getAnnotations(); + String organization = annotations != null ? annotations.get(ReshaprAnnotations.ORGANIZATION) : null; if (spec.getOauth2() != null) { OAuth2Spec oauth2 = spec.getOauth2(); @@ -151,21 +154,39 @@ protected UpdateControl doReconcile(ConfigurationPlan resourc remotePlan.setBackendSecretId(backendSecretId); } - io.reshapr.client.model.ConfigurationPlan createdPlan = api.createConfigPlan(remotePlan); - String newPlanId = createdPlan.getId(); - - if (spec.isApiKey()) { - api.renewConfigPlanApiKey(newPlanId); + String newPlanId; + if (alreadyImported) { + remotePlan.setId(currentPlanId); + try { + io.reshapr.client.model.ConfigurationPlan updated = api.updateConfigPlan(currentPlanId, remotePlan); + newPlanId = updated != null && updated.getId() != null ? updated.getId() : currentPlanId; + logger.infof("ConfigurationPlan '%s' updated in control plane with id=%s", name, newPlanId); + } catch (ApiException e) { + if (e.getCode() == 404) { + logger.warnf("Recorded control plane ConfigurationPlan id=%s for '%s' no longer exists (HTTP 404) — recreating", + currentPlanId, name); + io.reshapr.client.model.ConfigurationPlan created = api.createConfigPlan(remotePlan); + newPlanId = created != null ? created.getId() : null; + if (spec.isApiKey() && newPlanId != null) { + api.renewConfigPlanApiKey(newPlanId); + } + } else { + throw e; + } + } + } else { + io.reshapr.client.model.ConfigurationPlan createdPlan = api.createConfigPlan(remotePlan); + newPlanId = createdPlan != null ? createdPlan.getId() : null; + logger.infof("ConfigurationPlan '%s' created in control plane with id=%s", name, newPlanId); + if (spec.isApiKey() && newPlanId != null) { + api.renewConfigPlanApiKey(newPlanId); + } } - if (alreadyImported && !newPlanId.equals(currentPlanId)) { - try { - api.deleteConfigPlan(currentPlanId); - } catch (ApiException e) { - if (e.getCode() != 404) { - logger.warnf(e, "Failed to delete previous ConfigurationPlan id=%s", currentPlanId); - } - } + if (newPlanId == null) { + logger.warnf("ConfigurationPlan '%s': control plane did not return an id — rescheduling", name); + return recordStatus(resource, Status.IN_PROGRESS, + "Control plane did not return a ConfigurationPlan id").rescheduleAfter(RETRY_DELAY_MS); } status.setStatus(Status.READY); diff --git a/operator/src/test/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconcilerTest.java b/operator/src/test/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconcilerTest.java index 6990d90..82287d2 100644 --- a/operator/src/test/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconcilerTest.java +++ b/operator/src/test/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconcilerTest.java @@ -23,6 +23,7 @@ import io.reshapr.kubernetes.api.configurationplan.v1alpha1.CachePolicy; import io.reshapr.kubernetes.api.configurationplan.v1alpha1.ConfigurationPlan; import io.reshapr.kubernetes.api.configurationplan.v1alpha1.ConfigurationPlanSpec; +import io.reshapr.kubernetes.api.configurationplan.v1alpha1.ConfigurationPlanStatus; import io.reshapr.kubernetes.api.configurationplan.v1alpha1.HeaderPolicy; import io.reshapr.kubernetes.api.configurationplan.v1alpha1.HeaderRename; import io.reshapr.kubernetes.api.configurationplan.v1alpha1.HeaderRules; @@ -277,4 +278,87 @@ void doReconcile_withMinimalSpec_usesDefaultToolExposureMode() throws Exception assertTrue(sentPlan.getExcludedOperations() == null || sentPlan.getExcludedOperations().isEmpty()); } } + + @Test + void doReconcile_whenAlreadyImportedAndSpecChanged_updatesInPlaceInsteadOfRecreating() throws Exception { + ReshaprApiClientFactory factory = mock(ReshaprApiClientFactory.class); + ConfigurationPlanReconciler reconciler = new ConfigurationPlanReconciler(factory); + ApiClient apiClient = mock(ApiClient.class); + ConfigurationPlan cp = buildFullSpecConfigurationPlan(); + + ConfigurationPlanStatus status = new ConfigurationPlanStatus(); + status.setConfigurationPlanId(REMOTE_PLAN_ID); + status.setObservedGeneration(1L); + status.setStatus(io.reshapr.kubernetes.api.model.Status.READY); + cp.setStatus(status); + cp.getMetadata().setGeneration(2L); + + try (MockedConstruction mocked = Mockito.mockConstruction(DefaultApi.class, + (mock, ctx) -> { + when(mock.getServices(any(BigDecimal.class), any(BigDecimal.class))) + .thenReturn(List.of(remoteService())); + + Secret createdSecret = new Secret(); + createdSecret.setId(REMOTE_SECRET_ID); + when(mock.createSecret(any(Secret.class))).thenReturn(createdSecret); + + io.reshapr.client.model.ConfigurationPlan updated = new io.reshapr.client.model.ConfigurationPlan(); + updated.setId(REMOTE_PLAN_ID); + when(mock.updateConfigPlan(Mockito.eq(REMOTE_PLAN_ID), any(io.reshapr.client.model.ConfigurationPlan.class))) + .thenReturn(updated); + })) { + + reconciler.doReconcile(cp, null, apiClient); + + DefaultApi mockedApi = mocked.constructed().get(0); + verify(mockedApi).updateConfigPlan(Mockito.eq(REMOTE_PLAN_ID), any(io.reshapr.client.model.ConfigurationPlan.class)); + verify(mockedApi, never()).createConfigPlan(any()); + verify(mockedApi, never()).deleteConfigPlan(any()); + } + } + + @Test + void doReconcile_whenAlreadyImportedAndNotFound_recreatesConfigPlan() throws Exception { + ReshaprApiClientFactory factory = mock(ReshaprApiClientFactory.class); + ConfigurationPlanReconciler reconciler = new ConfigurationPlanReconciler(factory); + ApiClient apiClient = mock(ApiClient.class); + ConfigurationPlan cp = buildFullSpecConfigurationPlan(); + + ConfigurationPlanStatus status = new ConfigurationPlanStatus(); + status.setConfigurationPlanId("stale-plan-id"); + status.setObservedGeneration(1L); + status.setStatus(io.reshapr.kubernetes.api.model.Status.READY); + cp.setStatus(status); + cp.getMetadata().setGeneration(2L); + + try (MockedConstruction mocked = Mockito.mockConstruction(DefaultApi.class, + (mock, ctx) -> { + when(mock.getServices(any(BigDecimal.class), any(BigDecimal.class))) + .thenReturn(List.of(remoteService())); + + Secret createdSecret = new Secret(); + createdSecret.setId(REMOTE_SECRET_ID); + when(mock.createSecret(any(Secret.class))).thenReturn(createdSecret); + + // 404 on update + when(mock.updateConfigPlan(Mockito.eq("stale-plan-id"), any(io.reshapr.client.model.ConfigurationPlan.class))) + .thenThrow(new io.reshapr.client.ApiException(404, "Not Found")); + + io.reshapr.client.model.ConfigurationPlan recreated = new io.reshapr.client.model.ConfigurationPlan(); + recreated.setId(REMOTE_PLAN_ID); + when(mock.createConfigPlan(any(io.reshapr.client.model.ConfigurationPlan.class))) + .thenReturn(recreated); + })) { + + reconciler.doReconcile(cp, null, apiClient); + + DefaultApi mockedApi = mocked.constructed().get(0); + verify(mockedApi).updateConfigPlan(Mockito.eq("stale-plan-id"), any(io.reshapr.client.model.ConfigurationPlan.class)); + verify(mockedApi).createConfigPlan(any(io.reshapr.client.model.ConfigurationPlan.class)); + verify(mockedApi).renewConfigPlanApiKey(REMOTE_PLAN_ID); + assertEquals(REMOTE_PLAN_ID, cp.getStatus().getConfigurationPlanId()); + } + } } + +