Skip to content

Make InitPacket and InitSecurityKeys public #447

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

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Make InitPacket and InitSecurityKeys public #447

merged 1 commit into from
Sep 20, 2023

Conversation

upsampled
Copy link
Contributor

@upsampled upsampled commented Sep 8, 2023

This PR will allow GoSNMPServer to use mainline gosnmp/gosnmp again (along with some minimal changes to it).

Closes: #446

@TimRots
Copy link
Member

TimRots commented Sep 14, 2023

Instead of duplicating some existing logic, what are your thoughts on making the InitPacket() and usmAllocateNewSalt() methods public? It would be helpful to add clear documentation explaining their intent and possibly include some test cases.

@upsampled
Copy link
Contributor Author

@TimRots I think InitPacket can replace GenSalt without needing usmAllocateNewSalt to be publicl; however, if we did that I would recommend making genlocalkey public so gosnmp would handle the Key generation to replace GenKey.

I'll modify this PR to reflect this in a few days.

Copy link
Member

@TimRots TimRots left a comment

Choose a reason for hiding this comment

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

Thank you for the updated PR @upsampled, this looks great to me.

If we can get consensus I think some comments would be needed to describe what InitPacket and InitSecurityKeys are doing?

@SuperQ what do you think ?

@TimRots TimRots requested a review from SuperQ September 19, 2023 19:47
Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM.

Would you mind squashing the commits to make this a more descriptive change?

@SuperQ SuperQ changed the title Compatibility with GoSNMPServer Make InitPacket and InitSecurityKeys public Sep 20, 2023
@upsampled
Copy link
Contributor Author

upsampled commented Sep 20, 2023

@TimRots updated the documentation

@SuperQ squashed it to one commit

(typo in a comment, re-pushing)

this allows external packages, like the GoSNMPServer,  to use gosnmp to handle encrypted sessions.

Signed-off-by: Liam Kelly <liam@commdevices.com>
@SuperQ SuperQ requested a review from TimRots September 20, 2023 13:22
Copy link
Member

@TimRots TimRots left a comment

Choose a reason for hiding this comment

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

Looks great to me. Thank you for contributing.

@SuperQ SuperQ merged commit 9457f61 into gosnmp:master Sep 20, 2023
@upsampled upsampled deleted the upsampled-GoSNMPServer-Compat branch September 20, 2023 22:59
kajtzu added a commit to basen/gosnmp that referenced this pull request Sep 28, 2023
codeboten referenced this pull request in open-telemetry/opentelemetry-collector-contrib Nov 24, 2023
[![Mend Renovate logo
banner](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/gosnmp/gosnmp](https://togithub.com/gosnmp/gosnmp) |
require | minor | `v1.36.1` -> `v1.37.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>gosnmp/gosnmp (github.com/gosnmp/gosnmp)</summary>

### [`v1.37.0`](https://togithub.com/gosnmp/gosnmp/releases/tag/v1.37.0)

[Compare
Source](https://togithub.com/gosnmp/gosnmp/compare/v1.36.1...v1.37.0)

#### What's Changed

- Make InitPacket and InitSecurityKeys public by
[@&#8203;upsampled](https://togithub.com/upsampled) in
[https://github.com/gosnmp/gosnmp/pull/447](https://togithub.com/gosnmp/gosnmp/pull/447)
- Refactor TrapListener's Close Method by
[@&#8203;TimRots](https://togithub.com/TimRots) in
[https://github.com/gosnmp/gosnmp/pull/449](https://togithub.com/gosnmp/gosnmp/pull/449)
- Allow RequestID to be shrunk if possible by
[@&#8203;upsampled](https://togithub.com/upsampled) in
[https://github.com/gosnmp/gosnmp/pull/451](https://togithub.com/gosnmp/gosnmp/pull/451)
- Add net-snmp validation testing by
[@&#8203;upsampled](https://togithub.com/upsampled) in
[https://github.com/gosnmp/gosnmp/pull/452](https://togithub.com/gosnmp/gosnmp/pull/452)
- Add fuzzing to CI by [@&#8203;SuperQ](https://togithub.com/SuperQ) in
[https://github.com/gosnmp/gosnmp/pull/444](https://togithub.com/gosnmp/gosnmp/pull/444)
- Allow global password cache to be turned off by
[@&#8203;upsampled](https://togithub.com/upsampled) in
[https://github.com/gosnmp/gosnmp/pull/454](https://togithub.com/gosnmp/gosnmp/pull/454)
- build(deps): bump github.com/google/go-cmp from 0.5.9 to 0.6.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/gosnmp/gosnmp/pull/455](https://togithub.com/gosnmp/gosnmp/pull/455)

#### New Contributors

- [@&#8203;upsampled](https://togithub.com/upsampled) made their first
contribution in
[https://github.com/gosnmp/gosnmp/pull/447](https://togithub.com/gosnmp/gosnmp/pull/447)

**Full Changelog**:
gosnmp/gosnmp@v1.36.1...v1.37.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy41OS44IiwidXBkYXRlZEluVmVyIjoiMzcuNTkuOCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility with GoSNMPServer
3 participants