Merge "Expand submodule subscription support"

Documentation/user-submodules.txt

@@ -1,5 +1,5 @@
-Gerrit Code Review - Superprojects subscribed to submodules updates
-===================================================================
+Gerrit Code Review - Superproject subscription to submodules updates
+====================================================================
 
 Description
 -----------
@@ -25,7 +25,7 @@ commit having the updated gitlinks.
 Git Submodules Overview
 -----------------------
 
-It is a git feature that allows an external repository to be
+Submodules are a git feature that allows an external repository to be
 attached inside a repository at a specific path. The objective here
 is to provide a brief overview, further details can be found
 in the official git submodule command documentation.
@@ -37,7 +37,7 @@ at path 'a' by executing the following command when being inside
 'super':
 =====
 git submodule add ssh://server/a a
-====
+=====
 
 Still considering the above example, after its execution notice that
 inside the local repository 'super' the 'a' folder is considered a
@@ -63,11 +63,11 @@ Creating a New Subscription
 Defining the Submodule Branch
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-This is required because Submodule subscription is actually the
+This is required because submodule subscription is actually the
 subscription of a submodule project and one of its branches for
 a branch of a super project.
 
-Since it manages subscriptions in the branch scope, we could have
+Since Gerrit manages subscriptions in the branch scope, we could have
 a scenario having a project called 'super' having a branch 'integration'
 subscribed to a project called 'a' in branch 'integration', and also
 having the same 'super' project but in branch 'dev' subscribed to the 'a'
@@ -77,22 +77,23 @@ After adding the git submodule to a super project, one should edit
 the .gitmodules file to add a branch field to each submodule
 section which is supposed to be subscribed.
 
-The branch field is not filled by the git submodule command. Its value
-should indicate the branch of a submodule project that when updated
-will trigger automatic update of its registered gitlink.
+As the branch field is a Gerrit specific field it will not be filled
+automatically by the git submodule command, so one needs to edit it
+manually. Its value should indicate the branch of a submodule project
+that when updated will trigger automatic update of its registered
+gitlink.
 
 The branch value could be '.' if the submodule project branch
 has the same name as the destination branch of the commit having
 gitlinks/.gitmodules file.
 
-The branch field of a submodule section is a custom git submodule
-feature for Gerrit use. One should always be sure to fill it in
-editing .gitmodules file after adding submodules to a super project,
-if it is the intention to make use of the Gerrit feature introduced here.
+If the intention is to make use of the Gerrit feature described
+here, one should always be sure to update the .gitmodules file after
+adding submodules to a super project.
 
-Any git submodules which are added and not have the branch field
-available in the .gitmodules file will not be subscribed by Gerrit
-to automatically update the superproject.
+If a git submodule is added but the branch field is not added to the
+.gitmodules file Gerrit will not create a subscribtion for the
+submodule and there will be no automatic updates to the superproject.
 
 Detecting and Subscribing Submodules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -118,17 +119,22 @@ is merged in branch 'dev-of-a' of 'a' project, Gerrit automatically
 creates a new commit on branch 'dev' of 'super' updating the gitlink
 to point to the just merged commit.
 
-Canonical Web Url
-~~~~~~~~~~~~~~~~~
+Limitations in subscription
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Gerrit will automatically update only the superprojects that added
-the submodules of urls of the running server (the one described in
-the canonical web url value in Gerrit configuration file).
+Gerrit will only automatically update superprojects where the
+submodules are hosted on the same Gerrit instance as the superproject.
+
+Gerrit determines this by checking the hostname of the submodule
+specified in the .gitmodules file and comparing it with the Gerrit
+instance's hostname. By default the hostname from the canonical web
+url is used, but if SSH is turned on and the SSH server name is
+different from the canonical hostname it will be used instead.
 
 The Gerrit instance administrator group should always certify to
 provide the canonical web url value in its configuration file. Users
-should certify to use the url value of the running Gerrit instance to
-add/subscribe submodules.
+should certify to use the correct hostname of the running Gerrit
+instance to add/subscribe submodules.
 
 Removing Subscriptions
 ----------------------

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java

@@ -42,6 +42,8 @@ import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.AuthConfigModule;
 import com.google.gerrit.server.config.CanonicalWebUrlModule;
 import com.google.gerrit.server.config.CanonicalWebUrlProvider;
+import com.google.gerrit.server.config.SshdListenAddressModule;
+import com.google.gerrit.server.config.SshdListenAddressProvider;
 import com.google.gerrit.server.config.GerritGlobalModule;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.MasterNodeStartup;
@@ -295,6 +297,16 @@ public class Daemon extends SiteProgram {
     modules.add(new SmtpEmailSender.Module());
     modules.add(new SignedTokenEmailTokenVerifier.Module());
     modules.add(new PluginModule());
+
+    if (sshd) {
+      modules.add(new SshdListenAddressModule() {
+        @Override
+        protected Class<? extends Provider<String>> provider() {
+          return SshdListenAddressProvider.class;
+        }
+      });
+    }
+
     if (httpd) {
       modules.add(new CanonicalWebUrlModule() {
         @Override

gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddress.java

@@ -0,0 +1,29 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.config;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+
+import java.lang.annotation.Retention;
+
+/**
+ * Marker on a {@link String} holding the SSH listen address for this server.
+ */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface SshdListenAddress {
+}

gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressModule.java

@@ -0,0 +1,30 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.config;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Provider;
+
+/** Supports binding the {@link SshdListenAddress} annotation. */
+public abstract class SshdListenAddressModule extends AbstractModule {
+  @Override
+  protected void configure() {
+    final Class<? extends Provider<String>> provider = provider();
+    bind(String.class).annotatedWith(SshdListenAddress.class)
+        .toProvider(provider);
+  }
+
+  protected abstract Class<? extends Provider<String>> provider();
+}

gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressProvider.java

@@ -0,0 +1,46 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.config;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.eclipse.jgit.lib.Config;
+
+/** Provides {@link SshdListenAddressl} from {@code sshd.listenAddress}. */
+public class SshdListenAddressProvider implements Provider<String> {
+  private final String sshAddress;
+
+  @Inject
+  public SshdListenAddressProvider(@GerritServerConfig final Config config) {
+    String sshdListenAddress = config.getString("sshd", null, "listenAddress");
+    String sshdAdvertisedAddress = config.getString("sshd", null, "advertisedAddress");
+
+    /*
+     * If advertised address is specified it should take precedence over the
+     * "normal" listening address.
+     */
+    if (sshdAdvertisedAddress != null && ! sshdAdvertisedAddress.isEmpty()) {
+      sshAddress = sshdAdvertisedAddress;
+    } else {
+      sshAddress = sshdListenAddress;
+    }
+  }
+
+  @Override
+  public String get() {
+    return sshAddress;
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.SshdListenAddress;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.util.SubmoduleSectionParser;
 import com.google.gwtorm.server.OrmException;
@@ -78,6 +79,7 @@ public class SubmoduleOp {
   private RevCommit mergeTip;
   private RevWalk rw;
   private final Provider<String> urlProvider;
+  private final Provider<String> sshProvider;
   private ReviewDb schema;
   private Repository db;
   private Project destProject;
@@ -93,6 +95,7 @@ public class SubmoduleOp {
   public SubmoduleOp(@Assisted final Branch.NameKey destBranch,
       @Assisted RevCommit mergeTip, @Assisted RevWalk rw,
       @CanonicalWebUrl @Nullable final Provider<String> urlProvider,
+      @SshdListenAddress @Nullable final Provider<String> sshProvider,
       final SchemaFactory<ReviewDb> sf, @Assisted Repository db,
       @Assisted Project destProject, @Assisted List<Change> submitted,
       @Assisted final Map<Change.Id, CodeReviewCommit> commits,
@@ -102,6 +105,7 @@ public class SubmoduleOp {
     this.mergeTip = mergeTip;
     this.rw = rw;
     this.urlProvider = urlProvider;
+    this.sshProvider = sshProvider;
     this.schemaFactory = sf;
     this.db = db;
     this.destProject = destProject;
@@ -137,6 +141,32 @@ public class SubmoduleOp {
     }
 
     try {
+      String thisServer = null;
+      String sshHostname = null;
+      String webHostname = new URI(urlProvider.get()).getHost();
+
+      // If SSH is disabled on the command line there will be no
+      // sshProvider to ask. Translate this to the gerrit.config
+      // option "off" inside the [sshd] section.
+      if (sshProvider == null) {
+        sshHostname = "off";
+      } else {
+        // Make sure we only remove the port part and not a substring
+        // of an IPv6 address.
+        sshHostname = sshProvider.get().split(":\\d+$")[0];
+      }
+      // Use the web host name (canonical web url) for the following
+      // conditions:
+      // - If the ssh hostname is the same as the web host
+      // - If we listen on all addresses
+      // - If sshd is turned off
+      if (webHostname.equals(sshHostname) || sshHostname.equals("*") ||
+          sshHostname.equals("off")) {
+        thisServer = webHostname;
+      } else {
+        thisServer = sshHostname;
+      }
+
       final TreeWalk tw = TreeWalk.forPath(db, GIT_MODULES, mergeTip.getTree());
       if (tw != null
           && (FileMode.REGULAR_FILE.equals(tw.getRawMode(0)) || FileMode.EXECUTABLE_FILE
@@ -145,8 +175,6 @@ public class SubmoduleOp {
         BlobBasedConfig bbc =
             new BlobBasedConfig(null, db, mergeTip, GIT_MODULES);
 
-        final String thisServer = new URI(urlProvider.get()).getHost();
-
         final Branch.NameKey target =
             new Branch.NameKey(new Project.NameKey(destProject.getName()),
                 destBranch.get());

gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java

@@ -71,6 +71,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
   private SubmoduleSubscriptionAccess subscriptions;
   private ReviewDb schema;
   private Provider<String> urlProvider;
+  private Provider<String> sshProvider;
   private GitRepositoryManager repoManager;
   private GitReferenceUpdated replication;
 
@@ -84,18 +85,19 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
     schema = createStrictMock(ReviewDb.class);
     subscriptions = createStrictMock(SubmoduleSubscriptionAccess.class);
     urlProvider = createStrictMock(Provider.class);
+    sshProvider = createStrictMock(Provider.class);
     repoManager = createStrictMock(GitRepositoryManager.class);
     replication = createStrictMock(GitReferenceUpdated.class);
   }
 
   private void doReplay() {
-    replay(schemaFactory, schema, subscriptions, urlProvider, repoManager,
-        replication);
+    replay(schemaFactory, schema, subscriptions, urlProvider, sshProvider,
+        repoManager, replication);
   }
 
   private void doVerify() {
-    verify(schemaFactory, schema, subscriptions, urlProvider, repoManager,
-        replication);
+    verify(schemaFactory, schema, subscriptions, urlProvider, sshProvider,
+        repoManager, replication);
   }
 
   /**
@@ -117,7 +119,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
     final Branch.NameKey branchNameKey =
         new Branch.NameKey(new Project.NameKey("test-project"), "test-branch");
 
-    expect(urlProvider.get()).andReturn("http://localhost:8080");
+    expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
+    expect(sshProvider.get()).andReturn("*:29418");
 
     expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
     final ResultSet<SubmoduleSubscription> emptySubscriptions =
@@ -130,9 +133,9 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
     doReplay();
 
     final SubmoduleOp submoduleOp =
-        new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider,
-            schemaFactory, realDb, null, new ArrayList<Change>(), null, null,
-            null, null);
+        new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb),
+            urlProvider, sshProvider, schemaFactory, realDb, null,
+            new ArrayList<Change>(), null, null, null, null);
 
     submoduleOp.update();
 
@@ -626,7 +629,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
         new Branch.NameKey(new Project.NameKey("target-project"),
             sourceBranchNameKey.get());
 
-    expect(urlProvider.get()).andReturn("http://localhost:8080");
+    expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
+    expect(sshProvider.get()).andReturn("*:29418");
 
     expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
     final ResultSet<SubmoduleSubscription> subscribers =
@@ -657,10 +661,9 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
 
     final SubmoduleOp submoduleOp =
         new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
-            sourceRepository), urlProvider, schemaFactory, sourceRepository,
-            new Project(sourceBranchNameKey.getParentKey()), submitted,
-            mergedCommits, myIdent, repoManager, replication);
-
+            sourceRepository), urlProvider, sshProvider, schemaFactory,
+            sourceRepository, new Project(sourceBranchNameKey.getParentKey()),
+            submitted, mergedCommits, myIdent, repoManager, replication);
     submoduleOp.update();
 
     doVerify();
@@ -727,7 +730,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
         new Branch.NameKey(new Project.NameKey("target-project"),
             sourceBranchNameKey.get());
 
-    expect(urlProvider.get()).andReturn("http://localhost:8080");
+    expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
+    expect(sshProvider.get()).andReturn("*:29418");
 
     expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
     final ResultSet<SubmoduleSubscription> subscribers =
@@ -760,10 +764,9 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
 
     final SubmoduleOp submoduleOp =
         new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
-            sourceRepository), urlProvider, schemaFactory, sourceRepository,
-            new Project(sourceBranchNameKey.getParentKey()), submitted,
-            mergedCommits, myIdent, repoManager, replication);
-
+            sourceRepository), urlProvider, sshProvider, schemaFactory,
+            sourceRepository, new Project(sourceBranchNameKey.getParentKey()),
+            submitted, mergedCommits, myIdent, repoManager, replication);
     submoduleOp.update();
 
     doVerify();
@@ -864,7 +867,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
 
     final RevCommit mergeTip = git.commit().setMessage("test").call();
 
-    expect(urlProvider.get()).andReturn("http://localhost:8080").times(2);
+    expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
+    expect(sshProvider.get()).andReturn("*:29418");
 
     expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
     expect(subscriptions.bySuperProject(mergedBranch)).andReturn(
@@ -915,9 +919,9 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
 
     final SubmoduleOp submoduleOp =
         new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb),
-            urlProvider, schemaFactory, realDb, new Project(mergedBranch
-                .getParentKey()), new ArrayList<Change>(), null, null,
-            repoManager, null);
+            urlProvider, sshProvider, schemaFactory, realDb, new Project(
+                mergedBranch.getParentKey()), new ArrayList<Change>(), null,
+            null, repoManager, null);
 
     submoduleOp.update();
   }

gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java

@@ -27,6 +27,8 @@ import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.AuthConfigModule;
 import com.google.gerrit.server.config.CanonicalWebUrlModule;
+import com.google.gerrit.server.config.SshdListenAddressModule;
+import com.google.gerrit.server.config.SshdListenAddressProvider;
 import com.google.gerrit.server.config.GerritGlobalModule;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerConfigModule;
@@ -238,6 +240,12 @@ public class WebAppInitializer extends GuiceServletContextListener {
         return HttpCanonicalWebUrlProvider.class;
       }
     });
+    modules.add(new SshdListenAddressModule() {
+      @Override
+      protected Class<? extends Provider<String>> provider() {
+        return SshdListenAddressProvider.class;
+      }
+    });
     modules.add(new MasterNodeStartup());
     return cfgInjector.createChildInjector(modules);
   }