✨ Add componentRoutes support to ROSAControlPlane - #6183
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Welcome @reedcort! |
|
Hi @reedcort. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
8366a6c to
e213b72
Compare
|
@serngawy The PR is ready for review. I have made the recommended changes. |
|
/ok-to-test |
e213b72 to
d54fc81
Compare
|
/retest-required |
|
/hold |
|
@andclt would you review as well |
| desiredRoutes := rosaScope.ControlPlane.Spec.ComponentRoutes | ||
| if len(desiredRoutes) == 0 { | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Instead of returning early here should we check similarly to what findExistingLogForwarders does for reconcileLogForwarders if there's an existing ingress with component routes set and clear them?
There was a problem hiding this comment.
yeah customers should be able to pass in empty strings to clear existing routes.
| keys := []rosacontrolplanev1.ComponentRouteKey{rosacontrolplanev1.ComponentRouteConsole, rosacontrolplanev1.ComponentRouteDownloads} | ||
| routes := make(map[string]*cmv1.ComponentRouteBuilder, len(keys)) | ||
| for _, key := range keys { | ||
| routes[string(key)] = cmv1.NewComponentRoute().Hostname("").TlsSecretRef("") |
There was a problem hiding this comment.
just out of curiosity does the OCM API accepts empty strings for Hostname and TlsSecretRef?
d54fc81 to
2833071
Compare
Add support for customizing console and downloads route hostnames and TLS certificates on ROSA HCP clusters via the ROSAControlPlane CRD. Signed-off-by: Cortney Reed <creed@redhat.com>
2833071 to
d6599c3
Compare
|
/test pull-cluster-api-provider-aws-apidiff-main |
|
@reedcort: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Test Results ✅ Tested componentRoutes end-to-end on a ROSA HCP cluster (OCP 4.22.8) using this image on a Minikube hub with CAPI v1.13.4. All cases passed:
|
|
@nrb would you review/lgtm this PR, I believe the |
|
/unhold |
What type of PR is this?
/kind feature
/kind api-change
What this PR does / why we need it:
Adds support for customizing console and downloads route hostnames and TLS certificates on ROSA HCP clusters managed via Cluster API.
This enables ROSA HCP customers to configure custom component routes (e.g.
console.company.com) through theROSAControlPlaneCRD as a day-2 operation. The feature is already supported via the OCM API, ROSA CLI, and Terraform provider.Which issue(s) this PR fixes:
ROSA HCP clusters managed via Cluster API cannot configure custom hostnames for console and downloads routes. This PR adds
componentRoutesto theROSAControlPlanespec to enable this.Special notes for your reviewer:
ComponentRoutesis an optional map field onRosaControlPlaneSpec— no impact on existing clustersconsoleanddownloadskeys are valid. OAuth is not configurable on HCPAI Usage:
Claude Code (Claude Opus) was used to assist with code generation and test writing. I reviewed every line before committing and validated the changes compile and pass unit tests locally.
Checklist:
Release note: