Skip to content

Conversation

@gojoy
Copy link
Contributor

@gojoy gojoy commented Dec 2, 2025

Description

after go 1.24.8. url.Parse enforce stricter parsing of bracketed IPv6 hostnames, When the URL like https://[10.250.0.1]:443, the operation fails. Therefore, in the kubeconfig file generated by install-cni, the square brackets around KUBERNETES_SERVICE_HOST should be removed.

2025-12-02 17:43:33.194 [ERROR][279844] cni-plugin/plugin.go 593: Final result of CNI DEL was an
error. error=host must be a URL or a host:port pair: "https://[10.250.0.1]:443"

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

Fix parsing of URLs in CNI configuration. 

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@gojoy gojoy requested a review from a team as a code owner December 2, 2025 12:34
@marvin-tigera marvin-tigera added this to the Calico v3.32.0 milestone Dec 2, 2025
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Dec 2, 2025
@caseydavenport
Copy link
Member

@gojoy does this mean we need to re-insert the brackets for v6 addresses?

@gojoy gojoy force-pushed the fix-kubeconfig-host branch from 3b79477 to 1e11897 Compare December 3, 2025 02:51
@gojoy
Copy link
Contributor Author

gojoy commented Dec 3, 2025

@gojoy does this mean we need to re-insert the brackets for v6 addresses?

Thank you for the reminder. I believe specifying the Host in the REST configuration is a good practice, as the client-go automatically handles IPv6 addresses through URL parsing. Refer to writeKubeconfig

Signed-off-by: GitHub <noreply@github.com>
@gojoy gojoy force-pushed the fix-kubeconfig-host branch from 1e11897 to ecf3589 Compare December 4, 2025 02:35
@gojoy
Copy link
Contributor Author

gojoy commented Dec 8, 2025

Would you mind reviewing it once more? @caseydavenport

data = strings.ReplaceAll(data, "__KUBERNETES_SERVICE_PROTOCOL__", getEnv("KUBERNETES_SERVICE_PROTOCOL", "https"))
data = strings.ReplaceAll(data, "__KUBERNETES_SERVICE_HOST__", getEnv("KUBERNETES_SERVICE_HOST", ""))
data = strings.ReplaceAll(data, "__KUBERNETES_SERVICE_PORT__", getEnv("KUBERNETES_SERVICE_PORT", ""))
data = strings.ReplaceAll(data, "__KUBERNETES_HOST_PORT__", net.JoinHostPort(getEnv("KUBERNETES_SERVICE_HOST", ""), getEnv("KUBERNETES_SERVICE_PORT", "")))
Copy link
Member

@caseydavenport caseydavenport Dec 8, 2025

Choose a reason for hiding this comment

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

Suggested change
data = strings.ReplaceAll(data, "__KUBERNETES_HOST_PORT__", net.JoinHostPort(getEnv("KUBERNETES_SERVICE_HOST", ""), getEnv("KUBERNETES_SERVICE_PORT", "")))
data = strings.ReplaceAll(data, "__KUBERNETES_SERVICE_HOST__", getEnv("KUBERNETES_SERVICE_HOST", ""))
data = strings.ReplaceAll(data, "__KUBERNETES_SERVICE_PORT__", getEnv("KUBERNETES_SERVICE_PORT", ""))
data = strings.ReplaceAll(data, "__KUBERNETES_SERVICE__", net.JoinHostPort(getEnv("KUBERNETES_SERVICE_HOST", ""), getEnv("KUBERNETES_SERVICE_PORT", "")))

I think we need to leave the existing replace statements - some users provide their own CNI configuration templates and may be relying on this replacement. Just because they aren't used by the default config template doesn't mean they might not be used elsewhere.

Let's also call it KUBERNETES_SERVICE since HOST_PORT has a meaning already in k8s networking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-pr-required Change is not yet documented release-note-required Change has user-facing impact (no matter how small)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants