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

IBM Semeru Runtime Certified Edition for z/OS, Kerberos and mssql-jdbc don't work together #2576 #2581

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

muskan124947
Copy link
Contributor

@muskan124947 muskan124947 commented Jan 10, 2025

Github Issue: #2576

@muskan124947
Copy link
Contributor Author

/azp run public-mssql-jdbc.windows

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@muskan124947
Copy link
Contributor Author

/azp run public-mssql-jdbc.linux

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@muskan124947
Copy link
Contributor Author

/azp run CI-MacOS

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 21.42857% with 22 lines in your changes missing coverage. Please review.

Project coverage is 51.22%. Comparing base (03cfcfd) to head (6187602).

Files with missing lines Patch % Lines
...om/microsoft/sqlserver/jdbc/JaasConfiguration.java 0.00% 12 Missing ⚠️
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 50.00% 5 Missing and 1 partial ⚠️
...m/microsoft/sqlserver/jdbc/KerbAuthentication.java 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2581   +/-   ##
=========================================
  Coverage     51.22%   51.22%           
- Complexity     3956     3957    +1     
=========================================
  Files           147      147           
  Lines         33657    33678   +21     
  Branches       5624     5624           
=========================================
+ Hits          17241    17252   +11     
- Misses        14002    14011    +9     
- Partials       2414     2415    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

machavan
machavan previously approved these changes Jan 16, 2025
Copy link
Contributor

@machavan machavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run

  • azp run
    -ADO tests with this branch

@muskan124947
Copy link
Contributor Author

/azp run public-mssql-jdbc.linux

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@muskan124947
Copy link
Contributor Author

@muskan124947 muskan124947 marked this pull request as draft January 23, 2025 04:21
@muskan124947 muskan124947 marked this pull request as ready for review January 24, 2025 09:01

private static AppConfigurationEntry[] generateDefaultConfiguration() throws SQLServerException {
try {
if (useIbmModule) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only need to do this if isIBM(). Would it be better to just add this in Util.isIBM()?

Copy link
Contributor

@machavan machavan Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lilgreenbird The issue was reported due to a problem with Util.isIBM method implementation ( #2139)

Do you mean just adding the try catch logic in Util.isIBM method to improve it and leaving everything else unchanged?

So something like this ?

    private static Boolean isIBM = null;
    static boolean isIBM() {
        if (isIBM != null) {
            return isIBM;
        }

        String vmName = System.getProperty("java.vm.name");
        if (SYSTEM_JRE.startsWith("IBM") && vmName.startsWith("IBM")) {
            isIBM = true;
            return isIBM;
        } 

        try {
            Class.forName("com.ibm.security.auth.module.Krb5LoginModule");
            isIBM = true;
        } catch (ClassNotFoundException ex) {
            isIBM = false;
        }
        return isIBM;
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that's what I meant :)

*/
JaasConfiguration(Configuration delegate) {
JaasConfiguration(Configuration delegate) throws SQLServerException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codecov error, need a test case for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Under Peer Review
Development

Successfully merging this pull request may close these issues.

IBM Semeru Runtime Certified Edition for z/OS, Kerberos and mssql-jdbc don't work together
4 participants