Skip to content

ROX-28296: add OpenShift reencrypt route#14416

Merged
stehessel merged 13 commits intomasterfrom
hack/ocp-reencrypt-route
Apr 1, 2025
Merged

ROX-28296: add OpenShift reencrypt route#14416
stehessel merged 13 commits intomasterfrom
hack/ocp-reencrypt-route

Conversation

@stehessel
Copy link
Copy Markdown
Collaborator

@stehessel stehessel commented Feb 26, 2025 •
edited
Loading

Description

Add a Helm flag to add a reencrypt route on OpenShift clusters. The reencrypt route serves a certificate that was signed by the OpenShift CA.

SCR-20250227-mzrp

Sadly I was not able to add unit tests because of outdated route API specifications in helmtest (see garethr/openshift-json-schema#7). I imagine that is also the reason why there are no tests for the existing routes today.

Edit: I'll do follow ups for the official documentation and the operator CR.

User-facing documentation

  • CHANGELOG is updated OR update is not needed
  • documentation PR is created and is linked above OR is not needed

Testing and quality

  • the change is production ready: the change is GA or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

see screenshot

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Feb 26, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Feb 26, 2025 •
edited
Loading

Images are ready for the commit at 29e9072.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-368-g29e9072de3.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 26, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.88%. Comparing base (cde3b5f) to head (29e9072).
Report is 4 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #14416      +/-   ##
==========================================
- Coverage   48.88%   48.88%   -0.01%     
==========================================
  Files        2547     2547              
  Lines      186906   186906              
==========================================
- Hits        91365    91364       -1     
- Misses      88309    88310       +1     
  Partials     7232     7232
Flag Coverage Δ
go-unit-tests 48.88% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stehessel stehessel force-pushed the hack/ocp-reencrypt-route branch 8 times, most recently from 34f739f to 329bf67 Compare February 28, 2025 13:57
@stehessel stehessel changed the title hackathon: add OpenShift reencrypt route ROX-28296: add OpenShift reencrypt route Feb 28, 2025
@stehessel stehessel force-pushed the hack/ocp-reencrypt-route branch from 329bf67 to ad596a9 Compare February 28, 2025 14:27
@stehessel stehessel marked this pull request as ready for review February 28, 2025 14:28
@stehessel stehessel requested review from a team and mclasmeier and removed request for a team February 28, 2025 14:28
@stehessel
Copy link
Copy Markdown
Collaborator Author

stehessel commented Feb 28, 2025

/retest

@stehessel stehessel force-pushed the hack/ocp-reencrypt-route branch from ad596a9 to 07ca412 Compare March 3, 2025 08:40
kovayur
kovayur reviewed Mar 4, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@kovayur kovayur left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution Stephan, it's a very useful feature!

@stehessel stehessel force-pushed the hack/ocp-reencrypt-route branch 4 times, most recently from e0f50bd to 2873269 Compare March 10, 2025 21:14
@stehessel stehessel requested a review from kovayur March 10, 2025 21:15
@stehessel
Copy link
Copy Markdown
Collaborator Author

stehessel commented Mar 11, 2025

/retest

1 similar comment
@stehessel
Copy link
Copy Markdown
Collaborator Author

stehessel commented Mar 11, 2025

/retest

@stehessel stehessel requested a review from mclasmeier March 25, 2025 14:55
mclasmeier
mclasmeier requested changes Mar 25, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mclasmeier mclasmeier left a comment

Choose a reason for hiding this comment

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

empty

@stehessel stehessel force-pushed the hack/ocp-reencrypt-route branch from 5a11097 to 9051ed0 Compare March 26, 2025 12:10
stehessel
stehessel commented Apr 1, 2025
View reviewed changes
@stehessel stehessel requested a review from mclasmeier April 1, 2025 10:19
mclasmeier
mclasmeier reviewed Apr 1, 2025
View reviewed changes
@stehessel stehessel force-pushed the hack/ocp-reencrypt-route branch from ee2d7c0 to 1426ef3 Compare April 1, 2025 11:24
mclasmeier
mclasmeier reviewed Apr 1, 2025
View reviewed changes
mclasmeier
mclasmeier reviewed Apr 1, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mclasmeier mclasmeier left a comment

Choose a reason for hiding this comment

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

Looking good, thank you!
Could you maybe add a helmtest as well?

@stehessel
Copy link
Copy Markdown
Collaborator Author

stehessel commented Apr 1, 2025 •
edited
Loading

Could you maybe add a helmtest as well?

Unfortunately I can't (with reasonable effort, at least to my knowledge) - the OCP CRDs included in helmtest do not include Route - hence why there are no tests for the current route exposure. See my comment here.

Sadly I was not able to add unit tests because of outdated route API specifications in helmtest (see garethr/openshift-json-schema#7). I imagine that is also the reason why there are no tests for the existing routes today.

@mclasmeier
Copy link
Copy Markdown
Contributor

mclasmeier commented Apr 1, 2025

Right! I remember you mentioning this before.

@stehessel stehessel merged commit b8b5900 into master Apr 1, 2025
97 checks passed
@stehessel stehessel deleted the hack/ocp-reencrypt-route branch April 1, 2025 16:28
This was referenced Apr 7, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kovayur kovayur kovayur approved these changes

@mclasmeier mclasmeier mclasmeier approved these changes

Assignees

No one assigned

Labels

area/helm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@stehessel @rhacs-bot @mclasmeier @kovayur