You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2018/07/16 08:46:46 UTC
[sling-whiteboard] branch master updated: Make whitelister support
bundles that are present in multiple features.
This is an automated email from the ASF dual-hosted git repository.
davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
The following commit(s) were added to refs/heads/master by this push:
new d0ef2db Make whitelister support bundles that are present in multiple features.
d0ef2db is described below
commit d0ef2db0255f948ab747a70736761d38146b9751
Author: David Bosschaert <da...@gmail.com>
AuthorDate: Mon Jul 16 09:46:19 2018 +0100
Make whitelister support bundles that are present in multiple features.
---
featuremodel/feature-whitelist/pom.xml | 5 -
.../feature/whitelist/impl/ResolverHookImpl.java | 104 +++++++++++++--------
.../feature/whitelist/impl/WhitelistEnforcer.java | 51 +---------
.../impl/WhitelistServiceFactoryImpl.java | 6 +-
.../sling/feature/whitelist/package-info.java | 23 +++++
.../whitelist/impl/ResolverHookImplTest.java | 49 ++++++++--
.../impl/WhitelistServiceFactoryImplTest.java | 83 ++++++++++++++++
7 files changed, 217 insertions(+), 104 deletions(-)
diff --git a/featuremodel/feature-whitelist/pom.xml b/featuremodel/feature-whitelist/pom.xml
index 6402ca9..8690576 100644
--- a/featuremodel/feature-whitelist/pom.xml
+++ b/featuremodel/feature-whitelist/pom.xml
@@ -78,11 +78,6 @@
<artifactId>osgi.core</artifactId>
<scope>provided</scope>
</dependency>
- <dependency>
- <groupId>org.osgi</groupId>
- <artifactId>osgi.cmpn</artifactId>
- <scope>provided</scope>
- </dependency>
<!-- Testing -->
<dependency>
diff --git a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/ResolverHookImpl.java b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/ResolverHookImpl.java
index 9f57a43..d6181f6 100644
--- a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/ResolverHookImpl.java
+++ b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/ResolverHookImpl.java
@@ -31,8 +31,9 @@ import org.osgi.util.tracker.ServiceTracker;
import java.util.Collection;
import java.util.Collections;
-import java.util.Iterator;
+import java.util.HashSet;
import java.util.Set;
+import java.util.logging.Level;
class ResolverHookImpl implements ResolverHook {
private static final long SERVICE_WAIT_TIMEOUT = 60000;
@@ -73,68 +74,93 @@ class ResolverHookImpl implements ResolverHook {
// The Feature Service could not be found, skip candidate pruning
if (fs == null) {
- // WhitelistEnforcer.LOG.warn("Could not obtain the feature service, no whitelist enforcement");
+ WhitelistEnforcer.LOG.warning("Could not obtain the feature service, no whitelist enforcement");
return;
}
- String reqFeat = fs.getFeatureForBundle(reqBundleName, reqBundleVersion);
- Set<String> regions = whitelistService.listRegions(reqFeat);
- if (regions == null)
+ Set<String> reqFeatures = fs.getFeaturesForBundle(reqBundleName, reqBundleVersion);
+ Set<String> regions;
+ if (reqFeatures == null) {
regions = Collections.emptySet();
+ reqFeatures = Collections.emptySet();
+ } else {
+ regions = new HashSet<>();
+ for (String feature : reqFeatures) {
+ regions.addAll(whitelistService.listRegions(feature));
+ }
+ }
- nextCapability:
- for (Iterator<BundleCapability> it = candidates.iterator(); it.hasNext(); ) {
- BundleCapability bc = it.next();
+ Set<BundleCapability> coveredCaps = new HashSet<>();
+ nextCapability:
+ for (BundleCapability bc : candidates) {
BundleRevision rev = bc.getRevision();
- // A bundle is allowed to wire to itself
Bundle capBundle = rev.getBundle();
long capBundleID = capBundle.getBundleId();
- if (capBundleID == 0)
- continue nextCapability; // always allow capability from the system bundle
+ if (capBundleID == 0) {
+ // always allow capability from the system bundle
+ coveredCaps.add(bc);
+ continue nextCapability;
+ }
- if (capBundleID == reqBundleID)
- continue nextCapability; // always allow capability from same bundle
+ if (capBundleID == reqBundleID) {
+ // always allow capability from same bundle
+ coveredCaps.add(bc);
+ continue nextCapability;
+ }
String capBundleName = capBundle.getSymbolicName();
Version capBundleVersion = capBundle.getVersion();
- String capFeat = fs.getFeatureForBundle(capBundleName, capBundleVersion);
- if (capFeat == null)
- continue nextCapability; // always allow capability not coming from a feature
-
- // Within a single feature everything can wire to everything else
- if (reqFeat != null && reqFeat.equals(capFeat))
- continue nextCapability;
+ Set<String> capFeatures = fs.getFeaturesForBundle(capBundleName, capBundleVersion);
+ if (capFeatures == null || capFeatures.isEmpty())
+ capFeatures = Collections.singleton(null);
- // If the feature hosting the capability has no regions defined, everyone can access
- if (whitelistService.listRegions(capFeat) == null)
- continue nextCapability;
+ for (String capFeat : capFeatures) {
+ if (capFeat == null) {
+ // always allow capability not coming from a feature
+ coveredCaps.add(bc);
+ continue nextCapability;
+ }
- Object pkg = bc.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
- if (pkg instanceof String) {
- String packageName = (String) pkg;
- // If the export is in the global region.
- if (Boolean.TRUE.equals(whitelistService.regionWhitelistsPackage(WhitelistService.GLOBAL_REGION, packageName)))
+ if (reqFeatures.contains(capFeat)) {
+ // Within a single feature everything can wire to everything else
+ coveredCaps.add(bc);
continue nextCapability;
+ }
- // If the export is in a region that the feature is also in, then allow
- for (String region : regions) {
- // We've done this one already
- if (WhitelistService.GLOBAL_REGION.equals(region))
- continue;
+ if (whitelistService.listRegions(capFeat) == null) {
+ // If the feature hosting the capability has no regions defined, everyone can access
+ coveredCaps.add(bc);
+ continue nextCapability;
+ }
- if (!Boolean.FALSE.equals(whitelistService.regionWhitelistsPackage(region, packageName)))
+ Object pkg = bc.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
+ if (pkg instanceof String) {
+ String packageName = (String) pkg;
+ if (Boolean.TRUE.equals(whitelistService.regionWhitelistsPackage(WhitelistService.GLOBAL_REGION, packageName))) {
+ // If the export is in the global region everyone can access
+ coveredCaps.add(bc);
continue nextCapability;
+ }
+
+ for (String region : regions) {
+ if (!Boolean.FALSE.equals(whitelistService.regionWhitelistsPackage(region, packageName))) {
+ // If the export is in a region that the feature is also in, then allow
+ coveredCaps.add(bc);
+ continue nextCapability;
+ }
+ }
}
-
- // The capability package is not visible by the requirer
- // remove from the candidates.
- it.remove();
- System.out.println("***** Removed: " + bc);
}
}
+
+ // Remove any capabilities that are not covered
+ if (candidates.retainAll(coveredCaps)) {
+ WhitelistEnforcer.LOG.log(Level.INFO,
+ "Removed one ore more candidates for requirement {0} as they are not in the correct region", requirement);
+ }
} catch (InterruptedException e) {
// ignore
}
diff --git a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistEnforcer.java b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistEnforcer.java
index f6fa331..e12038d 100644
--- a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistEnforcer.java
+++ b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistEnforcer.java
@@ -26,9 +26,10 @@ import org.osgi.framework.wiring.BundleRevision;
import org.osgi.util.tracker.ServiceTracker;
import java.util.Collection;
+import java.util.logging.Logger;
class WhitelistEnforcer implements ResolverHookFactory {
- // static final Logger LOG = LoggerFactory.getLogger(WhitelistEnforcer.class);
+ static final Logger LOG = Logger.getLogger(WhitelistEnforcer.class.getName());
final ServiceTracker<Features, Features> featureServiceTracker;
final WhitelistService whitelistService;
@@ -42,52 +43,4 @@ class WhitelistEnforcer implements ResolverHookFactory {
public ResolverHook begin(Collection<BundleRevision> triggers) {
return new ResolverHookImpl(featureServiceTracker, whitelistService);
}
-
- /*
- @Override
- public synchronized void updated(Dictionary<String, ?> properties) throws ConfigurationException {
- if (wlsRegistration != null) {
- wlsRegistration.unregister();
- wlsRegistration = null;
- }
-
- if (properties == null) {
- whitelistService = null;
- return;
- }
-
- Map<String, Set<String>> frm = new HashMap<>();
- Map<String, Set<String>> rpm = new HashMap<>();
-
- for (Enumeration<String> e = properties.keys(); e.hasMoreElements(); ) {
- String key = e.nextElement().trim();
-
- if (key.startsWith(CONFIG_REGION_MAPPING_PREFIX)) {
- String region = key.substring(CONFIG_REGION_MAPPING_PREFIX.length());
- Set<String> packages = getStringPlusValue(properties.get(key));
- rpm.put(region, packages);
- } else if (key.startsWith(CONFIG_FEATURE_MAPPING_PREFIX)) {
- String feature = key.substring(CONFIG_FEATURE_MAPPING_PREFIX.length());
- Set<String> regions = getStringPlusValue(properties.get(key));
- frm.put(feature, regions);
- }
- }
-
- whitelistService = new WhitelistServiceImpl(rpm, frm);
- wlsRegistration = bundleContext.registerService(WhitelistService.class, whitelistService, null);
- }
-
- Set<String> getStringPlusValue(Object val) {
- if (val == null)
- return null;
-
- if (val instanceof Collection) {
- return ((Collection<?>) val).stream().map(Object::toString)
- .collect(Collectors.toSet());
- } else if (val instanceof String[]) {
- return new HashSet<>(Arrays.asList((String[]) val));
- }
- return Collections.singleton(val.toString());
- }
- */
}
diff --git a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImpl.java b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImpl.java
index 2d67428..29e20b7 100644
--- a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImpl.java
+++ b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImpl.java
@@ -44,10 +44,14 @@ public class WhitelistServiceFactoryImpl implements WhitelistServiceFactory {
Map<String, Set<String>> packages = mappings.get("packages");
Map<String, Set<String>> regions = mappings.get("regions");
- WhitelistService wls = new WhitelistServiceImpl(packages, regions);
+ WhitelistService wls = createWhitelistService(packages, regions);
WhitelistEnforcer enforcer = new WhitelistEnforcer(wls, featuresServiceTracker);
Hashtable<String, Set<String>> props = new Hashtable<>(packages);
props.putAll(regions);
bundleContext.registerService(ResolverHookFactory.class, enforcer, props);
}
+
+ WhitelistService createWhitelistService(Map<String, Set<String>> packages, Map<String, Set<String>> regions) {
+ return new WhitelistServiceImpl(packages, regions);
+ }
}
diff --git a/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/package-info.java b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/package-info.java
new file mode 100644
index 0000000..dedd3c2
--- /dev/null
+++ b/featuremodel/feature-whitelist/src/main/java/org/apache/sling/feature/whitelist/package-info.java
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+@org.osgi.annotation.versioning.Version("0.1.0")
+package org.apache.sling.feature.whitelist;
+
+
diff --git a/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/ResolverHookImplTest.java b/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/ResolverHookImplTest.java
index 18ab601..b20d71e 100644
--- a/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/ResolverHookImplTest.java
+++ b/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/ResolverHookImplTest.java
@@ -43,7 +43,6 @@ import java.util.Set;
import static org.junit.Assert.assertEquals;
public class ResolverHookImplTest {
-
@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
public void testFilterMatches() throws Exception {
@@ -53,12 +52,20 @@ public class ResolverHookImplTest {
String f4 = "gid4:aid4:1.2.3";
Features fs = Mockito.mock(Features.class);
- Mockito.when(fs.getFeatureForBundle("a.b.c", new Version(0,0,0))).thenReturn(f); // b7
- Mockito.when(fs.getFeatureForBundle("some.other.bundle", new Version(9,9,9,"suffix"))).thenReturn(f2); // b9
- Mockito.when(fs.getFeatureForBundle("a-bundle", new Version(1,0,0,"SNAPSHOT"))).thenReturn(f2); // b10
- Mockito.when(fs.getFeatureForBundle("a.b.c", new Version(1,2,3))).thenReturn(f3); // b17
- Mockito.when(fs.getFeatureForBundle("x.y.z", new Version(9,9,9))).thenReturn(f3); // b19
- Mockito.when(fs.getFeatureForBundle("zzz", new Version(1,0,0))).thenReturn(f4); // b20
+ Mockito.when(fs.getFeaturesForBundle("a.b.c", new Version(0,0,0)))
+ .thenReturn(Collections.singleton(f)); // b7
+ Mockito.when(fs.getFeaturesForBundle("some.other.bundle", new Version(9,9,9,"suffix")))
+ .thenReturn(Collections.singleton(f2)); // b9
+ Mockito.when(fs.getFeaturesForBundle("a-bundle", new Version(1,0,0,"SNAPSHOT")))
+ .thenReturn(Collections.singleton(f2)); // b10
+ Mockito.when(fs.getFeaturesForBundle("a.b.c", new Version(1,2,3)))
+ .thenReturn(Collections.singleton(f3)); // b17
+ Mockito.when(fs.getFeaturesForBundle("z.z.z", new Version(3,2,1)))
+ .thenReturn(new HashSet<>(Arrays.asList(f, f3))); // b18
+ Mockito.when(fs.getFeaturesForBundle("x.y.z", new Version(9,9,9)))
+ .thenReturn(Collections.singleton(f3)); // b19
+ Mockito.when(fs.getFeaturesForBundle("zzz", new Version(1,0,0)))
+ .thenReturn(Collections.singleton(f4)); // b20
ServiceTracker st = Mockito.mock(ServiceTracker.class);
Mockito.when(st.waitForService(Mockito.anyLong())).thenReturn(fs);
@@ -79,10 +86,32 @@ public class ResolverHookImplTest {
WhitelistService wls = new WhitelistServiceImpl(rpm, frm);
ResolverHookImpl rh = new ResolverHookImpl(st, wls);
+ // A requirement from a bundle that has no feature cannot access one in a region
+ BundleRequirement req9 = mockRequirement(11, "qqq", new Version(6,6,6));
+ BundleCapability bc9 = mockCapability("org.bar", 17, "a.b.c", new Version(1,2,3));
+ ArrayList c9 = new ArrayList<>(Arrays.asList(bc9));
+ rh.filterMatches(req9, c9);
+ assertEquals(0, c9.size());
+
+ // A requirement from a bundle that has no feature can still access on in the global region
+ BundleRequirement req10 = mockRequirement(11, "qqq", new Version(6,6,6));
+ BundleCapability bc10 = mockCapability("org.bar.tar", 18, "z.z.z", new Version(3,2,1));
+ ArrayList c10 = new ArrayList<>(Arrays.asList(bc10));
+ rh.filterMatches(req10, c10);
+ assertEquals(Collections.singletonList(bc10), c10);
+
+ // A requirement from a bundle that has no feature can be satisfied by a capability
+ // from a bundle that has no feature
+ BundleRequirement req11 = mockRequirement(11, "qqq", new Version(6,6,6));
+ BundleCapability bc11 = mockCapability("org.bar.tar", 16, "x", new Version(3,2,1));
+ ArrayList c11 = new ArrayList<>(Arrays.asList(bc11));
+ rh.filterMatches(req11, c11);
+ assertEquals(Collections.singletonList(bc11), c11);
+
// Check that we can get the capability from another bundle in the same region
- // Bundle 7 is in feature f with regions r1, r2
- BundleRequirement req = mockRequirement(17, "a.b.c", new Version(1,2,3));
- BundleCapability bc1 = mockCapability("org.foo", 19, "x.y.z", new Version(9,9,9));
+ // Bundle 7 is in feature f with regions r1, r2. Bundle 9 is in feature f2 with regions r2
+ BundleRequirement req = mockRequirement(7, "a.b.c", new Version(0,0,0));
+ BundleCapability bc1 = mockCapability("org.foo", 9, "some.other.bundle", new Version(9,9,9,"suffix"));
List<BundleCapability> candidates = new ArrayList<>(Arrays.asList(bc1));
rh.filterMatches(req, candidates);
assertEquals(Collections.singletonList(bc1), candidates);
diff --git a/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImplTest.java b/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImplTest.java
new file mode 100644
index 0000000..e53cbc4
--- /dev/null
+++ b/featuremodel/feature-whitelist/src/test/java/org/apache/sling/feature/whitelist/impl/WhitelistServiceFactoryImplTest.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.sling.feature.whitelist.impl;
+
+import org.apache.sling.feature.whitelist.WhitelistService;
+import org.apache.sling.feature.whitelist.WhitelistServiceFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.hooks.resolver.ResolverHookFactory;
+import org.osgi.util.tracker.ServiceTracker;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Dictionary;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class WhitelistServiceFactoryImplTest {
+ @SuppressWarnings({ "rawtypes", "unchecked" })
+ @Test
+ public void testWhitelistServiceFactory() {
+ List<ResolverHookFactory> resolverHookFactory = new ArrayList<>();
+ Map<String, Map<String, Set<String>>> wlsCfg = new HashMap<>();
+
+ ServiceTracker st = Mockito.mock(ServiceTracker.class);
+ BundleContext bc = Mockito.mock(BundleContext.class);
+ Mockito.when(bc.registerService(Mockito.isA(Class.class), Mockito.isA(Object.class), Mockito.isA(Dictionary.class)))
+ .then(i -> { resolverHookFactory.add(i.getArgument(1)); return null; });
+
+ WhitelistServiceFactory wsf = new WhitelistServiceFactoryImpl(bc, st) {
+ @Override
+ WhitelistService createWhitelistService(Map<String, Set<String>> packages, Map<String, Set<String>> regions) {
+ wlsCfg.put("packages", packages);
+ wlsCfg.put("regions", regions);
+ return super.createWhitelistService(packages, regions);
+ }
+ };
+
+ Map<String, Map<String, Set<String>>> m = new HashMap<>();
+ Map<String, Set<String>> packages = new HashMap<>();
+ packages.put("region1", new HashSet<>(Arrays.asList("org.foo", "org.bar")));
+ packages.put("region2", Collections.singleton("org.foo.bar"));
+ m.put("packages", packages);
+
+ Map<String, Set<String>> regions = new HashMap<>();
+ regions.put("f1", new HashSet<String>(Arrays.asList("region1", "region3")));
+ regions.put("f2", Collections.singleton("region2"));
+ regions.put("f3", Collections.singleton("region4"));
+ regions.put("f4", Collections.singleton("region2"));
+ m.put("regions", regions);
+
+ wsf.initialize(m);
+
+ assertEquals(wlsCfg, m);
+
+ ResolverHookFactory rhf = resolverHookFactory.get(0);
+ assertTrue(rhf instanceof WhitelistEnforcer);
+ }
+}