From 23feaf54f7054da3a27ce88b958982df3887818c Mon Sep 17 00:00:00 2001 From: aantal Date: Wed, 1 Jul 2026 14:31:03 +0200 Subject: [PATCH 1/3] MGMT-24693: Normalize unbracketed IPv6 ignition endpoint URLs before validation The edge CAPI disconnected test pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-disconnected-capi started failing after PR #10396 upgraded assisted-service to Go 1.26. Go 1.26 tightened net/url.Parse: URLs with an unbracketed IPv6 host and port are now rejected (urlstrictcolons). Hypershift builds ignition endpoints as host:port without brackets (e.g. fd2e:6f44:5dd8:c956::14:31187), and cluster-api-provider-agent copies that into AgentClusterInstall. The operator then syncs the URL to the backend, where ValidateHTTPFormat fails: parse "https://fd2e:6f44:5dd8:c956::14:31187": invalid port ":6f44:5dd8:c956::14:31187" after host That surfaces as ACI SpecSynced=False (InputError), the worker agent stays insufficient (ignition-downloadable pending), and the test times out waiting for a day-2 worker node. The same IPv6 lab and URL format passed on Go 1.25 (e.g. PR #10319, Jun 2026); the regression correlates with the toolchain bump, not with the OCP 5.0 payload shift. Add NormalizeHTTPURL to bracket IPv6 literal hosts via net.JoinHostPort. Apply it in ValidateHTTPFormat and in parseIgnitionEndpoint so URLs from ACI are normalized before API validation and storage. The root cause is upstream in Hypershift URL construction; this change is a defensive fix on the assisted-service side. A companion fix in cluster-api-provider-agent normalizes the URL when writing ACI. --- .../clusterdeployments_controller.go | 3 +- pkg/validations/validations.go | 39 +++++++++++++++++++ pkg/validations/validations_test.go | 17 ++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/internal/controller/controllers/clusterdeployments_controller.go b/internal/controller/controllers/clusterdeployments_controller.go index 55497030869a..db8ea0495097 100644 --- a/internal/controller/controllers/clusterdeployments_controller.go +++ b/internal/controller/controllers/clusterdeployments_controller.go @@ -53,6 +53,7 @@ import ( "github.com/openshift/assisted-service/pkg/auth" logutil "github.com/openshift/assisted-service/pkg/log" "github.com/openshift/assisted-service/pkg/mirrorregistries" + pkgvalidations "github.com/openshift/assisted-service/pkg/validations" "github.com/openshift/assisted-service/restapi/operations/installer" operations "github.com/openshift/assisted-service/restapi/operations/manifests" hivev1 "github.com/openshift/hive/apis/hive/v1" @@ -905,7 +906,7 @@ func (r *ClusterDeploymentsReconciler) parseIgnitionEndpoint(ctx context.Context kubeapiIgnitionEndpoint *hiveext.IgnitionEndpoint) (*models.IgnitionEndpoint, error) { ignitionEndpoint := &models.IgnitionEndpoint{} - ignitionEndpoint.URL = swag.String(kubeapiIgnitionEndpoint.Url) + ignitionEndpoint.URL = swag.String(pkgvalidations.NormalizeHTTPURL(kubeapiIgnitionEndpoint.Url)) caCertificateReference := kubeapiIgnitionEndpoint.CaCertificateReference if caCertificateReference != nil { diff --git a/pkg/validations/validations.go b/pkg/validations/validations.go index d54bb8e7f817..eb283cc9c243 100644 --- a/pkg/validations/validations.go +++ b/pkg/validations/validations.go @@ -83,8 +83,47 @@ func ValidateNTPSource(ntpSource string) bool { return false } +// NormalizeHTTPURL rewrites HTTP(S) URLs that contain an unbracketed IPv6 literal host +// so they can be parsed by net/url (Go 1.26+). +func NormalizeHTTPURL(theurl string) string { + if theurl == "" { + return theurl + } + const schemeSep = "://" + schemeIdx := strings.Index(theurl, schemeSep) + if schemeIdx < 0 { + return theurl + } + scheme := theurl[:schemeIdx+len(schemeSep)] + remainder := theurl[schemeIdx+len(schemeSep):] + + path := "" + if slash := strings.Index(remainder, "/"); slash >= 0 { + path = remainder[slash:] + remainder = remainder[:slash] + } + + if strings.HasPrefix(remainder, "[") { + return theurl + } + + lastColon := strings.LastIndex(remainder, ":") + if lastColon < 0 { + return theurl + } + + host := remainder[:lastColon] + port := remainder[lastColon+1:] + if port == "" || net.ParseIP(host) == nil || !strings.Contains(host, ":") { + return theurl + } + + return scheme + net.JoinHostPort(host, port) + path +} + // ValidateHTTPFormat validates the HTTP and HTTPS format func ValidateHTTPFormat(theurl string) error { + theurl = NormalizeHTTPURL(theurl) u, err := url.Parse(theurl) if err != nil { return fmt.Errorf("URL '%s' format is not valid: %w", theurl, err) diff --git a/pkg/validations/validations_test.go b/pkg/validations/validations_test.go index 1ea723ce4c5a..2e3095332223 100644 --- a/pkg/validations/validations_test.go +++ b/pkg/validations/validations_test.go @@ -94,6 +94,8 @@ var _ = Describe("URL validations", func() { {"http://10.9.8.7:123/config", ""}, {"http://[1080::8:800:200c:417a]:123", ""}, {"http://[1080::8:800:200c:417a]:123/config", ""}, + {"https://fd2e:6f44:5dd8:c956::14:31187", ""}, + {"https://fd2e:6f44:5dd8:c956::14:31187/ignition", ""}, { "http://[1080:0:0:0:8:800:200C:417A]:8888 ", "URL 'http://[1080:0:0:0:8:800:200C:417A]:8888 ' format is not valid: parse \"http://[1080:0:0:0:8:800:200C:417A]:8888 \": invalid port \":8888 \" after host", @@ -130,6 +132,21 @@ var _ = Describe("URL validations", func() { }) }) + Context("NormalizeHTTPURL", func() { + It("brackets unbracketed IPv6 ignition URLs", func() { + Expect(NormalizeHTTPURL("https://fd2e:6f44:5dd8:c956::14:31187")). + To(Equal("https://[fd2e:6f44:5dd8:c956::14]:31187")) + Expect(NormalizeHTTPURL("https://fd2e:6f44:5dd8:c956::14:31187/ignition")). + To(Equal("https://[fd2e:6f44:5dd8:c956::14]:31187/ignition")) + }) + + It("leaves IPv4 and already bracketed IPv6 URLs unchanged", func() { + Expect(NormalizeHTTPURL("https://1.2.3.4:555/ignition")).To(Equal("https://1.2.3.4:555/ignition")) + Expect(NormalizeHTTPURL("http://[1080::8:800:200c:417a]:123/config")). + To(Equal("http://[1080::8:800:200c:417a]:123/config")) + }) + }) + DescribeTable("ValidateNoProxyFormat", func(noProxy string, expectSuccess bool) { err := ValidateNoProxyFormat(noProxy) From 13d8ea0bd08d7a3e25734cf6ad96c09e7d5cc922 Mon Sep 17 00:00:00 2001 From: aantal Date: Wed, 1 Jul 2026 17:17:54 +0200 Subject: [PATCH 2/3] creating a uniform URL normalization through the project --- internal/bminventory/inventory.go | 1 + .../inventory_ignition_endpoint_unit_test.go | 22 +++++++++++++ internal/host/hostutil/host_utils.go | 4 ++- .../hostutil/host_utils_ignition_url_test.go | 31 +++++++++++++++++++ internal/host/hostutil/host_utils_test.go | 9 ++++++ 5 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 internal/bminventory/inventory_ignition_endpoint_unit_test.go create mode 100644 internal/host/hostutil/host_utils_ignition_url_test.go diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go index 8d4cd53c6176..5dc468119f36 100644 --- a/internal/bminventory/inventory.go +++ b/internal/bminventory/inventory.go @@ -4669,6 +4669,7 @@ func (b *bareMetalInventory) validateIgnitionEndpoint(ignitionEndpoint *models.I if ignitionEndpoint == nil || ignitionEndpoint.URL == nil { return nil } + *ignitionEndpoint.URL = pkgvalidations.NormalizeHTTPURL(*ignitionEndpoint.URL) if err := pkgvalidations.ValidateHTTPFormat(*ignitionEndpoint.URL); err != nil { log.WithError(err).Errorf("Invalid Ignition endpoint URL: %s", *ignitionEndpoint.URL) return common.NewApiError(http.StatusBadRequest, err) diff --git a/internal/bminventory/inventory_ignition_endpoint_unit_test.go b/internal/bminventory/inventory_ignition_endpoint_unit_test.go new file mode 100644 index 000000000000..3f088888b1f7 --- /dev/null +++ b/internal/bminventory/inventory_ignition_endpoint_unit_test.go @@ -0,0 +1,22 @@ +package bminventory + +import ( + "testing" + + "github.com/openshift/assisted-service/models" + "github.com/sirupsen/logrus" +) + +func TestValidateIgnitionEndpointNormalizesUnbracketedIPv6(t *testing.T) { + bm := &bareMetalInventory{} + url := "https://fd2e:6f44:5dd8:c956::14:31187" + ignitionEndpoint := &models.IgnitionEndpoint{URL: &url} + + if err := bm.validateIgnitionEndpoint(ignitionEndpoint, logrus.New()); err != nil { + t.Fatalf("validateIgnitionEndpoint returned error: %v", err) + } + want := "https://[fd2e:6f44:5dd8:c956::14]:31187" + if *ignitionEndpoint.URL != want { + t.Fatalf("got URL %q, want %q", *ignitionEndpoint.URL, want) + } +} diff --git a/internal/host/hostutil/host_utils.go b/internal/host/hostutil/host_utils.go index 9db1fa396675..96d0c8337e27 100644 --- a/internal/host/hostutil/host_utils.go +++ b/internal/host/hostutil/host_utils.go @@ -21,6 +21,7 @@ import ( "github.com/openshift/assisted-service/internal/constants" "github.com/openshift/assisted-service/models" "github.com/openshift/assisted-service/pkg/conversions" + pkgvalidations "github.com/openshift/assisted-service/pkg/validations" "github.com/pkg/errors" "github.com/samber/lo" "github.com/sirupsen/logrus" @@ -372,7 +373,8 @@ func GetIgnitionEndpointAndCert(cluster *common.Cluster, host *models.Host, logg // Use custom ignition endpoint if provided if cluster.IgnitionEndpoint != nil && cluster.IgnitionEndpoint.URL != nil { - url, err := url.Parse(*cluster.IgnitionEndpoint.URL) + normalizedURL := pkgvalidations.NormalizeHTTPURL(*cluster.IgnitionEndpoint.URL) + url, err := url.Parse(normalizedURL) if err != nil { return "", nil, err } diff --git a/internal/host/hostutil/host_utils_ignition_url_test.go b/internal/host/hostutil/host_utils_ignition_url_test.go new file mode 100644 index 000000000000..1e1e77e741b6 --- /dev/null +++ b/internal/host/hostutil/host_utils_ignition_url_test.go @@ -0,0 +1,31 @@ +package hostutil + +import ( + "testing" + + "github.com/openshift/assisted-service/internal/common" + "github.com/openshift/assisted-service/models" + "github.com/sirupsen/logrus" +) + +func TestGetIgnitionEndpointAndCertUnbracketedIPv6CustomEndpoint(t *testing.T) { + customEndpoint := "https://fd2e:6f44:5dd8:c956::14:31187" + cluster := common.Cluster{ + Cluster: models.Cluster{ + IgnitionEndpoint: &models.IgnitionEndpoint{URL: &customEndpoint}, + }, + } + host := models.Host{Role: models.HostRoleWorker} + + url, cert, err := GetIgnitionEndpointAndCert(&cluster, &host, logrus.New()) + if err != nil { + t.Fatalf("GetIgnitionEndpointAndCert returned error: %v", err) + } + if cert != nil { + t.Fatalf("expected no certificate, got %v", cert) + } + want := "https://[fd2e:6f44:5dd8:c956::14]:31187/worker" + if url != want { + t.Fatalf("got URL %q, want %q", url, want) + } +} diff --git a/internal/host/hostutil/host_utils_test.go b/internal/host/hostutil/host_utils_test.go index c2a508778c7f..9b5d7fabdf4c 100644 --- a/internal/host/hostutil/host_utils_test.go +++ b/internal/host/hostutil/host_utils_test.go @@ -157,6 +157,15 @@ var _ = Describe("Ignition endpoint URL generation", func() { Expect(cert).Should(BeNil()) Expect(err).ShouldNot(HaveOccurred()) }) + It("for cluster with unbracketed IPv6 custom IgnitionEndpoint", func() { + customEndpoint := "https://fd2e:6f44:5dd8:c956::14:31187" + Expect(db.Model(&cluster).Update("ignition_endpoint_url", customEndpoint).Error).ShouldNot(HaveOccurred()) + + url, cert, err := GetIgnitionEndpointAndCert(&cluster, &host, logrus.New()) + Expect(url).Should(Equal("https://[fd2e:6f44:5dd8:c956::14]:31187/worker")) + Expect(cert).Should(BeNil()) + Expect(err).ShouldNot(HaveOccurred()) + }) It("failing for cluster with wrong IgnitionEndpoint", func() { customEndpoint := "https\\://foo.bar:33735/acme" Expect(db.Model(&cluster).Update("ignition_endpoint_url", customEndpoint).Error).ShouldNot(HaveOccurred()) From 213bb6ce92a16f6ab48fe5c55065b9672339a0d7 Mon Sep 17 00:00:00 2001 From: aantal Date: Thu, 2 Jul 2026 16:40:12 +0200 Subject: [PATCH 3/3] adding some retries for image and catalog mirroring --- deploy/operator/capi/deploy_capi_cluster.sh | 6 +- deploy/operator/mirror_utils.sh | 67 ++++++++++++++++++++- deploy/operator/setup_hive.sh | 3 +- 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/deploy/operator/capi/deploy_capi_cluster.sh b/deploy/operator/capi/deploy_capi_cluster.sh index 2bb6317fea3f..34ba44d70873 100755 --- a/deploy/operator/capi/deploy_capi_cluster.sh +++ b/deploy/operator/capi/deploy_capi_cluster.sh @@ -75,13 +75,13 @@ if [ "${DISCONNECTED}" = "true" ]; then oc create secret generic "${ASSISTED_PULLSECRET_NAME}" --from-file=.dockerconfigjson="${ASSISTED_PULLSECRET_JSON}" --type=kubernetes.io/dockerconfigjson -n hypershift # 2. mirrored hypershift operator image to local mirror registry HYPERSHIFT_LOCAL_IMAGE="${LOCAL_REGISTRY}/$(get_image_repository_only ${HYPERSHIFT_IMAGE}):hypershift" - oc image mirror -a "${PULL_SECRET_FILE}" "${HYPERSHIFT_IMAGE}" "${HYPERSHIFT_LOCAL_IMAGE}" + run_mirror_command_with_retry oc image mirror -a "${PULL_SECRET_FILE}" "${HYPERSHIFT_IMAGE}" "${HYPERSHIFT_LOCAL_IMAGE}" export HYPERSHIFT_IMAGE="${HYPERSHIFT_LOCAL_IMAGE}" # 3. mirrored CAPI provider agent image to local mirror registry if [ ! -z "$PROVIDER_IMAGE" ] then export PROVIDER_LOCAL_IMAGE="${LOCAL_REGISTRY}/$(get_image_repository_only ${PROVIDER_IMAGE}):capi" - oc image mirror -a "${PULL_SECRET_FILE}" "${PROVIDER_IMAGE}" "${PROVIDER_LOCAL_IMAGE}" + run_mirror_command_with_retry oc image mirror -a "${PULL_SECRET_FILE}" "${PROVIDER_IMAGE}" "${PROVIDER_LOCAL_IMAGE}" export PROVIDER_IMAGE="${PROVIDER_LOCAL_IMAGE}" fi @@ -98,7 +98,7 @@ mirror: - name: ${RELEASE_IMAGE_HCP_OVERRIDE} - name: ${CAPI_IMAGE} EOM - oc-mirror --config isc.yaml --authfile "${PULL_SECRET_FILE}" --workspace file://$PWD/mirror docker://"${OCP_MIRROR_REGISTRY}" --v2 + run_mirror_command_with_retry oc-mirror --config isc.yaml --authfile "${PULL_SECRET_FILE}" --workspace "file://${PWD}/mirror" docker://"${OCP_MIRROR_REGISTRY}" --v2 # 4. ImageDigestMirrorSet for local mirror registry (prerequisite is the openshift release is mirrored to the local # registry). Note that older versions of OpenShift, before OpenShift 4.14, don't support this ImageDigestMirrorSet diff --git a/deploy/operator/mirror_utils.sh b/deploy/operator/mirror_utils.sh index f13bf9679b8c..be422c16052d 100644 --- a/deploy/operator/mirror_utils.sh +++ b/deploy/operator/mirror_utils.sh @@ -1,3 +1,64 @@ +function mirror_command_succeeded() { + log_file="${1}" + + if grep -qE 'one or more errors occurred|errors during mirroring|error: unable to copy layer' "${log_file}"; then + return 1 + fi + + return 0 +} + +function mirror_command_name() { + local args=("$@") + local i=0 + + if [[ "${args[0]}" == "env" ]]; then + i=1 + while [[ "${args[i]}" == *=* ]]; do + i=$((i + 1)) + done + fi + + echo "${args[i]}" +} + +function run_mirror_command_with_retry() { + attempts="${MIRROR_RETRY_ATTEMPTS:-5}" + interval="${MIRROR_RETRY_INTERVAL:-60}" + log_file="" + rc=0 + cmd_name=$(mirror_command_name "$@") + was_pipefail=false + shopt -q pipefail && was_pipefail=true + + for attempt in $(seq 1 "${attempts}"); do + log_file=$(mktemp) + echo "Mirror attempt ${attempt}/${attempts}: ${cmd_name}" + + set +o pipefail + "$@" 2>&1 | tee "${log_file}" + rc=${PIPESTATUS[0]} + if ${was_pipefail}; then + set -o pipefail + else + set +o pipefail + fi + + if [[ "${rc}" -eq 0 ]] && mirror_command_succeeded "${log_file}"; then + rm -f "${log_file}" + return 0 + fi + + echo "Mirror failed (exit=${rc}), waiting ${interval}s before retry..." + rm -f "${log_file}" + if [[ "${attempt}" -lt "${attempts}" ]]; then + sleep "${interval}" + fi + done + + return 1 +} + function mirror_package() { # Here we will do the next actions: # 1. Create an index of specific packages from specific remote indexes @@ -34,13 +95,13 @@ function mirror_package() { --packages "${package}" \ --tag "${local_registry_index_tag}" - GODEBUG=x509ignoreCN=0 podman push \ + run_mirror_command_with_retry env GODEBUG=x509ignoreCN=0 podman push \ --tls-verify=false \ "${local_registry_index_tag}" \ --authfile "${authfile}" manifests_dir=$(mktemp -d -t manifests-XXXXXXXXXX) - GODEBUG=x509ignoreCN=0 oc adm catalog mirror \ + run_mirror_command_with_retry env GODEBUG=x509ignoreCN=0 oc adm catalog mirror \ "${local_registry_index_tag}" \ "${local_registry_image_tag}" \ --registry-config="${authfile}" \ @@ -153,7 +214,7 @@ function ocp_mirror_release() { source_image="${2}" dest_mirror_repo="${3}" - oc adm -a "${pull_secret_file}" release mirror \ + run_mirror_command_with_retry oc adm -a "${pull_secret_file}" release mirror \ --from="${source_image}" \ --to="${dest_mirror_repo}" } diff --git a/deploy/operator/setup_hive.sh b/deploy/operator/setup_hive.sh index c3ff8fa9882a..3b7be50671a2 100755 --- a/deploy/operator/setup_hive.sh +++ b/deploy/operator/setup_hive.sh @@ -3,6 +3,7 @@ __dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source ${__dir}/common.sh source ${__dir}/utils.sh +source ${__dir}/mirror_utils.sh set -o xtrace @@ -139,7 +140,7 @@ function from_upstream() { if [ "${DISCONNECTED}" = "true" ]; then export IMG="${LOCAL_REGISTRY}/localimages/hive:latest" - oc image mirror \ + run_mirror_command_with_retry oc image mirror \ -a ${AUTHFILE} \ ${HIVE_IMAGE} \ ${IMG}