Skip to content

#675 - Node to Node encryption#655

Merged
matofeder merged 8 commits intomainfrom
proposal/675-node-to-node-encryption
Oct 22, 2024
Merged

#675 - Node to Node encryption#655
matofeder merged 8 commits intomainfrom
proposal/675-node-to-node-encryption

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.

@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

matofeder commented Sep 25, 2024

@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 <ivan@vnucko.com>
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 <ivan@vnucko.com>
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 <ivan@vnucko.com>
@matofeder
Copy link
Member

matofeder commented Oct 1, 2024

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?

@matofeder matofeder requested a review from mbuechse October 8, 2024 09:02
fdobrovolny and others added 5 commits October 9, 2024 14:58
@fdobrovolny @OgarOgarovic
WIP: #675 - Node to Node encryption
271c85d 
SovereignCloudStack/issues#675
Signed-off-by: Filip Dobrovolny <dobrovolny.filip@gmail.com>
@OgarOgarovic
First reviewable version
3a32752 
Signed-off-by: Ivan Vnučko <ivan@vnucko.com>
@OgarOgarovic
Corrections and updates
719ac58 
Signed-off-by: Ivan Vnučko <ivan@vnucko.com>
@OgarOgarovic
Updates responding to first review
1fecdc0 
Signed-off-by: Ivan Vnučko <ivan@vnucko.com>
@OgarOgarovic
Small fixes for the pipelines
ce61933 
Signed-off-by: Ivan Vnučko <ivan@vnucko.com>
@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

matofeder commented Oct 10, 2024

@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
@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

matofeder 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.

@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

OgarOgarovic commented Nov 5, 2024

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 mbuechse is a code owner

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