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 both uint64 and int64 to tlscommon types unpack methods #198

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

michel-laterman
Copy link
Contributor

What does this PR do?

Add int64 support to unpack methods which only supported uint64

Why is it important?

We see the error message

tls renegotation support is an unknown type: int64 accessing 'output.elasticsearch.ssl.renegotiation'

in our snapshots.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

Related issues

@michel-laterman michel-laterman added bug Something isn't working Team:Elastic-Agent Label for the Agent team labels Apr 16, 2024
@michel-laterman michel-laterman requested a review from a team as a code owner April 16, 2024 15:13
@michel-laterman michel-laterman requested review from fearful-symmetry and leehinman and removed request for a team April 16, 2024 15:13
@cmacknz
Copy link
Member

cmacknz commented Apr 16, 2024

Needs tests

@cmacknz
Copy link
Member

cmacknz commented Apr 16, 2024

(no tests is why we didn't catch this initially)

@michel-laterman
Copy link
Contributor Author

michel-laterman commented Apr 16, 2024

Not sure how the fleet-server is reading/unserializing the policies from es; the TestRepack* tests added previously didn't trigger this

@michel-laterman michel-laterman force-pushed the tlscommon-int64-unpack branch from 70c8f09 to 84e5935 Compare April 16, 2024 16:13
@@ -173,3 +173,80 @@ func Test_ServerConfig_Repack(t *testing.T) {
})
}
}

func Test_ServerConfig_RepackJSON(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if we encounter json serialization in the error we observed but i've added tests in case that is the issue.
I've also added Unpack unit tests to all the types

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@michel-laterman michel-laterman merged commit 6ea4f03 into elastic:main Apr 16, 2024
6 checks passed
@michel-laterman michel-laterman deleted the tlscommon-int64-unpack branch April 16, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants