You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2017/03/23 16:58:50 UTC
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
GitHub user tbouron opened a pull request:
https://github.com/apache/brooklyn-server/pull/608
`BundleMaker` REST call allowing to add ZIP/JAR to catalog - Part #2
Builds on top of #485. It implements the missing bits and pieces, such as:
- fields `version` and `bundle` are required under `brooklyn.catalog` when uploading a ZIP/JAR archive
- returns 400 HTTP status code is anything goes wrong during the process of the archive (as this is a bad input, therefore a bad request)
- avoid using the `CatalogPopulator` as it reloads all bundles. Instead, it uses the new `CatalogBundleLoader` that loads only a given bundle
- if the bundle has been installed, but the CAMP parser fails, the bundle is uninstalled
- adds more unit tests, especially to check error handling
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tbouron/brooklyn-server feature/upload-archive
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/608.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #608
----
commit 178f81591e5a9786157d768efe2091d16763ed9a
Author: Alex Heneveld <al...@alexs-macbook-pro.local>
Date: 2016-12-12T16:55:49Z
add ZIP or JAR via REST API, with tests
commit 2a8b4c75c836ace488bc23920ac69a1e0c66cede
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-03-09T09:59:56Z
require bundle name/version defined in catalog.bom when POSTing a ZIP/JAR
accepts id, symbolicName, or symbolic-name under brooklyn.catalog;
if osgi bundle manifest supplied, it must match.
commit 203d0d75e3edfaf44c5f15808cd4e4df0c322244
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2017-03-23T16:04:43Z
Require bundle and version fields when uploading an archive through the REST API
commit 37ad7e13849da29b34c7712c3aa589aa3b1dbf5a
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2017-03-23T16:05:29Z
Separate the CatalogPopulator from the CatalogBundleLoader
commit 088f9e340e9415ed443f973c213014cd7bcae982
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2017-03-23T16:15:24Z
Using the new CatalogBundleLoader and make sure the REST API returns a 400 + uninstall the bundle if an error occur
commit 61d6f338c0e363e63ec50221dbbe2e45e737b7b0
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2017-03-23T16:52:38Z
Add unit tests for the new upload archive feature
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
Thanks for the pointer, missed that in the changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r107733604
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java ---
@@ -38,27 +38,60 @@
import org.apache.brooklyn.rest.domain.CatalogLocationSummary;
import org.apache.brooklyn.rest.domain.CatalogPolicySummary;
+import com.google.common.annotations.Beta;
+
import io.swagger.annotations.Api;
-import io.swagger.annotations.ApiResponse;
-import io.swagger.annotations.ApiResponses;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
@Path("/catalog")
@Api("Catalog")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public interface CatalogApi {
- @Consumes
+ @Deprecated /** @deprecated since 0.11.0 use {@link #createFromYaml(String)} instead */
+ public Response create(String yaml);
+
+ @Consumes({MediaType.APPLICATION_JSON, "application/x-yaml",
+ // see http://stackoverflow.com/questions/332129/yaml-mime-type
+ "text/yaml", "text/x-yaml", "application/yaml"})
@POST
- @ApiOperation(value = "Add a catalog item (e.g. new type of entity, policy or location) by uploading YAML descriptor "
+ @ApiOperation(value = "Add a catalog item (e.g. new type of entity, policy or location) by uploading YAML descriptor. "
+ "Return value is map of ID to CatalogItemSummary, with code 201 CREATED.", response = String.class)
- public Response create(
+ public Response createFromYaml(
@ApiParam(name = "yaml", value = "YAML descriptor of catalog item", required = true)
@Valid String yaml);
@POST
+ @Beta
+ @Consumes // anything (if doesn't match other methods with specific content types
+ @ApiOperation(value = "Add items to the catalog, either YAML or JAR/ZIP, format autodetected. "
+ + "Specify a content-type header to skip auto-detection and invoke one of the more specific methods. "
+ + "Return value is 201 CREATED if bundle could be added.", response = String.class)
+ public Response createFromUpload(
+ @ApiParam(
+ name = "item",
+ value = "Item to install, as JAR/ZIP or Catalog YAML (autodetected)",
+ required = true)
+ byte[] item);
+
+ @POST
+ @Beta
+ @Consumes({"application/x-zip", "application/x-jar"})
+ @ApiOperation(value = "Add a catalog item (e.g. new type of entity, policy or location) by uploading OSGi bundle JAR, or ZIP which will be turned into bundle JAR, "
+ + "containing catalog.bom containing bundle name and version. "
+ + "Return value is 201 CREATED if bundle could be added.", response = String.class)
+ public Response createFromArchive(
--- End diff --
Would it be worth deleting this and `createFromYaml` so that Swagger on the UI had only one endpoint to present?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r108101905
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---
@@ -245,13 +246,21 @@ public Response createFromArchive(byte[] zipInput) {
if (!BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM)) {
// if the above feature is not enabled, let's do it manually (as a contract of this method)
- new CatalogBomScanner().new CatalogPopulator(
- ((LocalManagementContext) mgmt()).getOsgiManager().get().getFramework().getBundleContext(),
- mgmt()).addingBundle(bundle, null);
+ try {
+ new CatalogBundleLoader(new CatalogBomScanner(), mgmt()).scanForCatalog(bundle);
--- End diff --
@geomacy Yup, the dependency on `CatalogBomScanner` is ugly. I knew it had to be changed but wanted this to be merged as soon as possible so I just moved the code around rather than really refactor.
Re the whitelist injection for the REST API: I was wondering what if we add the `CatalogBomScanner` to the new `ScratchPad` area? It's supposed to be a singleton isn't it? We can then get back the correctly setup predicate from it.
WDYT?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
Not sure what you mean by "update the plugin configuration". Couldn't see any related configuration in the pom.xml. No strong opinion though, either one is fine by me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r107729254
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java ---
@@ -195,22 +210,29 @@ public void removedBundle(Bundle bundle, BundleEvent bundleEvent, Iterable<? ext
for (CatalogItem<?, ?> item : items) {
LOG.debug("Unloading {} {} from catalog", item.getSymbolicName(), item.getVersion());
- removeFromCatalog(item);
+ catalogBundleLoader.removeFromCatalog(item);
}
}
+ }
- private void removeFromCatalog(CatalogItem<?, ?> item) {
- try {
- getManagementContext().getCatalog().deleteCatalogItem(item.getSymbolicName(), item.getVersion());
- } catch (Exception e) {
- Exceptions.propagateIfFatal(e);
- LOG.warn(Strings.join(new String[] {
- "Failed to remove", item.getSymbolicName(), item.getVersion(), "from catalog"
- }, " "), e);
- }
+ @Beta
+ public class CatalogBundleLoader {
--- End diff --
I'd say it's worth splitting this out into a standalone class. Same for CatalogPopulator; which I think we should rename to something like 'CatalogLoadingBundleTracker'.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@neykov Should be fixed now. I also created https://github.com/apache/brooklyn-client/pull/41 to use the non-deprecated call
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@tbouron Looks like #485 broke the `brooklyn-client` build, can you try fixing it here.
See https://builds.apache.org/view/Brooklyn/job/brooklyn-client-master/41/console.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
I think this looks good; further work as discussed earlier can be done as a separate PR. Will merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
hi @tbouron I get an error trying to build this
```
[ERROR] Failed to execute goal com.github.kongchen:swagger-maven-plugin:3.1.4:generate (default) on project brooklyn-rest-api: Unable to parse configuration of mojo com.github.kongchen:swagger-maven-plugin:3.1.4:generate for parameter locations: Cannot assign configuration entry 'locations' with value 'org.apache.brooklyn.rest.api' of type java.lang.String to property of type java.util.List -> [Help 1]
```
I just synced and built this as-is, should I be merging pre-requisite PRs or something?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@neykov You didn't miss it, I added it after your comment to solve the issue ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@tbouron started getting test failures in [VanillaSoftwareProcessTest.testVanillaSoftwareProcessCanUseResourcesInBundles](https://builds.apache.org/view/Brooklyn/job/brooklyn-integration-tests/70/testngreports/org.apache.brooklyn.rest.test.entity/VanillaSoftwareProcessTest/testVanillaSoftwareProcessCanUseResourcesInBundles/) around the catalog changes. Could you have a look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
Thanks @tbouron.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r107961617
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---
@@ -245,13 +246,21 @@ public Response createFromArchive(byte[] zipInput) {
if (!BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM)) {
// if the above feature is not enabled, let's do it manually (as a contract of this method)
- new CatalogBomScanner().new CatalogPopulator(
- ((LocalManagementContext) mgmt()).getOsgiManager().get().getFramework().getBundleContext(),
- mgmt()).addingBundle(bundle, null);
+ try {
+ new CatalogBundleLoader(new CatalogBomScanner(), mgmt()).scanForCatalog(bundle);
--- End diff --
This is a bit of a problem; you shouldn't have to pass a `CatalogBomScanner()` in here; the point of doing this decoupling was so that the `CatalogBundleLoader` could avoid knowing about the `CatalogBomScanner`.
I see that the use that is made of this is handling the [whitelist/blacklist](https://github.com/apache/brooklyn-server/commit/7046ba59cefd7e82ffad3f97057b3646915d1f08#diff-c46e6ef1f0a91ac1f58e7a340ecc0f34) functionality. (Well there is also `CatalogBomScanner.bundleIds()` but that can be made a static method,
and maybe moved to `CatalogUtils`.)
This whitelist stuff is configured [here](https://github.com/apache/brooklyn-server/blob/master/karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml#L123), in the definition of the singleton scanner object. (Only supported in Karaf launcher at the moment, as you see.)
Doing `new CatalogBundleLoader(new CatalogBomScanner(), mgmt()).scanForCatalog(bundle);` means you are supplying an object that hasn't been configured with the right white/blacklists, so it bypasses this restriction.
I'm going to suggest this is something we can live with for now, but mark as a "TODO" for future action.
However, it is still ugly to have the dependency on the `CatalogBomScanner` in the tracker and the loader, so I suggest we replace it with a simple predicate that wraps the whitelist checks and inject the predicate into the loader. That way the CatalogBomScanner can inject the correct whitelist configuration in the blueprint.xml, while in the REST API here we just live with an "always true" for the moment.
The loader could also be created in the scanner and injected into the tracker, which will simplify the latter a bit.
That's all a bit vague; I have to play around with the code a bit to help me think, so here's a PR that
illustrates what I mean - https://github.com/tbouron/brooklyn-server/pull/2
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@neykov I'm talking [about this](https://github.com/apache/brooklyn-server/pull/608/commits/5fb40ff8ae077da061aee927a773ec7a232df97d)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
Sounds like we should bump the maven version requirement - https://github.com/apache/brooklyn-server/blob/master/parent/pom.xml#L948. Or perhaps downgrade `swagger-maven-plugin`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@neykov Yep, I'll look into it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
That was quick, thanks @tbouron.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@neykov we cannot downgrade `swagger-maven-plugin` because of the issue I reported in [my comment](https://github.com/apache/brooklyn-server/pull/608#issuecomment-289000101).
We can either update the plugin configuration to properly use list instead of comma separated string or bump the maven requirement. Up to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@geomacy I addressed your comments and separated out the `CatalogBomScanner`, `CatalogBundleTracker` and `CatalogBundleLoader`. I also dug into the swagger docs and found out how to hide specific `@ApiOperation`. I had to bump the version of the `swagger-maven-plugin` because it was throwing an NPE with the new `hidden` parameter.
I did a full build locally, with the latest master of the other submodules, everything went fine so I'm guessing the error reported by Jenkins was due to something else
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
I hope that PR of mine could just be used as-is to finish off that refactoring. Personally I wouldn't worry too much about getting this integrated just now with a singleton `CatalogBomScanner`, especially as this is disabled by default at the minute anyway (see #233), until it can be made to work well with rebind.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r107735126
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java ---
@@ -195,22 +210,29 @@ public void removedBundle(Bundle bundle, BundleEvent bundleEvent, Iterable<? ext
for (CatalogItem<?, ?> item : items) {
LOG.debug("Unloading {} {} from catalog", item.getSymbolicName(), item.getVersion());
- removeFromCatalog(item);
+ catalogBundleLoader.removeFromCatalog(item);
}
}
+ }
- private void removeFromCatalog(CatalogItem<?, ?> item) {
- try {
- getManagementContext().getCatalog().deleteCatalogItem(item.getSymbolicName(), item.getVersion());
- } catch (Exception e) {
- Exceptions.propagateIfFatal(e);
- LOG.warn(Strings.join(new String[] {
- "Failed to remove", item.getSymbolicName(), item.getVersion(), "from catalog"
- }, " "), e);
- }
+ @Beta
+ public class CatalogBundleLoader {
--- End diff --
We should also remove the changes introduced in 33f7940c18ba8cd17fd83500486e02376d3b8fd8 to CatalogPopulator, i.e. adding the `CatalogPopulator(BundleContext bundleContext, ManagementContext managementContext) ` constructor and `bundleContext` member variable, and re-simplifying `open()` and `close()` and anything else that uses them. Those changes were introduced for the purposes of this PR, but are not needed now that the new class has been factored out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@neykov Fixed in #608
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r107731388
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java ---
@@ -195,22 +210,29 @@ public void removedBundle(Bundle bundle, BundleEvent bundleEvent, Iterable<? ext
for (CatalogItem<?, ?> item : items) {
LOG.debug("Unloading {} {} from catalog", item.getSymbolicName(), item.getVersion());
- removeFromCatalog(item);
+ catalogBundleLoader.removeFromCatalog(item);
}
}
+ }
- private void removeFromCatalog(CatalogItem<?, ?> item) {
- try {
- getManagementContext().getCatalog().deleteCatalogItem(item.getSymbolicName(), item.getVersion());
- } catch (Exception e) {
- Exceptions.propagateIfFatal(e);
- LOG.warn(Strings.join(new String[] {
- "Failed to remove", item.getSymbolicName(), item.getVersion(), "from catalog"
- }, " "), e);
- }
+ @Beta
+ public class CatalogBundleLoader {
--- End diff --
In CatalogBomScanner, now that this refactorization has being done we should remove the constructor
```
CatalogPopulator(BundleContext bundleContext, ManagementContext managementContext)
```
which was introduced for this purpose, including deleting the `bundleContext` and `managementContext` members and re-simplifying any methods that use them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r108382104
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java ---
@@ -0,0 +1,180 @@
+/*
+ * 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.brooklyn.core.catalog.internal;
+
+import static org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType.TEMPLATE;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.stream.Streams;
+import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.yaml.Yamls;
+import org.osgi.framework.Bundle;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.yaml.snakeyaml.DumperOptions;
+import org.yaml.snakeyaml.Yaml;
+
+import com.google.common.annotations.Beta;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
+@Beta
+public class CatalogBundleLoader {
+
+ private static final Logger LOG = LoggerFactory.getLogger(CatalogBundleLoader.class);
+ private static final String CATALOG_BOM_URL = "catalog.bom";
+ private static final String BROOKLYN_CATALOG = "brooklyn.catalog";
+ private static final String BROOKLYN_LIBRARIES = "brooklyn.libraries";
+
+ private Predicate<Bundle> applicationsPermitted;
+ private ManagementContext managementContext;
+
+ public CatalogBundleLoader(Predicate<Bundle> applicationsPermitted, ManagementContext managementContext) {
+ this.applicationsPermitted = applicationsPermitted;
+ this.managementContext = managementContext;
+ }
+
+ /**
+ * Scan the given bundle for a catalog.bom and adds it to the catalog.
+ *
+ * @param bundle The bundle to add
+ * @return A list of items added to the catalog
+ * @throws RuntimeException if the catalog items failed to be added to the catalog
+ */
+ public Iterable<? extends CatalogItem<?, ?>> scanForCatalog(Bundle bundle) {
--- End diff --
I'd prefer a name that makes clear the method will add the catalog items (as well as scanning + returning the items found). For example `addCatalogItemsFromBundle(...)`?
(This method name was originally in `CatalogBomScanner`, and was refactored to here - but in CatalogBomScanner it was a private method so the name didn't need to be as clear.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
@geomacy Hum odd. I have build this multiple time locally with the latest master of all the sub-modules. Jenkins also seems happy.
I did [another PR](https://github.com/apache/brooklyn-client/pull/41) in `brooklyn-client` to fix the tests in the client (related to the changes I did here) but I don't see how that could be related.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/608
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
aha - Maven version issue. Upgrading 3.3.3 -> 3.3.9 fixed the problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #608: `BundleMaker` REST call allowing to add ZIP/JAR ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/608
Thanks for polishing this off @tbouron !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #608: `BundleMaker` REST call allowing to add Z...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/608#discussion_r107735424
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---
@@ -129,6 +176,97 @@ public Response create(String yaml) {
return Response.status(Status.CREATED).entity(result).build();
}
+ @Override
+ @Beta
+ public Response createFromArchive(byte[] zipInput) {
+ if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.ROOT, null)) {
+ throw WebResourceUtils.forbidden("User '%s' is not authorized to add catalog item",
+ Entitlements.getEntitlementContext().user());
+ }
+
+ BundleMaker bm = new BundleMaker(mgmt());
+ File f=null, f2=null;
+ try {
+ f = Os.newTempFile("brooklyn-posted-archive", "zip");
+ try {
+ Files.write(zipInput, f);
+ } catch (IOException e) {
+ Exceptions.propagate(e);
+ }
+
+ ZipFile zf;
+ try {
+ zf = new ZipFile(f);
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Invalid ZIP/JAR archive: "+e);
+ }
+ ZipArchiveEntry bom = zf.getEntry("catalog.bom");
+ if (bom==null) {
+ bom = zf.getEntry("/catalog.bom");
+ }
+ if (bom==null) {
+ throw new IllegalArgumentException("Archive must contain a catalog.bom file in the root");
+ }
+ String bomS;
+ try {
+ bomS = Streams.readFullyString(zf.getInputStream(bom));
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Error reading catalog.bom from ZIP/JAR archive: "+e);
+ }
+ VersionedName vn = BasicBrooklynCatalog.getVersionedName( BasicBrooklynCatalog.getCatalogMetadata(bomS) );
+
+ Manifest mf = bm.getManifest(f);
+ if (mf==null) {
+ mf = new Manifest();
+ }
+ String bundleNameInMF = mf.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
+ if (Strings.isNonBlank(bundleNameInMF)) {
+ if (!bundleNameInMF.equals(vn.getSymbolicName())) {
+ throw new IllegalArgumentException("JAR MANIFEST symbolic-name '"+bundleNameInMF+"' does not match '"+vn.getSymbolicName()+"' defined in BOM");
+ }
+ } else {
+ mf.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, vn.getSymbolicName());
+ }
+
+ String bundleVersionInMF = mf.getMainAttributes().getValue(Constants.BUNDLE_VERSION);
+ if (Strings.isNonBlank(bundleVersionInMF)) {
+ if (!bundleVersionInMF.equals(vn.getVersion().toString())) {
+ throw new IllegalArgumentException("JAR MANIFEST version '"+bundleVersionInMF+"' does not match '"+vn.getVersion()+"' defined in BOM");
+ }
+ } else {
+ mf.getMainAttributes().putValue(Constants.BUNDLE_VERSION, vn.getVersion().toString());
+ }
+ if (mf.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION)==null) {
+ mf.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), "1.0");
+ }
+
+ f2 = bm.copyAddingManifest(f, mf);
+
+ Bundle bundle = bm.installBundle(f2, true);
+
+ if (!BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM)) {
+ // if the above feature is not enabled, let's do it manually (as a contract of this method)
+ try {
+ new CatalogBomScanner().new CatalogBundleLoader(mgmt()).scanForCatalog(bundle);
--- End diff --
This should just be `new CatalogBundleLoader(mgmt()).scanForCatalog(bundle);` (with it as a standalone class)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---