Skip to content

Conversation

rwhundley
Copy link
Collaborator

Originating issue: IBMPrivateCloud/roadmap#66643

@ibm-ci-bot
Copy link

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

@@ -588,6 +588,9 @@ func (r *AuthenticationReconciler) getClusterAddress(authCR *operatorv1alpha1.Au
clusterInfoConfigMap := &corev1.ConfigMap{}

clusterAddressFieldName := "cluster_address"
if authCR.Spec.Config.ZenFrontDoor && ctrlcommon.ClusterHasZenExtensionGroupVersion(&r.DiscoveryClient) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason of using cluster_address_auth instead of cluster_address for ZenFrontDoor ?

Copy link
Collaborator Author

@rwhundley rwhundley May 13, 2025

Choose a reason for hiding this comment

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

cluster_address_auth has essentially meant "the host that is delegated to us from elsewhere" since it was introduced for ZenExtension reasons. Then we made [cluster_address and cluster_address_auth] always the same value, so maybe it matters less. And, in an ideal world, I'd get rid of all of these redundant values and have less to track. Unfortunately, I have no idea who or what is looking at which fields when, so this is being done mostly as a hedge against that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suppose the next question is why am I not using the function that's defined 150 lines earlier that is a more readable shorthand for the imperative conditional here? Probably because I was tired. 🙃 Given this has passed internal validations, I'll go ahead and start cleaning this branch up.

@@ -834,7 +834,7 @@ func (r *AuthenticationReconciler) generateCNCFClusterInfo(ctx context.Context,
clusterAddress := strings.Join([]string{strings.Join([]string{"cp-console", authCR.Namespace}, "-"), domainName}, ".")
clusterEndpoint := "https://" + clusterAddress
clusterAddressAuth := clusterAddress
if authCR.Spec.Config.ZenFrontDoor && ctrlcommon.ClusterHasZenExtensionGroupVersion(&r.DiscoveryClient) {
if shouldUseCPDHost(authCR, &r.DiscoveryClient) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just happened to notice this could be replaced by the function I had defined in routes.go, so I went ahead and did that so that it's being reused where appropriate.

@rwhundley rwhundley marked this pull request as ready for review May 13, 2025 15:07
@ibm-ci-bot ibm-ci-bot requested a review from yannizhang2019 May 13, 2025 15:07
@rwhundley
Copy link
Collaborator Author

Ah, right. Test changes. Need to work those out.

@rwhundley
Copy link
Collaborator Author

/retest

@rwhundley
Copy link
Collaborator Author

Putting this on hold until next release.

@rwhundley
Copy link
Collaborator Author

/retest

rwhundley added 2 commits July 7, 2025 11:24
Signed-off-by: Rob Hundley <[email protected]>
Signed-off-by: Rob Hundley <[email protected]>
rwhundley added 3 commits July 7, 2025 22:17
Signed-off-by: Rob Hundley <[email protected]>
Signed-off-by: Rob Hundley <[email protected]>
rwhundley added 5 commits July 9, 2025 09:03
Signed-off-by: Rob Hundley <[email protected]>
Signed-off-by: Rob Hundley <[email protected]>
Signed-off-by: Rob Hundley <[email protected]>
Signed-off-by: Rob Hundley <[email protected]>
Copy link
Member

@Tirumalavasa Tirumalavasa left a comment

Choose a reason for hiding this comment

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

/lgtm

@ibm-ci-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rwhundley, Tirumalavasa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Tirumalavasa,rwhundley]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rwhundley rwhundley merged commit 86635b3 into master Jul 11, 2025
1 check passed
@rwhundley rwhundley deleted the roadmap#66643 branch July 11, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants