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

Add OSGi support to mssql-jdbc #24

Closed
lamperi opened this issue Nov 24, 2016 · 11 comments
Closed

Add OSGi support to mssql-jdbc #24

lamperi opened this issue Nov 24, 2016 · 11 comments
Labels
Waiting for Response Waiting for a reply from the original poster, or affiliated party
Milestone

Comments

@lamperi
Copy link

lamperi commented Nov 24, 2016

OSGi support basically means adding OSGI specific headers to MANIFEST.MF done automatically by maven-bundle-plugin from Apache Felix or bnd-maven-plugin from Bndtools, so that the driver could be loaded in OSGi environmental.

One thing I see as problematic is the dependency to Microsoft Azure Keyvault, which is also not OSGI compatible and their plans do not promise any good: Azure/azure-sdk-for-java#471. Transitive Microsoft dependencies also seem to be non-OSGi compatible. Apache HTTPClient dependency should be ok as the jar is OSGi ready, although the HTTP client dependency seems to be only because of Azure connections.

@v-nisidh v-nisidh added the Enhancement An enhancement to the driver. Lower priority than bugs. label Nov 24, 2016
@v-nisidh
Copy link
Contributor

Appreciate your suggestion. We will take this as an enhancement. Thanks for pointing out maven-bundle-plugins.
I am not sure about Transitive dependencies but we think It will be good start to support OSGI.

@marschall
Copy link
Contributor

Actually OSGi support would include more than a MANIFEST.MF. It would also include the implementation of a DataSourceFactory and registering it as an OSGi service with the correct properties either through an activator or DS.

Yes, Microsoft Azure Keyvault may be a bit more involved but it's an optional dependency so it could be left out in a first approach.

@v-nisidh
Copy link
Contributor

v-nisidh commented Dec 2, 2016

Thanks @marschall for pointing out. I was under impression that we need implementation(s) only for OSGI Compliant.

Task List for Investigation:

  • OSGI Specific Headers
  • Transitive Dependency of Azure Keyvault
  • Implementation of DataSourceFactory
  • Registering it as an OSGI Service

Able to create OSGI Specific Headers with Transitive dependency.

@marschall
Copy link
Contributor

The Azure Keyvault solution on your end may actually be quite simple. IMHO the correct solution would be a combination of Import-Package and resolution:=optional rather than using Require-Bundle. That way it works without the Keyvault being present and with the Keyvault being present independent of the way how it's implemented. Users who need the Keyvault can either use the official bundles once they're published or their own wrapper bundles as a stop gap measure around them.

@v-nisidh v-nisidh modified the milestone: Long Term Goals Feb 6, 2017
@v-nisidh v-nisidh modified the milestones: Long Term Goals, Next Tasks Feb 14, 2017
@v-nisidh v-nisidh modified the milestones: 6.1.6, Next Tasks Feb 28, 2017
@vinayshankar
Copy link

When will 6.1.6 be released?

@v-nisidh
Copy link
Contributor

@lamperi : Used Apache Felix maven-bundle-plugin to generate OSGI specific headers. Tested for both profiles (JDK 1.7 & 1.8).

You can refer pom.xml from OSGI branch.

Can you verify this OSGI headers suffice your purpose or not?

Manifest-Version: 1.0
Bnd-LastModified: 1490235349470
Build-Jdk: 1.8.0_112
Built-By: nikhils
Bundle-Description: Microsoft JDBC Driver for SQL Server.		The Azure Key
  Vault feature in Microsoft JDBC Driver for SQL Server depends on 		Azu
 re SDK for JAVA and Azure Active Directory Library For Java.
Bundle-License: http://www.opensource.org/licenses/mit-license.php
Bundle-ManifestVersion: 2
Bundle-Name: Microsoft JDBC Driver for SQL Server
Bundle-SymbolicName: com.microsoft.sqlserver.mssql-jdbc
Bundle-Vendor: Microsoft Corporation
Bundle-Version: 6.1.5
Created-By: Apache Maven Bundle Plugin
Export-Package: com.microsoft.sqlserver.jdbc;uses:="javax.naming,javax.n
 aming.spi,javax.sql,javax.transaction.xa,microsoft.sql,org.ietf.jgss";v
 ersion="6.1.5",microsoft.sql;version="6.1.5"
Import-Package: com.microsoft.aad.adal4j;resolution:=optional,com.micros
 oft.azure.keyvault;resolution:=optional,com.microsoft.azure.keyvault.au
 thentication;resolution:=optional,com.microsoft.azure.keyvault.models;r
 esolution:=optional,com.microsoft.azure.keyvault.webkey;resolution:=opt
 ional,com.microsoft.windowsazure.core.pipeline.filter;resolution:=optio
 nal,com.microsoft.windowsazure.credentials;resolution:=optional,javax.c
 rypto,javax.crypto.spec,javax.naming,javax.naming.spi,javax.net.ssl,jav
 ax.security.auth,javax.security.auth.login,javax.security.auth.x500,jav
 ax.sql,javax.transaction.xa,javax.xml.bind;resolution:=optional,javax.x
 ml.parsers,javax.xml.stream;resolution:=optional,javax.xml.transform,ja
 vax.xml.transform.dom,javax.xml.transform.sax,javax.xml.transform.stax,
 javax.xml.transform.stream,microsoft.sql,org.apache.http;resolution:=op
 tional,org.apache.http.impl.client;resolution:=optional,org.apache.http
 .message;resolution:=optional,org.ietf.jgss,org.w3c.dom,org.xml.sax,org
 .xml.sax.helpers
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.7))"
Tool: Bnd-3.2.0.201605172007

@marschall
Copy link
Contributor

@v-nisidh a couple of things caught my eye

  • javax.xml.bind: is "only" used for base64 conversion, I don't think this is optional, hopefully once the driver is Java 8 only, this can be replaced with java.util.Base64
  • the bundle both imports and exports microsoft.sql, I don't know if this is an issue
  • I'm not familiar with the pros an cons of requiring the "osgi.ee" capability versus a bundle execution environment
  • there are pros and cons to lazy bundle activation
  • you probably want to make the bundle a singleton as I don't think JDBC can cope with two versions of the same driver

other than that it looks find to me.

@v-nisidh
Copy link
Contributor

Thanks @marschall, for pointing out microsoft.sql in both import & export while javax.xml.bind is due to transitive dependency of Azure.

@marschall & @lamperi : For this release we are not considering implementation of DataSourceFactory & registering as a service due to following reasons:

  1. Lack of OSGI enabled environment in our internal test lab. Although I spend some time with Apache Karaf but I think Karaf is not a good candidate for testing due to it's ability to deploying non osgi jars.
  2. For implementation of DataSourceFactory & BundleActivator we need to add another dependency org.osgi.enterprise. Not sure about impact on Non-OSGI world.
  3. For OSGI dependency, we will put this is an optional dependency but not sure how it will work in OSGI environment (due to optional). Again need some OSGI environment to test it.

I will definitely consider your last point while implementing BundleActivator and DataSourceFactory for OSGI

I opened PR #218 for adding OSGI headers.

@v-nisidh v-nisidh added Waiting for Response Waiting for a reply from the original poster, or affiliated party and removed Enhancement An enhancement to the driver. Lower priority than bugs. under development labels Mar 25, 2017
@v-nisidh
Copy link
Contributor

v-nisidh commented Mar 31, 2017

@lamperi I hope this PR #218 will be beneficial to you. We are adding OSGI headers in MANIFEST.MF.

@lamperi
Copy link
Author

lamperi commented Mar 31, 2017

Thanks. I checked the PR branch on my own machine, and it bundle goes active in Karaf correctly without wrapping. I don't have SQL Server setup currently but it looks fine otherwise!

@v-nisidh
Copy link
Contributor

v-nisidh commented Apr 3, 2017

@lamperi : I am closing this issue in favor of PR #218. We already included this in 6.1.6-preview version . Ref: Milestone 6.1.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for Response Waiting for a reply from the original poster, or affiliated party
Projects
None yet
Development

No branches or pull requests

4 participants