Browse Source

AMBARI-5042 Ambari Repo URL validator rejecting valid yum repo file:/// URL. (jaoki)

Jun Aoki 10 years ago
parent
commit
6213033697

+ 28 - 13
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java

@@ -3074,26 +3074,41 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
     String[] suffixes = configs.getRepoValidationSuffixes(request.getOsType());
     String[] suffixes = configs.getRepoValidationSuffixes(request.getOsType());
     for (String suffix : suffixes) {
     for (String suffix : suffixes) {
       String formatted_suffix = String.format(suffix, repoName);
       String formatted_suffix = String.format(suffix, repoName);
-      String spec = request.getBaseUrl();
+      String spec = request.getBaseUrl().trim();
 
 
+      // This logic is to identify if the end of baseurl has a slash ('/') and/or the beginning of suffix String (e.g. "/repodata/repomd.xml") 
+      // has a slash and they can form a good url.
+      // e.g. "http://baseurl.com/" + "/repodata/repomd.xml" becomes "http://baseurl.com/repodata/repomd.xml" but not "http://baseurl.com//repodata/repomd.xml"
       if (spec.charAt(spec.length() - 1) != '/' && formatted_suffix.charAt(0) != '/') {
       if (spec.charAt(spec.length() - 1) != '/' && formatted_suffix.charAt(0) != '/') {
-        spec = request.getBaseUrl() + "/" + formatted_suffix;
+        spec = spec + "/" + formatted_suffix;
       } else if (spec.charAt(spec.length() - 1) == '/' && formatted_suffix.charAt(0) == '/') {
       } else if (spec.charAt(spec.length() - 1) == '/' && formatted_suffix.charAt(0) == '/') {
-        spec = request.getBaseUrl() + formatted_suffix.substring(1);
+        spec = spec + formatted_suffix.substring(1);
       } else {
       } else {
-        spec = request.getBaseUrl() + formatted_suffix;
+        spec = spec + formatted_suffix;
       }
       }
 
 
-      try {
-        IOUtils.readLines(usp.readFrom(spec));
-      } catch (IOException ioe) {
-        errorMessage = "Could not access base url . " + request.getBaseUrl() + " . ";
-        if (LOG.isDebugEnabled()) {
-          errorMessage += ioe;
-        } else {
-          errorMessage += ioe.getMessage();
+      // if spec contains "file://" then check local file system.
+      final String FILE_SCHEME = "file://";
+      if(spec.toLowerCase().startsWith(FILE_SCHEME)){
+        String filePath = spec.substring(FILE_SCHEME.length());
+        File f = new File(filePath);
+        if(!f.exists()){
+          errorMessage = "Could not access base url . " + spec + " . ";
+          break;
+        }
+
+      }else{
+        try {
+          IOUtils.readLines(usp.readFrom(spec));
+        } catch (IOException ioe) {
+          errorMessage = "Could not access base url . " + request.getBaseUrl() + " . ";
+          if (LOG.isDebugEnabled()) {
+            errorMessage += ioe;
+          } else {
+            errorMessage += ioe.getMessage();
+          }
+          break;
         }
         }
-        break;
       }
       }
     }
     }
 
 

+ 53 - 0
ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java

@@ -48,6 +48,7 @@ import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.ComponentInfo;
 import org.apache.ambari.server.state.ComponentInfo;
 import org.apache.ambari.server.state.Host;
 import org.apache.ambari.server.state.Host;
 import org.apache.ambari.server.state.MaintenanceState;
 import org.apache.ambari.server.state.MaintenanceState;
+import org.apache.ambari.server.state.RepositoryInfo;
 import org.apache.ambari.server.state.SecurityType;
 import org.apache.ambari.server.state.SecurityType;
 import org.apache.ambari.server.state.Service;
 import org.apache.ambari.server.state.Service;
 import org.apache.ambari.server.state.ServiceComponent;
 import org.apache.ambari.server.state.ServiceComponent;
@@ -1817,4 +1818,56 @@ public class AmbariManagementControllerImplTest {
     }
     }
 
 
   }
   }
+
+  @Test
+  public void testVerifyRepositires() throws Exception {
+    // member state mocks
+    Injector injector = createStrictMock(Injector.class);
+    Capture<AmbariManagementController> controllerCapture = new Capture<AmbariManagementController>();
+
+    // expectations
+    // constructor init
+    injector.injectMembers(capture(controllerCapture));
+    expect(injector.getInstance(Gson.class)).andReturn(null);
+    expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(null);
+    expect(injector.getInstance(KerberosHelper.class)).andReturn(createNiceMock(KerberosHelper.class));
+
+    RepositoryInfo dummyRepoInfo = new RepositoryInfo();
+    dummyRepoInfo.setRepoName("repo_name");
+
+    expect(ambariMetaInfo.getRepository("stackName", "stackVersion", "redhat6", "repoId")).andReturn(dummyRepoInfo);
+
+    Configuration configuration = createNiceMock(Configuration.class);
+    String[] suffices = {"/repodata/repomd.xml"};
+    expect(configuration.getRepoValidationSuffixes("redhat6")).andReturn(suffices);
+
+    // replay mocks
+    replay(injector, clusters, ambariMetaInfo, configuration);
+
+    // test
+    AmbariManagementController controller = new AmbariManagementControllerImpl(null, clusters, injector);
+    setAmbariMetaInfo(ambariMetaInfo, controller);
+
+    // Manually injected
+    Class<?> c = controller.getClass();
+    Field f = c.getDeclaredField("configs");
+    f.setAccessible(true);
+    f.set(controller, configuration);
+
+    Set<RepositoryRequest> requests = new HashSet<RepositoryRequest>();
+    RepositoryRequest request = new RepositoryRequest("stackName", "stackVersion", "redhat6", "repoId");
+    request.setBaseUrl("file:///some/repo");
+	requests.add(request);
+
+	// A wrong file path is passed and IllegalArgumentException is expected
+	try{
+		controller.verifyRepositories(requests);
+		Assert.fail("IllegalArgumentException is expected");
+	}catch(IllegalArgumentException e){
+		Assert.assertEquals("Could not access base url . file:///some/repo/repodata/repomd.xml . ", e.getMessage());
+	}
+
+    verify(injector, clusters, ambariMetaInfo, configuration);
+  }
+
 }
 }