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

security: Update README.md #102

Merged
merged 2 commits into from
Aug 29, 2022
Merged

Conversation

mestery
Copy link
Contributor

@mestery mestery commented Aug 29, 2022

This adds more information into the security/README.md file detailing
what exactly the OPI Security APIs are currently targeting, and the
proposed architecture around the IPsec portion of the APIs.

Signed-off-by: Kyle Mestery mestery@mestery.com

@mestery mestery added the security APIs or code related to security area (e.g. ipsec) label Aug 29, 2022
@mestery mestery requested review from glimchb and sandersms August 29, 2022 13:48
@mestery mestery self-assigned this Aug 29, 2022
Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

the change looks great, just a few questions before we merge it:

  • copy-paste of image from POC repo ?
  • can we remove security-spec.md and left with readme only ?

This adds more information into the `security/README.md` file detailing
what exactly the OPI Security APIs are currently targeting, and the
proposed architecture around the IPsec portion of the APIs.

I also collapsed the security-spec.md file into the README.md file.

Signed-off-by: Kyle Mestery <mestery@mestery.com>
Signed-off-by: Kyle Mestery <mestery@mestery.com>
Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

this is great now,
thanks for fixing my small nits

@mestery mestery merged commit ed9bf75 into opiproject:main Aug 29, 2022
@mestery mestery deleted the security-readme branch August 29, 2022 14:43
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I'm reserving endorsement of the approach proposed here.
But so far as this text documenting the approach goes, I'm happy.
Thanks for addressing my earlier review.

@glimchb
Copy link
Member

glimchb commented Aug 29, 2022

@shorman-corigine we are welcoming the reviews and other opinions... you can comment freely

@ghost
Copy link

ghost commented Aug 29, 2022

@shorman-corigine we are welcoming the reviews and other opinions... you can comment freely

Of course :)
But I need to put my thoughts together first.

@glimchb
Copy link
Member

glimchb commented Aug 29, 2022

I do think we need to separate reference implementation VS generic APIs...
vici on this page is just an example...
everybody else are free to use their own implementation instead of strongswan or vici...
see the picture from https://github.com/opiproject/opi-poc/blob/main/storage/OPI-storage-SPDK-bridge.png

also storage in this repo doesn't talk about specific implementations storage readme

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security APIs or code related to security area (e.g. ipsec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants