Repository: knox Updated Branches: refs/heads/master d402f6d23 -> cf768a74d
http://git-wip-us.apache.org/repos/asf/knox/blob/cf768a74/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java index 011975a..52a7dfa 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java @@ -885,6 +885,18 @@ public class DefaultTopologyService } } catch (Exception e) { log.simpleDescriptorHandlingError(file.getName(), e); + + // If the referenced provider configuration is invalid, remove any existing reference relationships for the + // referencing descriptor. + if (e instanceof IllegalArgumentException) { + String descriptorName = FilenameUtils.normalize(file.getAbsolutePath()); + // Need to check if descriptor had previously referenced another provider config, so it can be removed + for (List<String> descs : providerConfigReferences.values()) { + if (descs.contains(descriptorName)) { + descs.remove(descriptorName); + } + } + } } } http://git-wip-us.apache.org/repos/asf/knox/blob/cf768a74/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java index d24a6cf..db15094 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java @@ -102,59 +102,64 @@ public class SimpleDescriptorHandler { public static Map<String, File> handle(SimpleDescriptor desc, File srcDirectory, File destDirectory, Service...gatewayServices) { - List<String> validServiceNames = new ArrayList<>(); - Map<String, String> serviceVersions = new HashMap<>(); - Map<String, Map<String, String>> serviceParams = new HashMap<>(); - Map<String, List<String>> serviceURLs = new HashMap<>(); - - // Discover the cluster details required by the descriptor - ServiceDiscovery.Cluster cluster = performDiscovery(desc, gatewayServices); - if (cluster != null) { - for (SimpleDescriptor.Service descService : desc.getServices()) { - String serviceName = descService.getName(); - - String serviceVer = descService.getVersion(); - if (serviceVer != null) { - serviceVersions.put(serviceName, serviceVer); - } + List<String> validServiceNames = new ArrayList<>(); + Map<String, String> serviceVersions = new HashMap<>(); + Map<String, Map<String, String>> serviceParams = new HashMap<>(); + Map<String, List<String>> serviceURLs = new HashMap<>(); + + ServiceDiscovery.Cluster cluster = null; + if (desc.getDiscoveryAddress() != null) { + // Discover the cluster details required by the descriptor + cluster = performDiscovery(desc, gatewayServices); + if (cluster == null) { + log.failedToDiscoverClusterServices(desc.getClusterName()); + } + } + + for (SimpleDescriptor.Service descService : desc.getServices()) { + String serviceName = descService.getName(); - List<String> descServiceURLs = descService.getURLs(); - if (descServiceURLs == null || descServiceURLs.isEmpty()) { + String serviceVer = descService.getVersion(); + if (serviceVer != null) { + serviceVersions.put(serviceName, serviceVer); + } + + List<String> descServiceURLs = descService.getURLs(); + if (descServiceURLs == null || descServiceURLs.isEmpty()) { + if (cluster != null) { descServiceURLs = cluster.getServiceURLs(serviceName, descService.getParams()); } + } - // Validate the discovered service URLs - List<String> validURLs = new ArrayList<>(); - if (descServiceURLs != null && !descServiceURLs.isEmpty()) { - // Validate the URL(s) - for (String descServiceURL : descServiceURLs) { - if (validateURL(serviceName, descServiceURL)) { - validURLs.add(descServiceURL); - } - } - - if (!validURLs.isEmpty()) { - validServiceNames.add(serviceName); + // Validate the discovered service URLs + List<String> validURLs = new ArrayList<>(); + if (descServiceURLs != null && !descServiceURLs.isEmpty()) { + // Validate the URL(s) + for (String descServiceURL : descServiceURLs) { + if (validateURL(serviceName, descServiceURL)) { + validURLs.add(descServiceURL); } } - // If there is at least one valid URL associated with the service, then add it to the map if (!validURLs.isEmpty()) { - serviceURLs.put(serviceName, validURLs); - } else { - log.failedToDiscoverClusterServiceURLs(serviceName, cluster.getName()); + validServiceNames.add(serviceName); } + } - // Service params - if (descService.getParams() != null) { - serviceParams.put(serviceName, descService.getParams()); - if (!validServiceNames.contains(serviceName)) { - validServiceNames.add(serviceName); - } + // If there is at least one valid URL associated with the service, then add it to the map + if (!validURLs.isEmpty()) { + serviceURLs.put(serviceName, validURLs); + } else { + log.failedToDiscoverClusterServiceURLs(serviceName, (cluster != null ? cluster.getName() : "")); + } + + // Service params + if (descService.getParams() != null) { + serviceParams.put(serviceName, descService.getParams()); + if (!validServiceNames.contains(serviceName)) { + validServiceNames.add(serviceName); } } - } else { - log.failedToDiscoverClusterServices(desc.getClusterName()); } // Provision the query param encryption password here, rather than relying on the random password generated @@ -194,10 +199,9 @@ public class SimpleDescriptorHandler { private static ProviderConfiguration handleProviderConfiguration(SimpleDescriptor desc, File providerConfig) { // Verify that the referenced provider configuration exists before attempting to read it - if (providerConfig == null) { + if (providerConfig == null || !providerConfig.exists()) { log.failedToResolveProviderConfigRef(desc.getProviderConfig()); - throw new IllegalArgumentException("Unresolved provider configuration reference: " + - desc.getProviderConfig() + " ; Topology update aborted!"); + throw new IllegalArgumentException("Unresolved provider configuration reference: " + desc.getProviderConfig()); } // Parse the contents of the referenced provider config @@ -345,7 +349,7 @@ public class SimpleDescriptorHandler { File providerConfigFile = resolveProviderConfigurationReference(desc.getProviderConfig(), srcDirectory); ProviderConfiguration providerConfiguration = handleProviderConfiguration(desc, providerConfigFile); if (providerConfiguration == null) { - throw new IllegalArgumentException("Invalid provider configuration."); + throw new IllegalArgumentException("Invalid provider configuration: " + desc.getProviderConfig()); } result.put(RESULT_REFERENCE, providerConfigFile); http://git-wip-us.apache.org/repos/asf/knox/blob/cf768a74/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java ---------------------------------------------------------------------- diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java index 0db0121..ae9371d 100644 --- a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java +++ b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java @@ -288,16 +288,22 @@ public class GatewayAdminTopologyFuncTest { private static String createDescriptor(String clusterName, String providerConfigRef) { + return createDescriptor(clusterName, providerConfigRef, true); + } + + private static String createDescriptor(String clusterName, String providerConfigRef, boolean discovery) { StringBuilder sb = new StringBuilder(); if (providerConfigRef == null) { providerConfigRef = "sandbox-providers"; } sb.append("{\n"); - sb.append(" \"discovery-type\":\"AMBARI\",\n"); - sb.append(" \"discovery-address\":\"http://c6401.ambari.apache.org:8080\",\n";); - sb.append(" \"discovery-user\":\"ambariuser\",\n"); - sb.append(" \"discovery-pwd-alias\":\"ambari.discovery.password\",\n"); + if (discovery) { + sb.append(" \"discovery-type\":\"AMBARI\",\n"); + sb.append(" \"discovery-address\":\"http://c6401.ambari.apache.org:8080\",\n";); + sb.append(" \"discovery-user\":\"ambariuser\",\n"); + sb.append(" \"discovery-pwd-alias\":\"ambari.discovery.password\",\n"); + } sb.append(" \"provider-config-ref\":\""); sb.append(providerConfigRef); sb.append("\",\n"); @@ -1095,9 +1101,9 @@ public class GatewayAdminTopologyFuncTest { .when().delete(href1).body(); String deletedMsg = responseBody.path("deleted"); assertEquals("provider config " + FilenameUtils.getBaseName(name1), deletedMsg); - assertFalse((new File(sharedProvidersDir, name1).exists())); + assertFalse((new File(sharedProvidersDir, name1)).exists()); - assertTrue((new File(sharedProvidersDir, name2).exists())); + assertTrue((new File(sharedProvidersDir, name2)).exists()); // Delete the other provider config responseBody = given() .auth().preemptive().basic(username, password) @@ -1108,7 +1114,7 @@ public class GatewayAdminTopologyFuncTest { .when().delete(href2).body(); deletedMsg = responseBody.path("deleted"); assertEquals("provider config " + FilenameUtils.getBaseName(name2), deletedMsg); - assertFalse((new File(sharedProvidersDir, name2).exists())); + assertFalse((new File(sharedProvidersDir, name2)).exists()); // Attempt to delete a provider config that does not exist given() @@ -1122,6 +1128,141 @@ public class GatewayAdminTopologyFuncTest { } + /** + * KNOX-1176 + */ + @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testDeleteReferencedProviderConfiguration() throws Exception { + LOG_ENTER(); + + final String username = "admin"; + final String password = "admin-password"; + final String serviceUrl = clusterUrl + "/api/v1/providerconfig"; + final String descriptorsUrl = clusterUrl + "/api/v1/descriptors"; + + final File sharedProvidersDir = new File(config.getGatewayConfDir(), "shared-providers"); + + // Manually add two provider config files to the shared-providers directory + File providerConfigOneFile = new File(sharedProvidersDir, "deleteme-one-config.xml"); + FileOutputStream stream = new FileOutputStream(providerConfigOneFile); + createProviderConfiguration().toStream(stream); + stream.close(); + assertTrue(providerConfigOneFile.exists()); + + File providerConfigTwoFile = new File(sharedProvidersDir, "deleteme-two-config.xml"); + stream = new FileOutputStream(providerConfigTwoFile); + createProviderConfiguration().toStream(stream); + stream.close(); + assertTrue(providerConfigTwoFile.exists()); + + // Request a listing of all the provider configs + ResponseBody responseBody = given() + .auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .then() + .statusCode(HttpStatus.SC_OK) + .contentType(MediaType.APPLICATION_JSON) + .when().get(serviceUrl).body(); + List<String> items = responseBody.path("items"); + assertEquals(2, items.size()); + String name1 = responseBody.path("items[0].name"); + String href1 = responseBody.path("items[0].href"); + String name2 = responseBody.path("items[1].name"); + String href2 = responseBody.path("items[1].href"); + + ///////////////////////////////////////////////////////////// + // PUT a descriptor, which references one of the provider configurations, which should make that provider + // configuration ineligible for deletion. + String descriptorName = "mycluster"; + String newDescriptorJSON = createDescriptor(descriptorName, name1, false); + given() + .auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(newDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_CREATED) + .when().put(descriptorsUrl + "/" + descriptorName); + + try { // Wait for the reference relationship to be established + Thread.sleep(10000); + } catch (InterruptedException e) { + // + } + + // Attempt to delete the referenced provider configs, and verify that it is NOT deleted + responseBody = given() + .auth().preemptive().basic(username, password) + .then() + .statusCode(HttpStatus.SC_NOT_MODIFIED) + .when().delete(href1).body(); + assertTrue("Provider config deletion should have been prevented.", + (new File(sharedProvidersDir, name1)).exists()); + + ///////////////////////////////////////////////////////////// + // Update the descriptor to reference the other provider config, such that the first one should become eligible for + // deletion. + String updatedDescriptorJSON = createDescriptor(descriptorName, name2, false); + given() + .auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(updatedDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_NO_CONTENT) + .when().put(descriptorsUrl + "/" + descriptorName); + + try { // Wait for the reference relationship to be established + Thread.sleep(10000); + } catch (InterruptedException e) { + // + } + + // Delete the originally-referenced provider config, and verify that it has been deleted + responseBody = given() + .auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .then() + .statusCode(HttpStatus.SC_OK) + .contentType(MediaType.APPLICATION_JSON) + .when().delete(href1).body(); + String deletedMsg = responseBody.path("deleted"); + assertEquals("provider config " + FilenameUtils.getBaseName(name1), deletedMsg); + assertFalse((new File(sharedProvidersDir, name1)).exists()); + + ///////////////////////////////////////////////////////////// + // (KNOX-1176) Update the descriptor to reference an invalid provider config, which should remove the prior + // provider configuration reference relationship (even though the new reference could not be resolved), and make + // that previously-referenced provider configuration eligible for deletion. + updatedDescriptorJSON = createDescriptor(descriptorName, "invalid-provider-config", false); + given() + .auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(updatedDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_NO_CONTENT) + .when().put(descriptorsUrl + "/" + descriptorName); + + try { // Wait for the reference relationship to be established + Thread.sleep(10000); + } catch (InterruptedException e) { + // + } + + // Delete the previously-referenced provider config, and verify that it has been deleted + responseBody = given() + .auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .then() + .statusCode(HttpStatus.SC_OK) + .contentType(MediaType.APPLICATION_JSON) + .when().delete(href2).body(); + deletedMsg = responseBody.path("deleted"); + assertEquals("provider config " + FilenameUtils.getBaseName(name2), deletedMsg); + assertFalse((new File(sharedProvidersDir, name2)).exists()); + + LOG_EXIT(); + } + + @Test( timeout = TestUtils.LONG_TIMEOUT ) public void testDescriptorCollection() throws Exception { LOG_ENTER(); @@ -1337,9 +1478,9 @@ public class GatewayAdminTopologyFuncTest { .when().delete(href1).body(); String deletedMsg = responseBody.path("deleted"); assertEquals("descriptor " + FilenameUtils.getBaseName(name1), deletedMsg); - assertFalse((new File(descriptorsDir, name1).exists())); + assertFalse((new File(descriptorsDir, name1)).exists()); - assertTrue((new File(descriptorsDir, name2).exists())); + assertTrue((new File(descriptorsDir, name2)).exists()); // Delete the other descriptor responseBody = given() .auth().preemptive().basic(username, password)
