Skip to content

Feature: Configuring rest components for TLS #927

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
Merged
bors-openebs-mayastor merged 1 commit into from
Feb 19, 2025
Merged

Feature: Configuring rest components for TLS #927

bors-openebs-mayastor merged 1 commit into from
Feb 19, 2025

Conversation

@Johnaius
Copy link

@Johnaius Johnaius commented Feb 5, 2025 •
edited
Loading

This PR configures components using rest protocols to speak tls. It is dependent on this PR in extensions repo.

Rest-api Clients

  • Modified certificate loading to use pkcs8_private_keys instead of rsa_private_keys. This was a change I made to get the generated certificates to work, they were apparently created in pkcs8 format, not rsa - I attempted to convert them with no luck, and found the pkcs8_private_keys in the rustl-pemfile crate...

Csi-controller and Diskpool operator:

  • Added support for TLS configuration based on CA certificate path.
  • if certs are provided, use https, if not use http.
  • error handling for HTTPS connections without a certificate etc

@Johnaius Johnaius mentioned this pull request Feb 5, 2025
Open
6 tasks
@Johnaius Johnaius mentioned this pull request Feb 6, 2025
Closed
tiagolobocastro
tiagolobocastro reviewed Feb 7, 2025
View reviewed changes
Copy link
Member

@tiagolobocastro tiagolobocastro left a comment

Choose a reason for hiding this comment

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

What about NoCertificateVerification as well?

Is that still required? Only for self-signed IIRC?

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 7, 2025

What about NoCertificateVerification as well?

Is that still required? Only for self-signed IIRC?

Ah no, seems that's when not specifying the certificate on the client side

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 7, 2025

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Feb 7, 2025
Try #927:
9698067
@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 7, 2025

try

Build failed:

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 7, 2025

bors delegate

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 7, 2025

bors d+

@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 7, 2025

✌️ Johnaius can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 7, 2025

You can now runs bors try @Johnaius

@Johnaius
Copy link
Author

Johnaius commented Feb 8, 2025

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Feb 8, 2025
Try #927:
8a17525
@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 8, 2025

try

Build failed:

@Johnaius Johnaius requested review from a team as code owners February 13, 2025 15:35
@Johnaius
Copy link
Author

Johnaius commented Feb 13, 2025

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Feb 13, 2025
Try #927:
a13fef6
@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 13, 2025

try

Build succeeded:

tiagolobocastro
tiagolobocastro reviewed Feb 13, 2025
View reviewed changes
Copy link
Member

@tiagolobocastro tiagolobocastro left a comment

Choose a reason for hiding this comment

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

Bors issues a merge with all commits, so could you please either remove debugging and code review commits, or even simply squash them all as as a single commit

@Johnaius
Copy link
Author

Johnaius commented Feb 13, 2025

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Feb 13, 2025
Try #927:
82bb124
@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 13, 2025

try

Build succeeded:

@Johnaius
Copy link
Author

Johnaius commented Feb 15, 2025

Added csi-node configurations

@Johnaius
Copy link
Author

Johnaius commented Feb 15, 2025

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Feb 15, 2025
Try #927:
30fa332
@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 15, 2025

try

Build succeeded:

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 17, 2025

Please go ahead and issue bors merge when you're ready @Johnaius
Thank you!

@Johnaius
Copy link
Author

Johnaius commented Feb 18, 2025

bors merge

@Johnaius
Copy link
Author

Johnaius commented Feb 18, 2025

bors merge

image

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 18, 2025

odd, this should work, any clues @niladrih ?
for now, I can issue it:
bors merge

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

Seems like it did not trigger a merge @tiagolobocastro. I will try once.

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

bors merge

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

bors ping

@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 19, 2025

pong

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

bors cancel

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

bors try-

@Abhinandan-Purkait Abhinandan-Purkait requested review from a team February 19, 2025 05:33
@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

bors merge

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Feb 19, 2025

How weird, it does work on the other PR but not here. @Johnaius Can you please rebase your PR once?

@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 19, 2025

bors cancel

@tiagolobocastro
feat: configure rest components for tls
832ea4e 
Signed-off-by: John Zakrzewski <[email protected]>

configuring diskpool operator to speak tls

Signed-off-by: John Zakrzewski <[email protected]>

debugging rest api and cert configuration

Signed-off-by: John Zakrzewski <[email protected]>

tidying up

Signed-off-by: John Zakrzewski <[email protected]>

resolving comments

Signed-off-by: John Zakrzewski <[email protected]>

fix: running linter

Signed-off-by: John Zakrzewski <[email protected]>

fix: address failed tests, rely on cert-manager for certs

Signed-off-by: John Zakrzewski <[email protected]>

fix: addressing comments

Signed-off-by: John Zakrzewski <[email protected]>
@tiagolobocastro
Copy link
Member

tiagolobocastro commented Feb 19, 2025

bors merge

@bors-openebs-mayastor
Copy link

bors-openebs-mayastor bot commented Feb 19, 2025

Build succeeded:

@bors-openebs-mayastor bors-openebs-mayastor bot merged commit 12875e6 into openebs:develop Feb 19, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiagolobocastro tiagolobocastro tiagolobocastro approved these changes

@Abhinandan-Purkait Abhinandan-Purkait Abhinandan-Purkait approved these changes

+1 more reviewer

@dsharma-dc dsharma-dc dsharma-dc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Johnaius @tiagolobocastro @Abhinandan-Purkait @dsharma-dc