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

#675 - Node to Node encryption #655

Merged
merged 8 commits into from
Oct 22, 2024
Merged

#675 - Node to Node encryption #655

merged 8 commits into from
Oct 22, 2024

Conversation

fdobrovolny
Copy link
Contributor

@fdobrovolny fdobrovolny commented Jul 8, 2024 •
edited by matofeder
Loading

@fdobrovolny fdobrovolny mentioned this pull request Jul 8, 2024
Open
9 tasks
@OgarOgarovic OgarOgarovic marked this pull request as ready for review August 27, 2024 07:42
@OgarOgarovic OgarOgarovic force-pushed the proposal/675-node-to-node-encryption branch 2 times, most recently from de7ce2f to cbd3fdd Compare August 28, 2024 09:05
@OgarOgarovic OgarOgarovic changed the title WIP: #675 - Node to Node encryption #675 - Node to Node encryption Sep 2, 2024
@matofeder matofeder self-requested a review September 3, 2024 08:25
artificial-intelligence
artificial-intelligence reviewed Sep 3, 2024
View reviewed changes
Copy link
Contributor

@artificial-intelligence artificial-intelligence left a comment

Choose a reason for hiding this comment

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

generally LGTM, but I want to reread it a second time, as it's quite a long document, at the very least there are some spelling mistakes lurking in there imho, but I didn't want to comment on minor errors before actually having read the whole thing first.

Will provide more feedback, hopefully tomorrow.
Thanks for working on this!

matofeder
matofeder approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member

@matofeder matofeder left a comment

Choose a reason for hiding this comment

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

Overall it is an interesting reading and it looks good from my perspective.

I left just some minor comments there.

Drafts/node-to-node-encryption.md Outdated Show resolved Hide resolved
Drafts/node-to-node-encryption.md Outdated Show resolved Hide resolved
Drafts/node-to-node-encryption.md Outdated Show resolved Hide resolved
Drafts/node-to-node-encryption.md Outdated Show resolved Hide resolved
Drafts/node-to-node-encryption.md Outdated Show resolved Hide resolved
Drafts/node-to-node-encryption.md Show resolved Hide resolved
@OgarOgarovic OgarOgarovic mentioned this pull request Sep 9, 2024
Closed
9 tasks
@OgarOgarovic OgarOgarovic mentioned this pull request Sep 24, 2024
Open
@matofeder
Copy link
Member

@OgarOgarovic check the failed pipelines

@OgarOgarovic OgarOgarovic force-pushed the proposal/675-node-to-node-encryption branch from 8593ef0 to b205def Compare September 25, 2024 13:17
OgarOgarovic added a commit to OgarOgarovic/kolla that referenced this pull request Sep 28, 2024
@OgarOgarovic
Add openvswitch-ipsec service image
c260b5e 
Add image for a new openvswitch-ipsec service for transparent IPsec
encryption of node to node traffic when using OVN neutron agent. Image
uses s6-overlay [1] for process supervision as it runs two long run
processes - ipsec monitor script and an IKE daemon.

There is a document [2] on review downstream providing more context.

[1] https://github.com/just-containers/s6-overlay
[2] SovereignCloudStack/standards#655

Change-Id: I7afe95856f35b35c6b6c26707a684266f7f98a30
Signed-off-by: Ivan Vnučko <[email protected]>
OgarOgarovic added a commit to OgarOgarovic/kolla-ansible that referenced this pull request Sep 29, 2024
@OgarOgarovic
Add openvswitch-ipsec service
3df471e 
Adds a role to deploy an openvswich-ipsec service container for
IPsec encryption of tenant network traffic. There is a document
downstream [1] providing more context.

This role depends on a new kolla openvswitch-ipsec image. It
needs OVN Neutron plugin agent set up and to enable OVN IPsec
with certificate generation:
`enable_ovn_ipsec: true`
`neutron_ovs_generate_certificates: true`

[1] - SovereignCloudStack/standards#655

Depends-on: I7afe95856f35b35c6b6c26707a684266f7f98a30
Change-Id: Icc951578906e387746971e8e7df3a38a57fa4735
Signed-off-by: Ivan Vnučko <[email protected]>
OgarOgarovic added a commit to OgarOgarovic/kolla-ansible that referenced this pull request Sep 29, 2024
@OgarOgarovic
Add openvswitch-ipsec service
ea2464d 
Adds a role to deploy an openvswich-ipsec service container for
IPsec encryption of tenant network traffic. There is a document
downstream [1] providing more context.

This role depends on a new kolla openvswitch-ipsec image. It
needs OVN Neutron plugin agent set up and to enable OVN IPsec
with certificate generation:
`enable_ovn_ipsec: true`
`neutron_ovs_generate_certificates: true`

[1] - SovereignCloudStack/standards#655

Depends-on: I7afe95856f35b35c6b6c26707a684266f7f98a30
Change-Id: Icc951578906e387746971e8e7df3a38a57fa4735
Signed-off-by: Ivan Vnučko <[email protected]>
@matofeder
Copy link
Member

generally LGTM, but I want to reread it a second time, as it's quite a long document, at the very least there are some spelling mistakes lurking in there imho, but I didn't want to comment on minor errors before actually having read the whole thing first.

Will provide more feedback, hopefully tomorrow. Thanks for working on this!

@artificial-intelligence would you like to provide more feedback on this?

@OgarOgarovic OgarOgarovic force-pushed the proposal/675-node-to-node-encryption branch from 2f4c1ef to ce61933 Compare October 9, 2024 13:40
@matofeder
Copy link
Member

@mbuechse @artificial-intelligence can we merge this one or do you have some further comments/discussion points?

@matofeder matofeder merged commit ced5954 into main Oct 22, 2024
10 checks passed
@matofeder matofeder deleted the proposal/675-node-to-node-encryption branch October 22, 2024 08:39
@mbuechse
Copy link
Contributor

mbuechse commented Nov 5, 2024

What is the reason for putting it into the Drafts folder? Why was it merged in "Proposal" state? If it's a decision record that has been accepted by the corresponding team (I guess Team IaaS in this case), then the status field should be Draft, the file should be renamed to, say, scs-0122-v1-node-to-node-encryption.md and be put under Standards.

@matofeder
Copy link
Member

What is the reason for putting it into the Drafts folder? Why was it merged in "Proposal" state? If it's a decision record that has been accepted by the corresponding team (I guess Team IaaS in this case), then the status field should be Draft, the file should be renamed to, say, scs-0122-v1-node-to-node-encryption.md and be put under Standards.

@mbuechse It makes sense. We will rename it and put it under the Standards. Before that, we will ask team IaaS for final validation. Thanks for bringing this up!

@OgarOgarovic
Copy link
Contributor

PR here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artificial-intelligence artificial-intelligence artificial-intelligence left review comments

@matofeder matofeder matofeder approved these changes

@bitkeks bitkeks Awaiting requested review from bitkeks

@mbuechse mbuechse Awaiting requested review from mbuechse

Assignees

@OgarOgarovic OgarOgarovic

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fdobrovolny @matofeder @mbuechse @OgarOgarovic @artificial-intelligence