From 755af87095967c393b8e87d472116a7964e8d4d7 Mon Sep 17 00:00:00 2001 From: Zettat123 <39446+zettat123@noreply.gitea.com> Date: Sat, 17 Oct 2026 04:09:17 +0000 Subject: [PATCH] enhance(k8s): reach Kubernetes services by their id when it is a DNS subdomain (#1320) On Kubernetes a service is reached by its id made a DNS label, so some service ids might become unreachable. For example, the `devstoreaccount1.azurite.local` service of Gitea's [MSSQL tests](https://github.com/go-gitea/gitea/blob/2fe74d3cdd5184a7e22e32e47772caa78d077489/.github/workflows/pull-db-tests.yml#L229) is only in the pod's hosts as `devstoreaccount1-azurite-local`, while the tests connect to the id as written. Container names must be DNS labels, but `hostAliases` accepts DNS subdomains, so an id that is one is now added there too, and to `no_proxy`, as on Docker where a service is reached by its id. The warning for a service not reachable by its id now also covers ids that differ only in case, as Kubernetes hostnames are lowercase. --------- Co-authored-by: silverwind Reviewed-on: https://gitea.com/gitea/runner/pulls/1320 Reviewed-by: Lunny Xiao Reviewed-by: bircni Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Co-authored-by: Zettat123 <39446+zettat123@noreply.gitea.com> --- act/container/kubernetes.go | 29 +++++++++++++++++++++-------- act/container/kubernetes_test.go | 13 +++++++++++++ docs/kubernetes.md | 2 +- internal/app/run/runner.go | 6 ++++-- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/act/container/kubernetes.go b/act/container/kubernetes.go index 0e4bd545..aceb5d27 100644 --- a/act/container/kubernetes.go +++ b/act/container/kubernetes.go @@ -93,17 +93,29 @@ func (p *KubernetesPod) JobContainer(input *NewContainerInput) ExecutionsEnviron return p.job } -var invalidLabelChars = regexp.MustCompile(`[^a-z0-9-]+`) +var ( + invalidLabelChars = regexp.MustCompile(`[^a-z0-9-]+`) + dnsSubdomain = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) +) -// KubernetesServiceName returns the container name and hostname of service id, made a DNS label as both must be. -func KubernetesServiceName(id string) string { +// kubernetesServiceName returns the container name of service id, made a DNS label as it must be. +func kubernetesServiceName(id string) string { name := strings.Trim(invalidLabelChars.ReplaceAllString(strings.ToLower(id), "-"), "-") return strings.TrimRight(name[:min(63, len(name))], "-") } -// ServiceContainer returns a container of the pod the job reaches at hostname KubernetesServiceName(id). +// KubernetesServiceHostnames returns the hostnames of service id, its container name and, where hostAliases accept it as a DNS subdomain, id lowercased. +func KubernetesServiceHostnames(id string) []string { + name, host := kubernetesServiceName(id), strings.ToLower(id) + if host != name && len(host) <= 253 && dnsSubdomain.MatchString(host) { // https://github.com/kubernetes/apimachinery/blob/v0.35.1/pkg/util/validation/validation.go#L198-L207 + return []string{name, host} + } + return []string{name} +} + +// ServiceContainer returns a container of the pod the job reaches at KubernetesServiceHostnames(id). func (p *KubernetesPod) ServiceContainer(id string, input *NewContainerInput) ExecutionsEnvironment { - service := &kubernetesContainer{pod: p, id: id, name: KubernetesServiceName(id), input: input} + service := &kubernetesContainer{pod: p, id: id, name: kubernetesServiceName(id), input: input} p.services = append(p.services, service) return service } @@ -131,7 +143,7 @@ func (p *KubernetesPod) create(ctx context.Context) error { return err } for _, service := range p.services { - if !strings.EqualFold(service.id, service.name) { + if !slices.ContainsFunc(KubernetesServiceHostnames(service.id), func(hostname string) bool { return strings.EqualFold(hostname, service.id) }) { // glibc and Go match /etc/hosts case-insensitively common.Logger(ctx).Warnf("Service %s is reachable as %s or localhost, as a Kubernetes hostname cannot be %q", service.id, service.name, service.id) } } @@ -165,7 +177,8 @@ func (p *KubernetesPod) manifest(secret string, secretData map[string]string, me serviceTemplate = templateContainers[index] templateSpec["containers"] = slices.Delete(slices.Clone(templateContainers), index, index+1) } - var containers, hostnames, volumes []any + var containers, volumes []any + var hostnames []string names := map[string]bool{} for _, container := range templateContainers { names[manifestField(container, "name")] = true @@ -184,7 +197,7 @@ func (p *KubernetesPod) manifest(secret string, secretData map[string]string, me continue } containers = append(containers, mergeManifest(serviceTemplate, spec, "")) - hostnames = append(hostnames, c.name) + hostnames = append(hostnames, KubernetesServiceHostnames(c.id)...) } for _, name := range []string{"workspace", "act"} { if !slices.ContainsFunc(templateVolumes, func(volume any) bool { return manifestField(volume, "name") == name }) { diff --git a/act/container/kubernetes_test.go b/act/container/kubernetes_test.go index 36ada2e6..48111066 100644 --- a/act/container/kubernetes_test.go +++ b/act/container/kubernetes_test.go @@ -129,6 +129,19 @@ func serveLocalExec(t *testing.T, w http.ResponseWriter, r *http.Request) { } } +func TestKubernetesServiceHostnames(t *testing.T) { + for id, hostnames := range map[string][]string{ + "db": {"db"}, + "My_DB": {"my-db"}, + "devstoreaccount1.azurite.local": {"devstoreaccount1-azurite-local", "devstoreaccount1.azurite.local"}, + "DB.Local": {"db-local", "db.local"}, + strings.Repeat("a", 64): {strings.Repeat("a", 63), strings.Repeat("a", 64)}, + strings.Repeat("a", 254): {strings.Repeat("a", 63)}, + } { + assert.Equal(t, hostnames, KubernetesServiceHostnames(id), id) + } +} + func TestKubernetesPodRunsJobThroughAPI(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("uses POSIX shell") diff --git a/docs/kubernetes.md b/docs/kubernetes.md index a903391d..adcd31d1 100644 --- a/docs/kubernetes.md +++ b/docs/kubernetes.md @@ -51,7 +51,7 @@ Job pods mount no service account token unless the template sets `automountServi ## Services -Services share the pod's network, so a step reaches one by its id or `localhost`, and two services cannot listen on the same port. An id is lowercased and has characters other than letters, digits and `-` replaced by `-`, so the service `my_db` is reached as `my-db`. Pass secrets to services through `env`, as `command` is visible in the pod spec. Of `options`, only `--health-*` is supported. They become a startup probe with Docker's defaults, which stops a service that fails its retries, so the job fails as on Docker. +Services share the pod's network, so a step reaches one by its id or `localhost`, and two services cannot listen on the same port. An id that is not a valid hostname is sanitized, so `my_db` is reached as `my-db`. Pass secrets to services through `env`, as `command` is visible in the pod spec. Of `options`, only `--health-*` is supported. They become a startup probe with Docker's defaults, which stops a service that fails its retries, so the job fails as on Docker. ## Unsupported diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 30531e3d..5acdca9a 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -501,9 +501,11 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. _ = job.RawServices.Decode(&services) // a whole-value expression names no service before evaluation serviceNames := slices.Sorted(maps.Keys(services)) if strings.HasPrefix(platform, labels.SchemeKubernetes+"://") { - for index, name := range serviceNames { - serviceNames[index] = container.KubernetesServiceName(name) + var hostnames []string + for _, name := range serviceNames { + hostnames = append(hostnames, container.KubernetesServiceHostnames(name)...) } + serviceNames = hostnames } proxyEnv := JobProxyEnv(envs, cacheURL, serviceNames) maps.Copy(envs, proxyEnv)