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

[storage][backend] Add psk field to NvmeRemoteController #308

Merged
merged 2 commits into from
Jul 12, 2023
Merged

[storage][backend] Add psk field to NvmeRemoteController #308

merged 2 commits into from
Jul 12, 2023

Conversation

glimchb
Copy link
Member

@glimchb glimchb commented Jul 6, 2023

this will enable Nvme/TLS secure connection to the target

Signed-off-by: Boris Glimcher Boris.Glimcher@emc.com

@glimchb
[storage][backend] Add psk field to NvmeRemoteController
0391b48 
this will enable Nvme/TLS secure connection to the target

using bytes and not string per https://linter.aip.dev/140/base64

Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb
[storage] auto-generate protos
1af77ff 
Signed-off-by: Boris Glimcher <Boris.Glimcher@emc.com>
@glimchb glimchb marked this pull request as ready for review July 6, 2023 16:02
@glimchb glimchb requested a review from a team as a code owner July 6, 2023 16:02
@glimchb glimchb added the Merge Candidate in the open merge window, next candidate for merge label Jul 6, 2023
@glimchb glimchb mentioned this pull request Jul 6, 2023
Merged
@glimchb glimchb requested a review from a team July 6, 2023 21:33
jainvipin
jainvipin approved these changes Jul 10, 2023
View reviewed changes
Copy link
Contributor

@jainvipin jainvipin left a comment

Choose a reason for hiding this comment

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

LGTM
While it can be argued that pre shared key for TLS need to be path specific, it doesn't makes sense to have different keys for different backend targets. And therefore keeping this in controller object looks good to me.

artek-koltun
artek-koltun approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@artek-koltun artek-koltun left a comment

Choose a reason for hiding this comment

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

looks ok for me

@glimchb glimchb merged commit 3ccebf8 into opiproject:main Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artek-koltun artek-koltun artek-koltun approved these changes

@jainvipin jainvipin jainvipin approved these changes

Assignees
No one assigned
Labels
Merge Candidate in the open merge window, next candidate for merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@glimchb @jainvipin @artek-koltun