fix(backend/kubernetes): Ensure valid naming of name field (#1661)

- Kubernetes v1.26 on VKE causes error when creating persistent volume
claim because of uppercase characters in name field

This patch is trivial just in order to get it working - happy to
implement differently.

The error in question:

```
The PersistentVolumeClaim "wp-01G1131R63FWBSPMA4ZAZTKLE-0-clone-0" is invalid: metadata.name: Invalid value: "wp-01G1131R63FWBSPMA4ZAZTKLE-0-clone-0": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
```
This commit is contained in:
Neil Hanlon 2023-03-21 15:00:45 -04:00 committed by GitHub
parent 7c56d7246d
commit a95a5b43bf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 101 additions and 39 deletions

View file

@ -115,6 +115,7 @@ pipeline:
from_secret: codecov_token from_secret: codecov_token
when: when:
path: *when_path path: *when_path
failure: ignore
services: services:
service-postgres: service-postgres:

View file

@ -119,8 +119,12 @@ func (e *kube) Setup(ctx context.Context, conf *types.Config) error {
log.Trace().Msgf("Setting up Kubernetes primitives") log.Trace().Msgf("Setting up Kubernetes primitives")
for _, vol := range conf.Volumes { for _, vol := range conf.Volumes {
pvc := PersistentVolumeClaim(e.config.Namespace, vol.Name, e.config.StorageClass, e.config.VolumeSize, e.config.StorageRwx) pvc, err := PersistentVolumeClaim(e.config.Namespace, vol.Name, e.config.StorageClass, e.config.VolumeSize, e.config.StorageRwx)
_, err := e.client.CoreV1().PersistentVolumeClaims(e.config.Namespace).Create(ctx, pvc, metav1.CreateOptions{}) if err != nil {
return err
}
_, err = e.client.CoreV1().PersistentVolumeClaims(e.config.Namespace).Create(ctx, pvc, metav1.CreateOptions{})
if err != nil { if err != nil {
return err return err
} }
@ -131,10 +135,14 @@ func (e *kube) Setup(ctx context.Context, conf *types.Config) error {
for _, stage := range conf.Stages { for _, stage := range conf.Stages {
if stage.Alias == "services" { if stage.Alias == "services" {
for _, step := range stage.Steps { for _, step := range stage.Steps {
log.Trace().Str("pod-name", podName(step)).Msgf("Creating service: %s", step.Name) stepName, err := dnsName(step.Name)
if err != nil {
return err
}
log.Trace().Str("pod-name", stepName).Msgf("Creating service: %s", step.Name)
// TODO: support ports setting // TODO: support ports setting
// svc, err := Service(e.config.Namespace, step.Name, podName(step), step.Ports) // svc, err := Service(e.config.Namespace, step.Name, stepName, step.Ports)
svc, err := Service(e.config.Namespace, step.Name, podName(step), []string{}) svc, err := Service(e.config.Namespace, step.Name, stepName, []string{})
if err != nil { if err != nil {
return err return err
} }
@ -161,16 +169,23 @@ func (e *kube) Setup(ctx context.Context, conf *types.Config) error {
// Start the pipeline step. // Start the pipeline step.
func (e *kube) Exec(ctx context.Context, step *types.Step) error { func (e *kube) Exec(ctx context.Context, step *types.Step) error {
pod := Pod(e.config.Namespace, step, e.config.PodLabels, e.config.PodAnnotations) pod, err := Pod(e.config.Namespace, step, e.config.PodLabels, e.config.PodAnnotations)
if err != nil {
return err
}
log.Trace().Msgf("Creating pod: %s", pod.Name) log.Trace().Msgf("Creating pod: %s", pod.Name)
_, err := e.client.CoreV1().Pods(e.config.Namespace).Create(ctx, pod, metav1.CreateOptions{}) _, err = e.client.CoreV1().Pods(e.config.Namespace).Create(ctx, pod, metav1.CreateOptions{})
return err return err
} }
// Wait for the pipeline step to complete and returns // Wait for the pipeline step to complete and returns
// the completion results. // the completion results.
func (e *kube) Wait(ctx context.Context, step *types.Step) (*types.State, error) { func (e *kube) Wait(ctx context.Context, step *types.Step) (*types.State, error) {
podName := podName(step) podName, err := dnsName(step.Name)
if err != nil {
return nil, err
}
finished := make(chan bool) finished := make(chan bool)
@ -225,7 +240,10 @@ func (e *kube) Wait(ctx context.Context, step *types.Step) (*types.State, error)
// Tail the pipeline step logs. // Tail the pipeline step logs.
func (e *kube) Tail(ctx context.Context, step *types.Step) (io.ReadCloser, error) { func (e *kube) Tail(ctx context.Context, step *types.Step) (io.ReadCloser, error) {
podName := podName(step) podName, err := dnsName(step.Name)
if err != nil {
return nil, err
}
up := make(chan bool) up := make(chan bool)
@ -305,10 +323,14 @@ func (e *kube) Destroy(_ context.Context, conf *types.Config) error {
for _, stage := range conf.Stages { for _, stage := range conf.Stages {
for _, step := range stage.Steps { for _, step := range stage.Steps {
log.Trace().Msgf("Deleting pod: %s", podName(step)) stepName, err := dnsName(step.Name)
if err := e.client.CoreV1().Pods(e.config.Namespace).Delete(noContext, podName(step), deleteOpts); err != nil { if err != nil {
return err
}
log.Trace().Msgf("Deleting pod: %s", stepName)
if err := e.client.CoreV1().Pods(e.config.Namespace).Delete(noContext, stepName, deleteOpts); err != nil {
if errors.IsNotFound(err) { if errors.IsNotFound(err) {
log.Trace().Err(err).Msgf("Unable to delete pod %s", podName(step)) log.Trace().Err(err).Msgf("Unable to delete pod %s", stepName)
} else { } else {
return err return err
} }
@ -338,8 +360,11 @@ func (e *kube) Destroy(_ context.Context, conf *types.Config) error {
} }
for _, vol := range conf.Volumes { for _, vol := range conf.Volumes {
pvc := PersistentVolumeClaim(e.config.Namespace, vol.Name, e.config.StorageClass, e.config.VolumeSize, e.config.StorageRwx) pvc, err := PersistentVolumeClaim(e.config.Namespace, vol.Name, e.config.StorageClass, e.config.VolumeSize, e.config.StorageRwx)
err := e.client.CoreV1().PersistentVolumeClaims(e.config.Namespace).Delete(noContext, pvc.Name, deleteOpts) if err != nil {
return err
}
err = e.client.CoreV1().PersistentVolumeClaims(e.config.Namespace).Delete(noContext, pvc.Name, deleteOpts)
if err != nil { if err != nil {
if errors.IsNotFound(err) { if errors.IsNotFound(err) {
log.Trace().Err(err).Msgf("Unable to delete pvc %s", pvc.Name) log.Trace().Err(err).Msgf("Unable to delete pvc %s", pvc.Name)

View file

@ -10,7 +10,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
func Pod(namespace string, step *types.Step, labels, annotations map[string]string) *v1.Pod { func Pod(namespace string, step *types.Step, labels, annotations map[string]string) (*v1.Pod, error) {
var ( var (
vols []v1.Volume vols []v1.Volume
volMounts []v1.VolumeMount volMounts []v1.VolumeMount
@ -20,18 +20,23 @@ func Pod(namespace string, step *types.Step, labels, annotations map[string]stri
if step.WorkingDir != "" { if step.WorkingDir != "" {
for _, vol := range step.Volumes { for _, vol := range step.Volumes {
volumeName, err := dnsName(strings.Split(vol, ":")[0])
if err != nil {
return nil, err
}
vols = append(vols, v1.Volume{ vols = append(vols, v1.Volume{
Name: volumeName(vol), Name: volumeName,
VolumeSource: v1.VolumeSource{ VolumeSource: v1.VolumeSource{
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
ClaimName: volumeName(vol), ClaimName: volumeName,
ReadOnly: false, ReadOnly: false,
}, },
}, },
}) })
volMounts = append(volMounts, v1.VolumeMount{ volMounts = append(volMounts, v1.VolumeMount{
Name: volumeName(vol), Name: volumeName,
MountPath: volumeMountPath(vol), MountPath: volumeMountPath(vol),
}) })
} }
@ -88,11 +93,16 @@ func Pod(namespace string, step *types.Step, labels, annotations map[string]stri
}, },
} }
labels["step"] = podName(step) podName, err := dnsName(step.Name)
if err != nil {
return nil, err
}
return &v1.Pod{ labels["step"] = podName
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: podName(step), Name: podName,
Namespace: namespace, Namespace: namespace,
Labels: labels, Labels: labels,
Annotations: annotations, Annotations: annotations,
@ -101,7 +111,7 @@ func Pod(namespace string, step *types.Step, labels, annotations map[string]stri
RestartPolicy: v1.RestartPolicyNever, RestartPolicy: v1.RestartPolicyNever,
HostAliases: hostAliases, HostAliases: hostAliases,
Containers: []v1.Container{{ Containers: []v1.Container{{
Name: podName(step), Name: podName,
Image: step.Image, Image: step.Image,
ImagePullPolicy: pullPolicy, ImagePullPolicy: pullPolicy,
Command: entrypoint, Command: entrypoint,
@ -118,10 +128,8 @@ func Pod(namespace string, step *types.Step, labels, annotations map[string]stri
Volumes: vols, Volumes: vols,
}, },
} }
}
func podName(s *types.Step) string { return pod, nil
return dnsName(s.Name)
} }
func mapToEnvVars(m map[string]string) []v1.EnvVar { func mapToEnvVars(m map[string]string) []v1.EnvVar {

View file

@ -22,9 +22,14 @@ func Service(namespace, name, podName string, ports []string) (*v1.Service, erro
}) })
} }
dnsName, err := dnsName(name)
if err != nil {
return nil, err
}
return &v1.Service{ return &v1.Service{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: dnsName(name), Name: dnsName,
Namespace: namespace, Namespace: namespace,
}, },
Spec: v1.ServiceSpec{ Spec: v1.ServiceSpec{

View file

@ -1,7 +1,9 @@
package kubernetes package kubernetes
import ( import (
"errors"
"os" "os"
"regexp"
"strings" "strings"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
@ -10,8 +12,19 @@ import (
"k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd"
) )
func dnsName(i string) string { var (
return strings.Replace(i, "_", "-", -1) dnsPattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
ErrDNSPatternInvalid = errors.New("name is not a valid kubernetes DNS name")
)
func dnsName(i string) (string, error) {
res := strings.Replace(i, "_", "-", -1)
if found := dnsPattern.FindStringIndex(res); found == nil {
return "", ErrDNSPatternInvalid
}
return res, nil
} }
func isImagePullBackOffState(pod *v1.Pod) bool { func isImagePullBackOffState(pod *v1.Pod) bool {

View file

@ -8,7 +8,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
func PersistentVolumeClaim(namespace, name, storageClass, size string, storageRwx bool) *v1.PersistentVolumeClaim { func PersistentVolumeClaim(namespace, name, storageClass, size string, storageRwx bool) (*v1.PersistentVolumeClaim, error) {
_storageClass := &storageClass _storageClass := &storageClass
if storageClass == "" { if storageClass == "" {
_storageClass = nil _storageClass = nil
@ -22,9 +22,14 @@ func PersistentVolumeClaim(namespace, name, storageClass, size string, storageRw
accessMode = v1.ReadWriteOnce accessMode = v1.ReadWriteOnce
} }
return &v1.PersistentVolumeClaim{ volumeName, err := dnsName(strings.Split(name, ":")[0])
if err != nil {
return nil, err
}
pvc := &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: volumeName(name), Name: volumeName,
Namespace: namespace, Namespace: namespace,
}, },
Spec: v1.PersistentVolumeClaimSpec{ Spec: v1.PersistentVolumeClaimSpec{
@ -37,8 +42,6 @@ func PersistentVolumeClaim(namespace, name, storageClass, size string, storageRw
}, },
}, },
} }
}
func volumeName(i string) string { return pvc, nil
return dnsName(strings.Split(i, ":")[0])
} }

View file

@ -11,7 +11,7 @@ func TestPersistentVolumeClaim(t *testing.T) {
expectedRwx := ` expectedRwx := `
{ {
"metadata": { "metadata": {
"name": "someName", "name": "somename",
"namespace": "someNamespace", "namespace": "someNamespace",
"creationTimestamp": null "creationTimestamp": null
}, },
@ -32,7 +32,7 @@ func TestPersistentVolumeClaim(t *testing.T) {
expectedRwo := ` expectedRwo := `
{ {
"metadata": { "metadata": {
"name": "someName", "name": "somename",
"namespace": "someNamespace", "namespace": "someNamespace",
"creationTimestamp": null "creationTimestamp": null
}, },
@ -50,13 +50,20 @@ func TestPersistentVolumeClaim(t *testing.T) {
"status": {} "status": {}
}` }`
pvc := PersistentVolumeClaim("someNamespace", "someName", "local-storage", "1Gi", true) pvc, err := PersistentVolumeClaim("someNamespace", "somename", "local-storage", "1Gi", true)
assert.Nil(t, err)
j, err := json.Marshal(pvc) j, err := json.Marshal(pvc)
assert.Nil(t, err) assert.Nil(t, err)
assert.JSONEq(t, expectedRwx, string(j)) assert.JSONEq(t, expectedRwx, string(j))
pvc = PersistentVolumeClaim("someNamespace", "someName", "local-storage", "1Gi", false) pvc, err = PersistentVolumeClaim("someNamespace", "somename", "local-storage", "1Gi", false)
assert.Nil(t, err)
j, err = json.Marshal(pvc) j, err = json.Marshal(pvc)
assert.Nil(t, err) assert.Nil(t, err)
assert.JSONEq(t, expectedRwo, string(j)) assert.JSONEq(t, expectedRwo, string(j))
_, err = PersistentVolumeClaim("someNamespace", "some0INVALID3name", "local-storage", "1Gi", false)
assert.NotNil(t, err)
} }

View file

@ -280,7 +280,7 @@ func (b *StepBuilder) toInternalRepresentation(parsed *yaml.Config, environ map[
compiler.WithPrefix( compiler.WithPrefix(
fmt.Sprintf( fmt.Sprintf(
"wp_%s_%d", "wp_%s_%d",
ulid.Make().String(), strings.ToLower(ulid.Make().String()),
stepID, stepID,
), ),
), ),