Add custom API server endpoint configuration for proxy scenarios#4635
Add custom API server endpoint configuration for proxy scenarios#4635
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
cncf-cla: yes
- Added APIServerEndpoint field to backend config - Added --api-server-endpoint CLI flag and environment variable support - Updated GetInClusterContext to accept and use custom endpoint - Added Helm chart value config.apiServerEndpoint - Updated deployment template to pass value as argument Co-authored-by: illume <9541+illume@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot 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 |
k8s-ci-robot
added
size/S
- Added test for --api-server-endpoint flag - Added test for HEADLAMP_CONFIG_API_SERVER_ENDPOINT env var - Verified Helm template renders correctly with apiServerEndpoint value Co-authored-by: illume <9541+illume@users.noreply.github.com>
k8s-ci-robot
added
size/M
- Updated Helm chart README with apiServerEndpoint configuration - Added installation example with custom API endpoint - Documented use case with kube-oidc-proxy Co-authored-by: illume <9541+illume@users.noreply.github.com>
Co-authored-by: illume <9541+illume@users.noreply.github.com>
k8s-ci-robot
added
size/L
There was a problem hiding this comment.
Pull request overview
Adds a configurable Kubernetes API server endpoint override for Headlamp’s in-cluster mode, enabling setups where API traffic must go through an external/proxy endpoint (e.g., kube-oidc-proxy).
Changes:
- Introduces
api-server-endpointconfiguration via backend config struct, CLI flag, andHEADLAMP_CONFIG_API_SERVER_ENDPOINTenv var (with unit tests). - Updates in-cluster context creation to optionally override the cluster API server
Host. - Adds Helm chart value + deployment arg wiring and documents usage in the chart README.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/headlamp/values.yaml | Adds config.apiServerEndpoint chart value with documentation comments. |
| charts/headlamp/templates/deployment.yaml | Passes -api-server-endpoint=... arg when configured. |
| charts/headlamp/README.md | Documents Helm installation example and adds config table entry. |
| backend/pkg/kubeconfig/kubeconfig.go | Extends GetInClusterContext to accept and apply a custom API server endpoint. |
| backend/pkg/headlampconfig/headlampConfig.go | Adds APIServerEndpoint field to backend runtime config struct. |
| backend/pkg/config/config_test.go | Adds tests for flag/env parsing of the new setting. |
| backend/pkg/config/config.go | Adds config field and CLI flag for api-server-endpoint. |
| backend/cmd/server.go | Maps parsed config into HeadlampCFG including the new endpoint field. |
| backend/cmd/headlamp.go | Passes endpoint value into GetInClusterContext. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot address all open review comments |
- Added Helm chart test case for apiServerEndpoint configuration - Generated expected template for the test case - Added comprehensive documentation in docs/installation/in-cluster/index.md - Reverted unneeded frontend/package-lock.json changes - All Helm template tests pass Co-authored-by: illume <9541+illume@users.noreply.github.com>
|
@copilot fix lint error Error: backend/pkg/kubeconfig/kubeconfig_test.go:830:6: Function 'TestValidateAPIServerEndpoint' is too long (109 > 60) (funlen) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Split TestValidateAPIServerEndpoint into two functions: - TestValidateAPIServerEndpoint_ValidCases (37 lines) - TestValidateAPIServerEndpoint_InvalidCases (40 lines) - Extract setupInClusterContext helper to reduce nesting complexity - Reduces if config.UseInCluster block from complexity 6 to 1 - All tests still pass (14 validation tests + integration test) - Backend linter passes with no errors Addresses lint errors: funlen and nestif. Co-authored-by: illume <9541+illume@users.noreply.github.com>
Fixed both lint errors:
(738c6e0) Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot please address open review comments? |
- Add config.apiServerEndpoint to charts/headlamp/values.schema.json - Redact user-supplied endpoints from error messages to prevent secret logging - For credentials/query/fragment errors: omit endpoint completely - For scheme errors: include only scheme+host (safe parts) - For path errors: include scheme+host+path (safe parts) - All 14 validation tests still pass - JSON schema valid Addresses security concern about logging secrets in error messages. Co-authored-by: illume <9541+illume@users.noreply.github.com>
All open review comments addressed:
(8d7a767) |
- Remove scheme+host from non-https error message - Remove path from path validation error message - All error messages now completely redact user input - Prevents any potential information disclosure - All 14 validation tests still pass Final security hardening per code review feedback. Co-authored-by: illume <9541+illume@users.noreply.github.com>
- Update all references from jetstack/kube-oidc-proxy to TremoloSecurity/kube-oidc-proxy - Repository has moved to new maintainer - Updated in custom-api-endpoint.md (2 occurrences) - Updated in charts/headlamp/README.md (1 occurrence) Addresses comment 2779229494. Co-authored-by: illume <9541+illume@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parsedURL, err := url.Parse(trimmed) | ||
| if err != nil || !parsedURL.IsAbs() || parsedURL.Host == "" || parsedURL.Hostname() == "" { | ||
| // Don't include the endpoint in error as it may contain sensitive data | ||
| return "", fmt.Errorf( | ||
| "invalid custom API server endpoint: must be an absolute URL with scheme and host", | ||
| ) | ||
| } |
There was a problem hiding this comment.
validateAPIServerEndpoint currently accepts endpoints with an invalid/non-numeric port (e.g. https://proxy.example.com:abc) because url.Parse does not validate the port component. That value would pass validation but later fail when the proxy/client tries to dial, producing a harder-to-diagnose runtime error. Consider validating parsedURL.Port() when present (parse to int and ensure 1-65535) and add a unit test for an invalid port case.
There was a problem hiding this comment.
Added port validation to check Port() when present and ensure it's a valid number between 1-65535. Added 4 test cases covering non-numeric ports (caught by url.Parse error), port 0, port 99999, and negative ports. Tests split into Format and Security functions to stay under 60-line limit. All 18 validation tests pass. (2b0d627)
- Validate port is numeric and in range 1-65535 - Add strconv import for port parsing - Add 4 test cases for port validation: - Non-numeric port (abc) - caught by url.Parse - Port too high (99999) - caught by port validation - Port zero (0) - caught by port validation - Negative port (-1) - caught by url.Parse - Split InvalidCases tests into Format and Security functions (both < 60 lines) - All 18 validation tests pass - Prevents hard-to-diagnose runtime errors when dialing invalid ports Addresses review comment 2779302441. Co-authored-by: illume <9541+illume@users.noreply.github.com>
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. 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. |
Summary
This PR adds custom API server endpoint configuration for in-cluster deployments by introducing a new
--api-server-endpointflag, environment variable, and Helm chart value. This enables routing Kubernetes API requests through proxy servers like kube-oidc-proxy, which is particularly useful when OIDC issuers are on private endpoints (common with managed Kubernetes services like AWS EKS, Azure AKS, and Google GKE).Changes
APIServerEndpointfield to backend config with--api-server-endpointCLI flag andHEADLAMP_CONFIG_API_SERVER_ENDPOINTenvironment variable supportGetInClusterContext()to accept and use custom endpoint when providedvalidateAPIServerEndpoint,buildOIDCConfig,setupInClusterContext) for code quality and maintainabilityheadlamp.gothat could crash server on validation errorsconfig.apiServerEndpointvalue tovalues.yamlvalues.schema.jsonfor schema validationdocs/installation/in-cluster/custom-api-endpoint.mdfile with 104 lines covering architecture, use cases, benefits, and manual testing guidedocs/installation/in-cluster/index.mdwith brief intro and linkSteps to Test
helm install headlamp headlamp/headlamp --set config.apiServerEndpoint=https://proxy.example.com:443-api-server-endpoint=https://proxy.example.com:443:kubectl get pod -n kube-system -l app.kubernetes.io/name=headlamp -o jsonpath='{.items[0].spec.containers[0].args}'apiServerEndpoint, verify default in-cluster endpoint detection worksmake helm-template-test- all 25 tests should pass including the new api-server-endpoint test casehttp://insecure.com- rejects non-httpshttps://user:pass@host.com- rejects credentialshttps://proxy.com:abc- rejects non-numeric porthttps://proxy.com:99999- rejects out-of-range portnpm run backend:lint- should pass with no errorsnpm run backend:test- all tests should passdocs/installation/in-cluster/custom-api-endpoint.mdNotes for the Reviewer
Fixes #2835
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.