Deprecate ExternalHostname string in favor of External structure #113

mjudeikis · 2025-10-14T10:31:34Z

Summary

This PR deprecates the ExternalHostname string field in the FrontProxySpec in favor of a new structured External configuration that provides more flexibility for configuring external access to front-proxy instances.

Key Changes

API Changes

Deprecate FrontProxySpec.ExternalHostname field with deprecation notice
Add new FrontProxySpec.External structure of type ExternalConfig with the following fields:
- Hostname (required): External hostname for the front-proxy
- Port (required): External port for the front-proxy
- PrivateHostname (optional): Internal hostname for kcp component communication
- PrivatePort (optional): Internal port for kcp component communication
Extend RootShard.Spec.External with optional PrivateHostname and PrivatePort fields for internal access patterns
Add CEL validation rule preventing simultaneous use of deprecated ExternalHostname and new External.hostname fields

Implementation Updates

Update kubeconfig generation logic to prioritize FrontProxy.Spec.External configuration over deprecated field
Update certificate generation to include private hostnames in DNS SANs when configured
Update RootShard external kubeconfig generation to support private hostname/port for internal communication
Pass FrontProxy object through kubeconfig controller chain for proper external configuration resolution

Backward Compatibility

The deprecated ExternalHostname field is maintained for backward compatibility. Existing configurations will continue to work, but users are encouraged to migrate to the new External structure.

What Type of PR Is This?

/kind api-change
/kind deprecation
/kind feature

Related Issue(s)

Fixes #

Release Notes

Deprecate FrontProxySpec.ExternalHostname in favor of structured External configuration with support for separate internal/external hostnames and ports.

Copilot

Pull Request Overview

This PR deprecates the ExternalHostname string field in the FrontProxySpec in favor of a new structured External configuration that provides more flexibility for configuring external access to front-proxy instances.

Add new External structure with hostname, port, and optional private hostname/port fields for both FrontProxy and RootShard specs
Deprecate ExternalHostname field with CEL validation to prevent simultaneous use of old and new fields
Update kubeconfig and certificate generation logic to prioritize the new External configuration

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
frontproxy_types.go	Adds new External field and deprecates ExternalHostname with CEL validation
rootshard_types.go	Extends ExternalConfig with optional PrivateHostname and PrivatePort fields
kubeconfig/secret.go	Updates kubeconfig generation to use new External configuration
frontproxy/certificates.go	Updates certificate generation to include private hostnames in DNS SANs
controller/kubeconfig/controller.go	Passes FrontProxy object through controller chain
rootshard/kubeconfigs.go	Adds support for private hostname/port in RootShard kubeconfig generation
Various generated files	Auto-generated code updates for new fields and deepcopy methods
CRD files	Updates to CustomResourceDefinition schemas for new fields
run-e2e-tests.sh	Adds health probe bind address configuration

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-14T10:33:05Z

internal/resources/frontproxy/certificates.go

 		dnsNames = append(dnsNames, r.rootShard.Spec.External.Hostname)
+		if r.rootShard.Spec.External.PrivateHostname != "" {
+			dnsNames = append(dnsNames, r.rootShard.Spec.External.PrivateHostname)
+		}

+		// DEPRECATED: keep support for the deprecated ExternalHostname field for now
+		// to not break existing front-proxy installations.
 		if r.frontProxy.Spec.ExternalHostname != "" {
 			dnsNames = append(dnsNames, r.frontProxy.Spec.ExternalHostname)
+		} else {
+			dnsNames = append(dnsNames, r.rootShard.Spec.External.Hostname)


The logic doesn't handle the new External.Hostname field. When using the new External configuration, the hostname from External.Hostname should be added to dnsNames, but this code only adds the deprecated ExternalHostname or falls back to rootShard hostname.

Suggested change

dnsNames = append(dnsNames, r.rootShard.Spec.External.Hostname)

if r.rootShard.Spec.External.PrivateHostname != "" {

dnsNames = append(dnsNames, r.rootShard.Spec.External.PrivateHostname)

}

// DEPRECATED: keep support for the deprecated ExternalHostname field for now

// to not break existing front-proxy installations.

if r.frontProxy.Spec.ExternalHostname != "" {

dnsNames = append(dnsNames, r.frontProxy.Spec.ExternalHostname)

} else {

dnsNames = append(dnsNames, r.rootShard.Spec.External.Hostname)

if r.rootShard.Spec.External.Hostname != "" {

dnsNames = append(dnsNames, r.rootShard.Spec.External.Hostname)

}

if r.rootShard.Spec.External.PrivateHostname != "" {

dnsNames = append(dnsNames, r.rootShard.Spec.External.PrivateHostname)

}

// Add the new External.Hostname field from the frontProxy spec if set

if r.frontProxy.Spec.External != nil && r.frontProxy.Spec.External.Hostname != "" {

dnsNames = append(dnsNames, r.frontProxy.Spec.External.Hostname)

}

// DEPRECATED: keep support for the deprecated ExternalHostname field for now

// to not break existing front-proxy installations.

if r.frontProxy.Spec.ExternalHostname != "" {

dnsNames = append(dnsNames, r.frontProxy.Spec.ExternalHostname)

Copilot · 2025-10-14T10:33:06Z

internal/resources/kubeconfig/secret.go

+		// New flow:
+		if frontProxy.Spec.External.Hostname != "" {
+			serverURL = fmt.Sprintf("https://%s:%d", frontProxy.Spec.External.Hostname, frontProxy.Spec.External.Port)
+		} else {
+			// Old flow:
+			serverURL = fmt.Sprintf("https://%s:%d", rootShard.Spec.External.Hostname, rootShard.Spec.External.Port)
+		}


Missing fallback to deprecated ExternalHostname field. The logic should check for the deprecated ExternalHostname when External.Hostname is empty, before falling back to rootShard configuration.

I think its ok. ExternalHostname was never used here

mjudeikis · 2025-10-14T11:17:47Z

/retest

embik · 2025-10-14T11:42:59Z

/retest

?

mjudeikis · 2025-10-14T11:49:48Z

/retest

?

Trying locally, as these looks like old flakes

mjudeikis · 2025-10-15T06:08:54Z

/retest
last try before I start debugging

mjudeikis · 2025-10-15T10:30:25Z

/retest

mjudeikis · 2025-10-15T17:23:10Z

/retest

mjudeikis · 2025-10-16T13:28:43Z

/retest

embik · 2025-10-16T13:50:46Z

/retest

still flaky but different flaky, I guess.

mjudeikis · 2025-10-16T14:36:18Z

/retest

mjudeikis · 2025-10-17T05:05:21Z

/retest

mjudeikis · 2025-10-17T06:29:36Z

/retest
lets see what CI tells us

Signed-off-by: Mangirdas Judeikis <[email protected]> On-behalf-of: SAP <[email protected]>

mjudeikis · 2025-10-17T07:43:31Z

/retest
kcp got bumped

embik

/approve

kcp-ci-bot · 2025-10-17T08:37:32Z

LGTM label has been added.

Git tree hash: f06199360dfe2d1c5e16b8879696e0a336073f06

kcp-ci-bot · 2025-10-17T08:37:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: embik

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~~ [embik]

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

mjudeikis requested a review from Copilot October 14, 2025 10:32

Copilot AI reviewed Oct 14, 2025

View reviewed changes

mjudeikis requested a review from embik October 14, 2025 10:33

mjudeikis mentioned this pull request Oct 14, 2025

Add BaseURL & issuer overrides, CABundle #101

Closed

mjudeikis added 2 commits October 17, 2025 09:39

Deprecate ExternalHostname string in favor of External structure

92adef4

Signed-off-by: Mangirdas Judeikis <[email protected]> On-behalf-of: SAP <[email protected]>

add fp hostname, not rootshard

b14dbd3

mjudeikis force-pushed the mjudeikis/deprecate.fp.url.setting branch from 76fcccf to d0d2a7c Compare October 17, 2025 06:40

mjudeikis force-pushed the mjudeikis/deprecate.fp.url.setting branch from d0d2a7c to b14dbd3 Compare October 17, 2025 08:07

embik approved these changes Oct 17, 2025

View reviewed changes

kcp-ci-bot assigned embik Oct 17, 2025

kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2025

kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 17, 2025

kcp-ci-bot merged commit 3923c22 into kcp-dev:main Oct 17, 2025
10 checks passed

mjudeikis mentioned this pull request Oct 20, 2025

feature: target a kcp workspace and custom endpoint with the 'kubeconfig' resource #94

Open

1 task

Deprecate ExternalHostname string in favor of External structure #113

Deprecate ExternalHostname string in favor of External structure #113

Uh oh!

Conversation

mjudeikis commented Oct 14, 2025

Summary

Key Changes

API Changes

Implementation Updates

Backward Compatibility

What Type of PR Is This?

Related Issue(s)

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

mjudeikis Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

mjudeikis commented Oct 14, 2025

Uh oh!

embik commented Oct 14, 2025

Uh oh!

mjudeikis commented Oct 14, 2025

Uh oh!

mjudeikis commented Oct 15, 2025

Uh oh!

mjudeikis commented Oct 15, 2025

Uh oh!

mjudeikis commented Oct 15, 2025

Uh oh!

mjudeikis commented Oct 16, 2025

Uh oh!

embik commented Oct 16, 2025

Uh oh!

mjudeikis commented Oct 16, 2025

Uh oh!

mjudeikis commented Oct 17, 2025

Uh oh!

mjudeikis commented Oct 17, 2025

Uh oh!

mjudeikis commented Oct 17, 2025

Uh oh!

embik left a comment

Choose a reason for hiding this comment

Uh oh!

kcp-ci-bot commented Oct 17, 2025

Uh oh!

kcp-ci-bot commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants