You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2021/07/31 13:50:43 UTC
[sling-org-apache-sling-feature-analyser] branch master updated:
SLING-10695 : Create analyser to check for paths in content packages
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-analyser.git
The following commit(s) were added to refs/heads/master by this push:
new 8a14dad SLING-10695 : Create analyser to check for paths in content packages
8a14dad is described below
commit 8a14dad854250f34e8211e98d9f5c89a7aaba589
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Sat Jul 31 15:50:31 2021 +0200
SLING-10695 : Create analyser to check for paths in content packages
---
readme.md | 19 +++-
.../task/impl/CheckBundleExportsImports.java | 2 -
.../task/impl/CheckContentPackagesForPaths.java | 110 +++++++++++++++++++++
.../scanner/impl/ContentPackageDescriptor.java | 3 +
.../scanner/impl/ContentPackageScanner.java | 19 ++--
...apache.sling.feature.analyser.task.AnalyserTask | 1 +
.../impl/CheckContentPackagesForPathsTest.java | 89 +++++++++++++++++
7 files changed, 227 insertions(+), 16 deletions(-)
diff --git a/readme.md b/readme.md
index 9f17ff9..36f0001 100644
--- a/readme.md
+++ b/readme.md
@@ -14,7 +14,7 @@ java org.apache.sling.feature.analyser.main.Main
# Feature Model Analyser as a Maven Plugin
-The Analyser can also be run as part of a maven build via the `slingfeature-maven-plugin`: https://github.com/apache/sling-slingfeature-maven-plugin
+The Analyser can also be run as part of a maven build via the [slingfeature-maven-plugin](https://github.com/apache/sling-slingfeature-maven-plugin)
The following analysers are defined:
@@ -34,9 +34,9 @@ The following analysers are defined:
* `feature-id`: Checks if the used feature id matches one of the given Maven coordinates.
-Additional analysers in relation to Feature Model API Regions can be found here: https://github.com/apache/sling-org-apache-sling-feature-extension-apiregions
+Additional analysers in relation to Feature Model API Regions can be found here: [org-apache-sling-feature-extension-apiregions](https://github.com/apache/sling-org-apache-sling-feature-extension-apiregions)
-For further documentation see: https://github.com/apache/sling-org-apache-sling-feature/blob/master/readme.md
+For further documentation see: [Feature Model](https://github.com/apache/sling-org-apache-sling-feature/blob/master/readme.md)
## `compare-features`
@@ -44,7 +44,7 @@ This analyser compares certain sections of two feature models.
This analyser requires additional configuration:
- Configuration key | Allowed values | Description
+ Configuration key | Allowed values | Description
----- | ----- | -----
`compare-type` | `ARTIFACTS` | The types of entities being compared. Currently only artifacts can be compared.
`compare-with` | Maven ID, e.g. `mygroup:myart:1.2.3` | The _golden_ feature to compare the features selected for the analyser with.
@@ -58,6 +58,15 @@ This analyser checks that the feature id matches one of the given accepted featu
This analyser requires additional configuration:
- Configuration key | Allowed values | Description
+ Configuration key | Allowed values | Description
----- | ----- | -----
`accepted-feature-ids` | comma-separated list of Maven IDs | The Maven ID/coordinates have the format `groupId:artifactId[:packaging[:classifier]]:version`. Each item is either a string which must be equal to the according item of the feature id, or a `*` which acts as wildcard (i.e. everything matches).
+
+## `content-packages-paths`
+
+This analyser checks for allowed and denied paths inside content packages. This analyser requires additional configuration:
+
+ Configuration key | Allowed values | Description
+ ----- | ----- | -----
+`includes` | Content paths | A comma separated list of content paths. If this is specified all content in the content package must match at least one of these.
+`excludes` | Content paths | A comma separated list of content paths. If this is specified all content in the contant package must not match any of these.
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java
index c93461c..2c010a8 100644
--- a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java
@@ -162,8 +162,6 @@ public class CheckBundleExportsImports implements AnalyserTask {
boolean errorReported = false;
for(final Map.Entry<BundleDescriptor, Report> entry : reports.entrySet()) {
- final String key = "Bundle " + entry.getKey().getArtifact().getId().getArtifactId() + ":" + entry.getKey().getArtifact().getId().getVersion();
-
if ( !entry.getValue().importWithoutVersion.isEmpty() ) {
ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(), " is importing package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without specifying a version range.");
}
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java
new file mode 100644
index 0000000..50243fc
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java
@@ -0,0 +1,110 @@
+/*
+ * 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.analyser.task.impl;
+
+import org.apache.sling.feature.analyser.task.AnalyserTask;
+import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
+import org.apache.sling.feature.scanner.ArtifactDescriptor;
+import org.apache.sling.feature.scanner.impl.ContentPackageDescriptor;
+
+/**
+ * This analyser checks for content paths in packages
+ */
+public class CheckContentPackagesForPaths implements AnalyserTask {
+
+ private static final String PROP_INCLUDES = "includes";
+
+ private static final String PROP_EXCLUDES = "excludes";
+
+ @Override
+ public String getName() {
+ return "Content Packages Path Check";
+ }
+
+ @Override
+ public String getId() {
+ return "content-packages-paths";
+ }
+
+ @Override
+ public void execute(final AnalyserTaskContext ctx)
+ throws Exception {
+ final Rules rules = getRules(ctx);
+
+ if (rules != null ) {
+ for (final ArtifactDescriptor d : ctx.getFeatureDescriptor().getArtifactDescriptors()) {
+ if (d instanceof ContentPackageDescriptor) {
+ checkPackage(ctx, (ContentPackageDescriptor) d, rules);
+ }
+ }
+ } else {
+ ctx.reportError("Configuration for task " + getId() + " is missing.");
+ }
+ }
+
+ static final class Rules {
+ public String[] includes;
+ public String[] excludes;
+ }
+
+ Rules getRules(final AnalyserTaskContext ctx) {
+ final String inc = ctx.getConfiguration().get(PROP_INCLUDES);
+ final String exc = ctx.getConfiguration().get(PROP_EXCLUDES);
+
+ if ( inc != null || exc != null ) {
+ final Rules r = new Rules();
+ r.includes = inc == null ? null : inc.split(",");
+ clean(r.includes);
+ r.excludes = exc == null ? null : exc.split(",");
+ clean(r.excludes);
+ return r;
+ }
+ return null;
+ }
+ private static void clean(final String[] array) {
+ if ( array != null ) {
+ for(int i=0;i<array.length;i++) {
+ array[i] = array[i].trim();
+ }
+ }
+ }
+
+ void checkPackage(final AnalyserTaskContext ctx, final ContentPackageDescriptor desc, final Rules rules) {
+ for(final String path : desc.paths) {
+ boolean isAllowed = rules.includes == null;
+ if ( !isAllowed ) {
+ for(final String i : rules.includes) {
+ if ( path.equals(i) || path.startsWith(i.concat("/")) ) {
+ isAllowed = true;
+ break;
+ }
+ }
+ }
+ if ( isAllowed && rules.excludes != null ) {
+ for(final String i : rules.excludes) {
+ if ( path.equals(i) || path.startsWith(i.concat("/")) ) {
+ isAllowed = false;
+ break;
+ }
+ }
+ }
+ if ( !isAllowed ) {
+ ctx.reportArtifactError(desc.getArtifact().getId(), "Content not allowed: ".concat(path));
+ }
+ }
+ }
+}
diff --git a/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageDescriptor.java b/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageDescriptor.java
index 039689c..e9bf935 100644
--- a/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageDescriptor.java
+++ b/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageDescriptor.java
@@ -46,6 +46,9 @@ public class ContentPackageDescriptor extends ArtifactDescriptorImpl {
/** Configurations in the content package. */
public final List<Configuration> configs = new ArrayList<>();
+ /** Paths in the content package. */
+ public final List<String> paths = new ArrayList<>();
+
/** Optional: the artifact of the content package. */
private Artifact contentPackage;
diff --git a/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageScanner.java b/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageScanner.java
index c428675..d3f99cd 100644
--- a/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageScanner.java
+++ b/src/main/java/org/apache/sling/feature/scanner/impl/ContentPackageScanner.java
@@ -100,6 +100,7 @@ public class ContentPackageScanner {
if ( !entryName.endsWith("/") && entryName.startsWith("jcr_root/") ) {
final String contentPath = entryName.substring(8);
+ cp.paths.add(contentPath);
FileType fileType = null;
@@ -108,17 +109,17 @@ public class ContentPackageScanner {
fileType = FileType.PACKAGE;
// check for libs or apps
- } else if (entryName.startsWith("jcr_root/libs/") || entryName.startsWith("jcr_root/apps/")) {
+ } else if (contentPath.startsWith("/libs/") || contentPath.startsWith("/apps/")) {
// check if this is an install folder (I)
// install folders are either named:
// "install" or
// "install.{runmode}"
- boolean isInstall = entryName.indexOf("/install/") != -1;
+ boolean isInstall = contentPath.indexOf("/install/") != -1;
if (!isInstall) {
- final int pos = entryName.indexOf("/install.");
+ final int pos = contentPath.indexOf("/install.");
if (pos != -1) {
- final int endSlashPos = entryName.indexOf('/', pos + 1);
+ final int endSlashPos = contentPath.indexOf('/', pos + 1);
if (endSlashPos != -1) {
isInstall = true;
}
@@ -129,11 +130,11 @@ public class ContentPackageScanner {
// config folders are either named:
// "config" or
// "config.{runmode}"
- isInstall = entryName.indexOf("/config/") != -1;
+ isInstall = contentPath.indexOf("/config/") != -1;
if (!isInstall) {
- final int pos = entryName.indexOf("/config.");
+ final int pos = contentPath.indexOf("/config.");
if (pos != -1) {
- final int endSlashPos = entryName.indexOf('/', pos + 1);
+ final int endSlashPos = contentPath.indexOf('/', pos + 1);
if (endSlashPos != -1) {
isInstall = true;
}
@@ -143,9 +144,9 @@ public class ContentPackageScanner {
if (isInstall) {
- if (entryName.endsWith(".jar")) {
+ if (contentPath.endsWith(".jar")) {
fileType = FileType.BUNDLE;
- } else if (entryName.endsWith(".xml") || entryName.endsWith(".config")) {
+ } else if (contentPath.endsWith(".xml") || contentPath.endsWith(".config") || contentPath.endsWith(".cfg.json")) {
fileType = FileType.CONFIG;
}
}
diff --git a/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask b/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
index 503dc36..998e1f3 100644
--- a/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
+++ b/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
@@ -7,6 +7,7 @@ org.apache.sling.feature.analyser.task.impl.CheckBundlesForResources
org.apache.sling.feature.analyser.task.impl.CheckCompareFeatures
org.apache.sling.feature.analyser.task.impl.CheckContentPackageForInstallables
org.apache.sling.feature.analyser.task.impl.CheckContentPackagesDependencies
+org.apache.sling.feature.analyser.task.impl.CheckContentPackagesForPaths
org.apache.sling.feature.analyser.task.impl.CheckDuplicateSymbolicName
org.apache.sling.feature.analyser.task.impl.CheckRepoinit
org.apache.sling.feature.analyser.task.impl.CheckRequirementsCapabilities
diff --git a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java
new file mode 100644
index 0000000..57e57ef
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java
@@ -0,0 +1,89 @@
+/*
+ * 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.analyser.task.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.net.URL;
+
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
+import org.apache.sling.feature.analyser.task.impl.CheckContentPackagesForPaths.Rules;
+import org.apache.sling.feature.scanner.impl.ContentPackageDescriptor;
+import org.junit.Test;
+
+public class CheckContentPackagesForPathsTest {
+
+ @Test public void testNoRulesConfiguration() {
+ final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
+ final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
+
+ assertNull(analyser.getRules(ctx));
+ }
+
+ @Test public void testIncludesRulesConfiguration() {
+ final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
+ final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
+ ctx.getConfiguration().put("includes", "/a, /b");
+
+ final Rules r = analyser.getRules(ctx);
+ assertNull(r.excludes);
+ assertEquals(2, r.includes.length);
+ assertEquals("/a", r.includes[0]);
+ assertEquals("/b", r.includes[1]);
+ }
+
+ @Test public void testExcludesRulesConfiguration() {
+ final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
+ final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
+ ctx.getConfiguration().put("excludes", "/a, /b");
+
+ final Rules r = analyser.getRules(ctx);
+ assertNull(r.includes);
+ assertEquals(2, r.excludes.length);
+ assertEquals("/a", r.excludes[0]);
+ assertEquals("/b", r.excludes[1]);
+ }
+
+ @Test public void testPathsCheck() throws IOException {
+ final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
+ final AnalyserTaskContextImpl ctx = new AnalyserTaskContextImpl();
+ ctx.getConfiguration().put("excludes", "/a");
+ ctx.getConfiguration().put("includes", "/ab,/b");
+
+ final Rules r = analyser.getRules(ctx);
+ final ContentPackageDescriptor desc = new ContentPackageDescriptor("name", new Artifact(ArtifactId.parse("g:a:1")),
+ new URL("https://sling.apache.org"));
+ desc.paths.add("/b/foo");
+ desc.paths.add("/a");
+ desc.paths.add("/a/foo");
+ desc.paths.add("/ab");
+ desc.paths.add("/b");
+ desc.paths.add("/c");
+
+ analyser.checkPackage(ctx, desc, r);
+
+ assertEquals(3, ctx.getErrors().size());
+ assertTrue(ctx.getErrors().get(0), ctx.getErrors().get(0).endsWith(" /a"));
+ assertTrue(ctx.getErrors().get(1), ctx.getErrors().get(1).endsWith(" /a/foo"));
+ assertTrue(ctx.getErrors().get(2), ctx.getErrors().get(2).endsWith(" /c"));
+ }
+}