Prevent removal of AccessSections that have more config
Access sections can have configs that Gerrit is not aware of. If this
is the case, the AccessSection should not be removed by Gerrit.
One example of a use case is when we roll out new permissions. We then
need to set them before we roll out the permission logic to all our
hosts. We do this by changing the project.config with some non-Gerrit
logic. This is a way of having a permission on a host before the logic
is there and guarantee that there is no interruption for users.
With the current implementation, Gerrit would remove these permissions.
Change-Id: Ie5dadfc64da02d2f4fe75fb750332ddc2890aaa9
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 0fc0712..975dc2b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -16,6 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.Permission;
@@ -33,6 +35,9 @@
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.group.SystemGroupBackend;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
@@ -355,6 +360,61 @@
.containsNoneIn(accessSectionInfo.permissions.keySet());
}
+ @Test
+ public void unknownPermissionRemainsUnchanged() throws Exception {
+ String access = "access";
+ String unknownPermission = "unknownPermission";
+ String registeredUsers = "group Registered Users";
+ String refsFor = "refs/for/*";
+ // Clone repository to forcefully add permission
+ TestRepository<InMemoryRepository> allProjectsRepo =
+ cloneProject(allProjects, admin);
+
+ // Fetch permission ref
+ GitUtil.fetch(allProjectsRepo, "refs/meta/config:cfg");
+ allProjectsRepo.reset("cfg");
+
+ // Load current permissions
+ String config = gApi.projects()
+ .name(allProjects.get())
+ .branch("refs/meta/config").file("project.config").asString();
+
+ // Append and push unknown permission
+ Config cfg = new Config();
+ cfg.fromText(config);
+ cfg.setString(access, refsFor, unknownPermission, registeredUsers);
+ config = cfg.toText();
+ PushOneCommit push = pushFactory.create(
+ db, admin.getIdent(), allProjectsRepo, "Subject", "project.config",
+ config);
+ push.to("refs/meta/config").assertOkStatus();
+
+ // Verify that unknownPermission is present
+ config = gApi.projects()
+ .name(allProjects.get())
+ .branch("refs/meta/config").file("project.config").asString();
+ cfg.fromText(config);
+ assertThat(cfg.getString(access, refsFor, unknownPermission))
+ .isEqualTo(registeredUsers);
+
+ // Make permission change through API
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+ accessInput.add.put(refsFor, accessSectionInfo);
+ gApi.projects().name(allProjects.get()).access(accessInput);
+ accessInput.add.clear();
+ accessInput.remove.put(refsFor, accessSectionInfo);
+ gApi.projects().name(allProjects.get()).access(accessInput);
+
+ // Verify that unknownPermission is still present
+ config = gApi.projects()
+ .name(allProjects.get())
+ .branch("refs/meta/config").file("project.config").asString();
+ cfg.fromText(config);
+ assertThat(cfg.getString(access, refsFor, unknownPermission))
+ .isEqualTo(registeredUsers);
+ }
+
private ProjectAccessInput newProjectAccessInput() {
ProjectAccessInput p = new ProjectAccessInput();
p.add = new HashMap<>();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index b94efce..689797b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -177,6 +177,7 @@
private long maxObjectSizeLimit;
private Map<String, Config> pluginConfigs;
private boolean checkReceivedObjects;
+ private Set<AccessSection> sectionsWithUnknownPermissions;
public static ProjectConfig read(MetaDataUpdate update) throws IOException,
ConfigInvalidException {
@@ -281,7 +282,12 @@
public void remove(AccessSection section) {
if (section != null) {
- accessSections.remove(section.getName());
+ AccessSection a = accessSections.get(section.getName());
+ if (sectionsWithUnknownPermissions.contains(a)) {
+ accessSections.remove(a);
+ } else {
+ a.setPermissions(new ArrayList<Permission>());
+ }
}
}
@@ -609,6 +615,7 @@
private void loadAccessSections(
Config rc, Map<String, GroupReference> groupsByName) {
accessSections = new HashMap<>();
+ sectionsWithUnknownPermissions = new HashSet<>();
for (String refName : rc.getSubsections(ACCESS)) {
if (RefConfigSection.isValid(refName) && isValidRegex(refName)) {
AccessSection as = getAccessSection(refName, true);
@@ -626,6 +633,8 @@
Permission perm = as.getPermission(varName, true);
loadPermissionRules(rc, ACCESS, refName, varName, groupsByName,
perm, Permission.hasRange(varName));
+ } else {
+ sectionsWithUnknownPermissions.add(as);
}
}
}
