Fix Sonar Violations

[brianalbrecht]

When I run this through static analysis using SonarQube with the default Java ruleset, it returns 6 critical and 115 major violations. Sonar violations will be visible to anyone who includes the SDK. We need to have all Critical and Major Sonar violations remediated. I'd like to see a clean run through Sonar before we release.

Can you please pull down SonarQube v5.1, scan the project, and fix any violations it finds? If you have trouble getting SonarQube setup, I can deploy a publicly available instance for you.

[idrabenia]

Depends on https://github.com/CenturyLinkCloud/clc-java-sdk/issues/93

[idrabenia]

Hi Guys @jruckle @brianalbrecht !

We configured SonarQube here

Unfortunately many of issues is not relevant. For instance, do we really need to add logging for tests if we do not require its. And so on. I think relevant issues maybe only 10-15% of all.

Other example of critical issue

static final String PASSWORD_KEY = "clc.password";

Remove this hard-coded password

It's not a password...

[idrabenia]

I think it would be great to assign somebody to go through all issues and fix it if relevant. Are you agreed?

My estimate - 16h.

[brianalbrecht]

Excellent. Thanks for setting that up.

Yes, some sonar issues aren't relevant. However, many of them are and they're also very visible to anyone who scans the project. The password key you noted also trips our static security scanner. We probably need to find a different name for that variable.

I agree with your proposal. I'd like to close as many of them as we can, to get the number of violations down to a manageable number that are explainable or obvious.

[idrabenia]

Yep, this is our SonarQube instance And your credentials will be clc_technical_specialist / qwerty123

[idrabenia]

@jruckle @brianalbrecht After Sonar updates installation about 50 issues was closed without any actions.

I believe that using such tool is wrong way to provide feedback to development. It great to use as recommendation systems but drive development using such formal and not relevant issues is a wrong way.

_We probably need to find a different name for that variable._ Not a problem we will rename it. But I principally disagree with this decision. I believe - any tool must help people to find issue but not manage people and drive development. I think development must be driven by real needs of business and creative ideas. It's really good approach that I use when take part in Apache Software Foundation projects.

[idrabenia]

Hi @jruckle ! Yep, we are start working on Sonar issues today. I closed multiple tickets byself, for some long issues - I write github tickets. For all rejected issues I wrote comments - why it's wrong and disable rules that produce such tickets.

[idrabenia]

@push1st1k Could you fix rest critical and major issues that we have.

Thanks!

[idrabenia]

Could you see some not closed issues at http://66.155.4.208:9000

Thanks!

[idrabenia]

Hi guys @jruckle @brianalbrecht ,

We fixed all critical and major issues on Sonar.

Could you review latest metrics for our project and provide your feedback - http://66.155.4.208:9000/ clc_technical_specialist / qwerty123

Thanks!

[brianalbrecht]

Thanks, @idrabenia . I'll take a look.

[brianalbrecht]

Hi @idrabenia. I've gone through the notations you've made on the Sonar rules, and run it through my Sonar as well. Overall, the project looks much better. However, there are a couple violations that you closed, that I'd like you to go ahead and fix:

1. There was a critical violation in PropertiesFileCredentialsProvider.java because PASSWORD_KEY was tripping the rule about hardcoding passwords. I'd like to find a different name for this variable so that it doesn't trip the rule. If you rename it to something that doesn't match the regex "password" it should close the issue. E.g: "PASS_KEY" or "PASSWD_KEY". I know this seems minor, but it's one less thing we have to explain if a customer scans it.
2. We really need to use yoda notation for string comparisons when checking against string literals. Please move the string literals to the left side of the comparison. It’s a best practice in Java to avoid Null Pointer Exceptions. This happens in a number of files: GroupMetadata.java L393; BaseServerResponse.java L70, 81; ServerFilter.java L262;PauseServerJobFuture.java L47; SuperCommandSampleApp.java L166.

Once these two things are fixed, we can probably close this issue out.

[idrabenia]

@jruckle Agreed, not a problem :)

[idrabenia]

@jruckle It's completed

[brianalbrecht]

Yeah, I'll take a look

[brianalbrecht]

Thanks, @idrabenia. @jruckle : I've verified that the changes were made.