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.

Sign up for GitHub

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

Jump to bottom

CountUniqueIPsecKeys function fixed. #29182

Merged
merged 2 commits into from Nov 30, 2023
Merged

CountUniqueIPsecKeys function fixed. #29182

merged 2 commits into from Nov 30, 2023

Conversation

viktor-kurchenko
Copy link
Contributor

@viktor-kurchenko viktor-kurchenko commented Nov 15, 2023 •
edited by pchaigno

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #issue-number

Handle non-AEAD IPsec keys in `cilium encrypt status`.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 15, 2023
@pchaigno pchaigno added release-note/bug This PR fixes an issue in a previous release of Cilium. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. labels Nov 15, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Nov 15, 2023
@pchaigno pchaigno linked an issue Nov 15, 2023 that may be closed by this pull request
IPSec key count for AES-CBC algorithm #29181
Closed
2 tasks
@pchaigno pchaigno added area/cli Impacts the command line interface of any command in the repository. needs-backport/1.12 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Nov 15, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.5 Nov 15, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.10 Nov 15, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.12.17 Nov 15, 2023
@viktor-kurchenko viktor-kurchenko marked this pull request as ready for review November 15, 2023 10:45
@viktor-kurchenko viktor-kurchenko requested a review from a team as a code owner November 15, 2023 10:45
@julianwiedmann julianwiedmann added the feature/ipsec Relates to Cilium's IPsec feature label Nov 15, 2023
pchaigno
pchaigno requested changes Nov 15, 2023
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice work!

pkg/common/ipsec/utils_test.go Outdated Show resolved Hide resolved
pkg/common/ipsec/utils.go Show resolved Hide resolved
pchaigno
pchaigno reviewed Nov 15, 2023
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Minor comments around error message and readability.

pkg/common/ipsec/utils.go Outdated Show resolved Hide resolved
pkg/common/ipsec/utils.go Outdated Show resolved Hide resolved
pkg/common/ipsec/utils.go Outdated Show resolved Hide resolved
chancez
chancez approved these changes Nov 15, 2023
View reviewed changes
Copy link
Contributor

@chancez chancez left a comment

Choose a reason for hiding this comment

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

I got pulled in via the metrics group but do not see any metrics changes. IPSEC isn't in my area of expertise so I'll defer to others.

@gentoo-root
Copy link
Contributor

Could you add some description of what's being fixed to the commit message? Fixes: #29181 would work best as the last line, after a proper title and a short description.

@viktor-kurchenko
Copy link
Contributor Author

Could you add some description of what's being fixed to the commit message? Fixes: #29181 would work best as the last line, after a proper title and a short description.

Done. I also squashed and renamed other commits (hope it looks better now).

Merged via the queue into main with commit 6f227fb Nov 30, 2023
210 of 213 checks passed
@pchaigno pchaigno deleted the pr/vk/ipsec/key/count/fix branch November 30, 2023 07:21
This was referenced Dec 5, 2023
Merged
Merged
@nbusseneau nbusseneau added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Dec 5, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.13 in 1.13.10 Dec 5, 2023
@nbusseneau nbusseneau mentioned this pull request Dec 5, 2023
Merged
10 tasks
@nbusseneau nbusseneau added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Dec 5, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.5 Dec 5, 2023
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.14 in 1.14.5 Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.12 in 1.12.17 Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.14 in 1.14.5 Dec 6, 2023
@github-actions github-actions bot added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.12 labels Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.12 to Backport done to v1.12 in 1.12.17 Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.12 in 1.12.17 Dec 6, 2023
@github-actions github-actions bot removed the backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. label Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.13 in 1.13.10 Dec 6, 2023
@github-actions github-actions bot added the backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. label Dec 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.12 in 1.12.17 Dec 6, 2023
This was referenced Dec 11, 2023
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@asauber asauber asauber approved these changes

@derailed derailed derailed approved these changes

@gentoo-root gentoo-root gentoo-root approved these changes

@chancez chancez chancez approved these changes

@jschwinger233 jschwinger233 jschwinger233 approved these changes

Assignees
No one assigned
Labels
area/cli Impacts the command line interface of any command in the repository. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/ipsec Relates to Cilium's IPsec feature ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.12.17
Backport done to v1.12
1.14.5
Backport done to v1.14
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IPSec key count for AES-CBC algorithm
10 participants
@viktor-kurchenko @gentoo-root @chaunceyjiang @pchaigno @derailed @chancez @asauber @jschwinger233 @nbusseneau @julianwiedmann