fix: fix pieces of code currently omitted by the linter

[JasonPowr]

Description

This PR is to Investigate and fix pieces of code currently omitted by the linter, i.e lines that have //nolint
Jira: https://issues.redhat.com/browse/MGDSTRM-900

Checklist (Definition of Done)

• All acceptance criteria specified in JIRA have been completed
• Unit and integration tests added that prove the fix is effective or the feature works (tested against emulated and non-emulated OCM environment)
• Documentation added for the feature
• CI and all relevant tests are passing
• Code Review completed
• Verified independently by reviewer
• All PR comments are resolved either by addressing them or creating follow up tasks
• Required metrics/dashboards/alerts have been added (or PR created).
• Required Standard Operating Procedure (SOP) is added.
• JIRA has been created for changes required on the client side

[JasonPowr]

Most of the //nolint issues are fixed in this PR however there are three that are not.

1. https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/internal/kafka/test/common/kafka_pollers.go#L67 I’ve looked into this a bit and I believe I am closing the resps body correctly but I am still running into issues with the linter stating that it must be closed.

2. https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/auth/helper.go#L158 ParseRSAPrivateKeyFromPEMWithPassword() Is Deprecated however the linter is returning this error when it runs:
   jwt.ParseRSAPrivateKeyFromPEMWithPassword is deprecated: This function is deprecated and should not be used anymore. It uses the deprecated x509.DecryptPEMBlock function, which was deprecated since RFC 1423 is regarded insecure by design. Unfortunately, there is no alternative in the Go standard library for now. See https://github.com/golang/go/issues/8860
   So I'm not sure if there is another way around this one.

3. https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L28 Wasn't sure how to fix this one.

[machi1990]

Thanks @JasonPowr fantastic work.

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/internal/kafka/test/common/kafka_pollers.go#L67 I’ve looked into this a bit and I believe I am closing the resps body correctly but I am still running into issues with the linter stating that it must be closed.

I think this is by design and it's up to the caller to Close the response. @ziccardi is this correct?

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/auth/helper.go#L158 ParseRSAPrivateKeyFromPEMWithPassword() Is Deprecated however the linter is returning this error when it runs:
jwt.ParseRSAPrivateKeyFromPEMWithPassword is deprecated: This function is deprecated and should not be used anymore. It uses the deprecated x509.DecryptPEMBlock function, which was deprecated since RFC 1423 is regarded insecure by design. Unfortunately, there is no alternative in the Go standard library for now. See golang/go#8860
So I'm not sure if there is another way around this one.

This one we need to wait for a drop in replacement implementation to be available or use external libs.
Since it is only used for unit testing, I think we can continue to ignore the deprecated method for linting until we've a replacement.

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L28 Wasn't sure how to fix this one.

For this one, the issue is the string "txid" which is used as key. We need to define a type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L12, the declare the "txid" as of that type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L15 and use it in place.

[ziccardi]

Thanks @JasonPowr fantastic work.

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/internal/kafka/test/common/kafka_pollers.go#L67 I’ve looked into this a bit and I believe I am closing the resps body correctly but I am still running into issues with the linter stating that it must be closed.

I think this is by design and it's up to the caller to Close the response. @ziccardi is this correct?

@machi1990 Correct.

[pawelpaszki]

lgtm

[JasonPowr]

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L28 Wasn't sure how to fix this one.

For this one, the issue is the string "txid" which is used as key. We need to define a type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L12, the declare the "txid" as of that type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L15 and use it in place.

So I tried to fix this like you said but it still does not seem to work. It fixes the linter Issue all right but seems to break everything else.
@machi1990

[codecov]

Codecov Report

Merging #1227 (de3c8de) into main (e9d79d6) will increase coverage by 0.79%.
The diff coverage is 12.50%.

@@            Coverage Diff             @@
##             main    #1227      +/-   ##
==========================================
+ Coverage   82.47%   83.27%   +0.79%     
==========================================
  Files         133      133              
  Lines       11813    11815       +2     
==========================================
+ Hits         9743     9839      +96     
+ Misses       1714     1622      -92     
+ Partials      356      354       -2     

Flag	Coverage Δ
unittests	83.27% <12.50%> (+0.79%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/db/context.go	0.00% <0.00%> (ø)
pkg/api/cluster_types.go	76.66% <100.00%> (-0.20%)	⬇️
internal/kafka/internal/config/kafka.go	53.12% <0.00%> (-0.85%)	⬇️
internal/kafka/internal/config/kafka_quota.go	100.00% <0.00%> (ø)
internal/kafka/internal/services/kafka.go	77.65% <0.00%> (+0.72%)	⬆️
pkg/workers/reconciler_config.go	100.00% <0.00%> (+50.00%)	⬆️
pkg/workers/leader_election_mgr.go	84.00% <0.00%> (+51.20%)	⬆️
pkg/workers/worker_interface.go	100.00% <0.00%> (+100.00%)	⬆️

[machi1990]

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L28 Wasn't sure how to fix this one.

For this one, the issue is the string "txid" which is used as key. We need to define a type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L12, the declare the "txid" as of that type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L15 and use it in place.

So I tried to fix this like you said but it still does not seem to work. It fixes the linter Issue all right but seems to break everything else. @machi1990

@JasonPowr I think we need to change all the other references of txid. There is also a cyclic dependency on the logger.go so we'll need break this cyclic dependency. I can push a follow PR to this or if you are okay I can push a separate commit.

[JasonPowr]

https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L28 Wasn't sure how to fix this one.

For this one, the issue is the string "txid" which is used as key. We need to define a type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L12, the declare the "txid" as of that type just like https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/main/pkg/db/context.go#L15 and use it in place.

So I tried to fix this like you said but it still does not seem to work. It fixes the linter Issue all right but seems to break everything else. @machi1990

@JasonPowr I think we need to change all the other references of txid. There is also a cyclic dependency on the logger.go so we'll need break this cyclic dependency. I can push a follow PR to this or if you are okay I can push a separate commit.

Sure yeah work away thanks for the help :)

[machi1990]

Pushed a commit on top of you change.

Let's see what CI says @JasonPowr , it should be happy :-)

[machi1990]

This was fixed by de3c8de which introduced the https://github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/blob/de3c8de28fba0d5031cfc2e1cf8d701bf3df4ccd/pkg/constants/db_context_keys.go to also fix the cyclic dependency issue