Intermittent failures in TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey

SaschaSchwarze0 commented 6 months ago

Report

We are compiling our own version of Keda and see errors when running make test that started to happen with Go 1.22.

The test case that fails is TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey.

Our analysis so far is the following:

the test case itself passes in the same secret twice: https://github.com/kedacore/keda/blob/v2.13.1/pkg/scaling/resolver/hashicorpvault_handler_test.go#L421-L433
the mock server however would return always a different value no matter if it is asked for the same secret twice: https://github.com/kedacore/keda/blob/v2.13.1/pkg/scaling/resolver/hashicorpvault_handler_test.go#L134-L152
the ResolveSecrets function here relies on building a map where the key is a struct containing strings and a pointer to a type which also contains strings: https://github.com/kedacore/keda/blob/v2.13.1/pkg/scaling/resolver/hashicorpvault_handler.go#L316-L322
The test case can only succeed if both secrets are put into one slice under one map key = that the map only contains one entry
We added the following code after the groups are created: fmt.Fprintf(os.Stderr, "len: %d, grouped: %v\n", len(grouped), grouped)
When the test case fails, we see this output:

=== RUN   TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey
len: 2, grouped: map[{pki/issue/default pki 0xc0004f4280}:[{certificate pki/issue/default certificate pki {test      } }] {pki/issue/default pki 0xc0004f4400}:[{certificate pki/issue/default certificate pki {test      } }]]
    hashicorpvault_handler_test.go:436:
                Error Trace:    .../keda/pkg/scaling/resolver/hashicorpvault_handler_test.go:436
                Error:          Not equal:
                                expected: "-----BEGIN CERTIFICATE-----\nmzx6wnt8rgZwre5gMXubAI3JJXLjaWqlPNu7-ZGymwZ0JTTSf4_YxFc0MpQ-i5k-CQ\n-----END CERTIFICATE-----"
                                actual  : "-----BEGIN CERTIFICATE-----\nBkZA-zHp12bzMIFl-rvnh3lVO5wS7MN_PQrpM_dMpPGBptY9iOYh5qcjS1_2Npm_zQ\n-----END CERTIFICATE-----"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,3 +1,3 @@
                                 -----BEGIN CERTIFICATE-----
                                -mzx6wnt8rgZwre5gMXubAI3JJXLjaWqlPNu7-ZGymwZ0JTTSf4_YxFc0MpQ-i5k-CQ
                                +BkZA-zHp12bzMIFl-rvnh3lVO5wS7MN_PQrpM_dMpPGBptY9iOYh5qcjS1_2Npm_zQ
                                 -----END CERTIFICATE-----
                Test:           TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey
                Messages:       Refetching same path should yield same value

We are wondering whether a struct with a "fresh" pointer to structs that contain the same values but are different structs, are really equal, or if maybe the new Go 1.22 behavior on for-range loops is hitting here.

Expected Behavior

make test to pass

Actual Behavior

It fails for us on Go 1.22

Steps to Reproduce the Problem

Use Go 1.22
run make test

Logs from KEDA operator

N/A

KEDA Version

2.13.1

Kubernetes Version

None

Platform

None

Scaler Details

No response

Anything else?

No response

stale[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

HeavyWombat commented 4 months ago

Would like to hear some feedback on the report. Did someone have a chance to look into it? Maybe @loispostula, could you have a glance? Thanks in advance.

stale[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

HeavyWombat commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

I would still be interested in some feedback regarding the original finding/question.

zroubalik commented 1 month ago

Interesting finding, I think that we should treat equal values as equal. Are you willing to contribute a fix? Thanks!

kedacore / keda