Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bell service implementations cannot load SPI #16317

Closed
tjwatson opened this issue Mar 22, 2021 · 7 comments
Closed

Bell service implementations cannot load SPI #16317

tjwatson opened this issue Mar 22, 2021 · 7 comments
Assignees
Labels

Comments

@tjwatson
Copy link
Member

tjwatson commented Mar 22, 2021

For epic #19368

A bell in liberty is configured with a reference to a shared library configuration that provides Java SE service loader meta-data (META-INF/services) to specify implementations of services that can be used to extend the functionality of Liberty. The class loader that is used to load shared libraries is only allowed to access API packages that are exposed by the features enabled with the server configuration.

This prevents a bell from implementing service interfaces which are included in SPI packages in Liberty. There are many scenarios where a Liberty feature has SPI to allow product extensions to extend the functionality of Liberty through the implementation of a service interface included in one of the Liberty SPI packages. Because SPI are only available to product extensions this forces developers to use their own product extension to register an OSGi service that implements the SPI service interface.

See thread https://groups.io/g/openliberty/topic/81482285#646

This design issue is to discuss the options for enabling shared libraries that are configured for a bell have access to SPI packages. Shared libraries can be configured for application class loader delegation. This enhancement for bells must not expose application class loaders to SPI packages.

Possible solutions

  1. Add a boolean attribute to <library/> configuration to enable access to SPI called spiVisibilityEnabled, default false
  2. Add a string attribute to <library/> configuration to specify SPI types visible called spiTypeVisibility, default to empty. Only supported type is ibm-spi. Currently Liberty does not differentiate types of SPI, a package is either SPI or not, but this option would allow us to add different types later.
  3. Others

Regardless of configuration, the solution must ensure that delegation to common shared libraries does not expose the application class loader to more packages than configured with the application class loader. The application class loader must never have access to SPI packages. A quick look at the application class loader delegation implementation seems to allow common shared library package visibility to be different from the application class loader package visibility setting already.

@NottyCode
Copy link
Member

NottyCode commented Mar 22, 2021

Another possible implementation would be to allow someone to configure via the existing apiTypeVisibility:

  <library apiTypeVisibility="spi" />

which was suggested in the email thread. My concern with this is if we do that and do not open this up to applications an application could never reference the library which would make it impossible for the bell to provide an application api (the reason for this is that I think the library and application class loader apiTypeVisbility needs to match). I'm also not keen on exposing SPIs to applications. However perhaps since this isn't default I'm being overly paranoid and we shouldn't worry about it.

I do think that bells is a simpler way to extend the runtime than expecting people to write OSGi features so I'm in favour of increasing the applicability of it.

@tjwatson
Copy link
Member Author

I also thought the library and application class loader apiTypeVisbility needs to match for an application to use the shared library, but I could not find documentation or code to confirm that.

@tjwatson
Copy link
Member Author

Design Call:

  • Tom W. and Andy M. need to discuss the issues that may arise if we allow a common shared library to have access to SPI and that library is used by an application class loader that does not have access to SPI.
  • Agreed that a solution must not grant access to SPI by application class loaders
  • Agreed that a separate configuration should be used instead of adding a new spi type to the existing apiTypeVisibility attribute.
  • General direction is to add spiTypeVisibility attribute that only allows a single type for now (e.g. spi). Allows the chance to add new spi types later if needed. Currently nobody had a concrete reason to add new SPI types.

Leaving the issue in the backlog to discuss again in two weeks.

@tjwatson
Copy link
Member Author

tjwatson commented Apr 6, 2021

Instead of opening up a can of worms to allow Libraries and Applications to see different sets of API it was decided that the rule to enforce a match on API visibility will remain. But we will implement a new configuration attribute for libraries spiTypeVisibility with the single value spi allowed. We decided the risk is low for the application to encounter cast exceptions if they package the SPI packages in their own application and try to configure it to use a Library that has access to SPI.

@tjwatson
Copy link
Member Author

tjwatson commented Apr 6, 2021

Note to whomever looks at implementing this. The current check to make sure the API visibility matches between application and library class loaders is done in com.ibm.ws.classloading.internal.providers.Providers.checkAPITypesMatch. This must continue to check that the apiTypeVisibility values match between the application and library. But now the library will be able to have an additional attribute that is separate from apiTypeVisibility to allow it to access SPI packages.

@dazavala dazavala self-assigned this Apr 7, 2021
@malincoln malincoln added the In Progress Items that are in active development. label Jun 7, 2021
@dazavala
Copy link
Contributor

dazavala commented Jun 22, 2022

Issue 20916 documents the BELL SPI visibility design concerns raised during UFO socialization. These changes are implemented and under review for beta release.

@dazavala dazavala added target:beta The Epic or Issue is targetted for the next beta target:200010-beta labels Aug 3, 2022
@dazavala dazavala added target:beta The Epic or Issue is targetted for the next beta target:220011 and removed target:beta The Epic or Issue is targetted for the next beta In Progress Items that are in active development. target:220011 labels Sep 12, 2022
@dazavala
Copy link
Contributor

Closing now. Story delivered to beta 220010. I'll reopen this issue in the event additional work is required for GA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Implemented
Development

No branches or pull requests

4 participants