diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index 6e11cd06..7682fbad 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -3,13 +3,14 @@ package deployment import ( goerrors "errors" "fmt" + "strings" + "github.com/kartverket/skiperator/pkg/reconciliation" "github.com/kartverket/skiperator/pkg/resourcegenerator/idporten" "github.com/kartverket/skiperator/pkg/resourcegenerator/maskinporten" "github.com/kartverket/skiperator/pkg/resourcegenerator/pod" "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/resourcegenerator/volume" - "strings" "github.com/go-logr/logr" skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" @@ -108,7 +109,7 @@ func Generate(r reconciliation.Reconciliation) error { } else { podTemplateLabels = util.GetPodAppSelector(application.Name) } - podTemplateLabels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(application.Spec.Image) + podTemplateLabels["app.kubernetes.io/version"] = resourceutils.HumanReadableVersion(&ctxLog, application.Spec.Image) // Add annotations to pod template, safe-to-evict added due to issues // with cluster-autoscaler and unable to evict pods with local volumes diff --git a/pkg/resourcegenerator/job/job.go b/pkg/resourcegenerator/job/job.go index bb9c6921..56098361 100644 --- a/pkg/resourcegenerator/job/job.go +++ b/pkg/resourcegenerator/job/job.go @@ -2,7 +2,9 @@ package job import ( "fmt" + skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" + "github.com/kartverket/skiperator/pkg/log" "github.com/kartverket/skiperator/pkg/reconciliation" "github.com/kartverket/skiperator/pkg/resourcegenerator/gcp" "github.com/kartverket/skiperator/pkg/resourcegenerator/pod" @@ -31,7 +33,7 @@ func Generate(r reconciliation.Reconciliation) error { Labels: make(map[string]string), } - setJobLabels(skipJob, meta.Labels) + setJobLabels(&ctxLog, skipJob, meta.Labels) job := batchv1.Job{ObjectMeta: meta} cronJob := batchv1.CronJob{ObjectMeta: meta} @@ -47,17 +49,17 @@ func Generate(r reconciliation.Reconciliation) error { } if skipJob.Spec.Cron != nil { - cronJob.Spec = getCronJobSpec(skipJob, cronJob.Spec.JobTemplate.Spec.Selector, cronJob.Spec.JobTemplate.Spec.Template.Labels, r.GetIdentityConfigMap()) + cronJob.Spec = getCronJobSpec(&ctxLog, skipJob, cronJob.Spec.JobTemplate.Spec.Selector, cronJob.Spec.JobTemplate.Spec.Template.Labels, r.GetIdentityConfigMap()) r.AddResource(&cronJob) } else { - job.Spec = getJobSpec(skipJob, job.Spec.Selector, job.Spec.Template.Labels, r.GetIdentityConfigMap()) + job.Spec = getJobSpec(&ctxLog, skipJob, job.Spec.Selector, job.Spec.Template.Labels, r.GetIdentityConfigMap()) r.AddResource(&job) } return nil } -func getCronJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelector, podLabels map[string]string, gcpIdentityConfigMap *corev1.ConfigMap) batchv1.CronJobSpec { +func getCronJobSpec(logger *log.Logger, skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelector, podLabels map[string]string, gcpIdentityConfigMap *corev1.ConfigMap) batchv1.CronJobSpec { spec := batchv1.CronJobSpec{ Schedule: skipJob.Spec.Cron.Schedule, TimeZone: skipJob.Spec.Cron.TimeZone, @@ -68,19 +70,19 @@ func getCronJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelS ObjectMeta: metav1.ObjectMeta{ Labels: skipJob.GetDefaultLabels(), }, - Spec: getJobSpec(skipJob, selector, podLabels, gcpIdentityConfigMap), + Spec: getJobSpec(logger, skipJob, selector, podLabels, gcpIdentityConfigMap), }, SuccessfulJobsHistoryLimit: util.PointTo(int32(3)), FailedJobsHistoryLimit: util.PointTo(int32(1)), } // it's not a default label, maybe it could be? // used for selecting workloads by netpols, grafana etc - setJobLabels(skipJob, spec.JobTemplate.Labels) + setJobLabels(logger, skipJob, spec.JobTemplate.Labels) return spec } -func getJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelector, podLabels map[string]string, gcpIdentityConfigMap *corev1.ConfigMap) batchv1.JobSpec { +func getJobSpec(logger *log.Logger, skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelector, podLabels map[string]string, gcpIdentityConfigMap *corev1.ConfigMap) batchv1.JobSpec { podVolumes, containerVolumeMounts := volume.GetContainerVolumeMountsAndPodVolumes(skipJob.Spec.Container.FilesFrom) envVars := skipJob.Spec.Container.Env @@ -131,19 +133,12 @@ func getJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelec // it's not a default label, maybe it could be? // used for selecting workloads by netpols, grafana etc - setJobLabels(skipJob, jobSpec.Template.Labels) + setJobLabels(logger, skipJob, jobSpec.Template.Labels) return jobSpec } -//func getSkipJobVersion(skipJob *skiperatorv1alpha1.SKIPJob) string { -// if skipJob.Spec.Container.Image != "" { -// return resourceutils.GetImageVersion(skipJob.Spec.Container.Image) -// } -// return "" -//} - -func setJobLabels(skipJob *skiperatorv1alpha1.SKIPJob, labels map[string]string) { +func setJobLabels(logger *log.Logger, skipJob *skiperatorv1alpha1.SKIPJob, labels map[string]string) { labels["app"] = skipJob.KindPostFixedName() - labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(skipJob.Spec.Container.Image) + labels["app.kubernetes.io/version"] = resourceutils.HumanReadableVersion(logger, skipJob.Spec.Container.Image) } diff --git a/pkg/resourcegenerator/resourceutils/helpers.go b/pkg/resourcegenerator/resourceutils/helpers.go index 19f434e1..e931e048 100644 --- a/pkg/resourcegenerator/resourceutils/helpers.go +++ b/pkg/resourcegenerator/resourceutils/helpers.go @@ -1,9 +1,23 @@ package resourceutils import ( + "regexp" + "strings" + skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" + "github.com/kartverket/skiperator/pkg/log" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "strings" +) + +const ( + LabelValueMaxLength = 63 + defaultImageVersion = "latest" + unknownImageVersion = "err-unknown" +) + +var ( + allowedChars = regexp.MustCompile(`[A-Za-z0-9_.-]`) + allowedPrefixCharacters = regexp.MustCompile(`[a-zA-Z0-9]`) ) func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool { @@ -18,24 +32,54 @@ func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool { return false } -func GetImageVersion(imageVersionString string) string { - parts := strings.Split(imageVersionString, ":") +// HumanReadableVersion parses an image reference (e.g. "ghcr.io/org/some-team/some-app:v1.2.3") +// and returns a human-readable version string. If the image reference is empty, it returns a default +// unknown version string. The function removes any digest part (everything after and including "@") +// from the image reference, extracts the version part (everything after the last ":"), trims non-alphanumeric +// prefix characters from the version part, replaces any characters not allowed in Kubernetes label values +// with a hyphen ("-"), and limits the version string to a maximum length of 63 characters. If the version +// part is not found, it returns a default version string. +func HumanReadableVersion(logger *log.Logger, imageReference string) string { + logz := (*logger).GetLogger().WithValues("helper", "HumanReadableVersion") - // Implicitly assume version "latest" if no version is specified - if len(parts) < 2 { - return "latest" + if len(imageReference) == 0 { + logz.Info("imageReference had length 0") + return unknownImageVersion } - versionPart := parts[1] + processedImageRef := strings.Clone(imageReference) - // Remove "@sha256" from version text if version includes a hash - if strings.Contains(versionPart, "@") { - versionPart = strings.Split(versionPart, "@")[0] + // Find position of first "@", remove it and everything after it + if strings.Contains(processedImageRef, "@") { + processedImageRef = strings.Split(processedImageRef, "@")[0] } - // Add build number to version if it is specified - if len(parts) > 2 { - return versionPart + "+" + parts[2] + lastColonPos := strings.LastIndex(processedImageRef, ":") + if lastColonPos == -1 || lastColonPos == len(processedImageRef)-1 { + return defaultImageVersion } + + versionPart := processedImageRef[lastColonPos+1:] + processedImageRef = processedImageRef[:lastColonPos] + + // Trim non-alphanumeric prefix + versionPart = strings.TrimLeftFunc(versionPart, func(r rune) bool { + return !allowedPrefixCharacters.MatchString(string(r)) + }) + + // For each character in versionPart, replace characters that are not allowed in label-value + versionPart = strings.Map(func(r rune) rune { + if allowedChars.MatchString(string(r)) { + return r + } + return '-' + }, versionPart) + + // Limit label-value to 63 characters + if len(versionPart) > LabelValueMaxLength { + versionPart = versionPart[:LabelValueMaxLength] + logz.Info("Trimming version length because it too long", "ref", imageReference, "trimmedVersion", versionPart) + } + return versionPart } diff --git a/pkg/resourcegenerator/resourceutils/helpers_test.go b/pkg/resourcegenerator/resourceutils/helpers_test.go index cc77f659..d8512840 100644 --- a/pkg/resourcegenerator/resourceutils/helpers_test.go +++ b/pkg/resourcegenerator/resourceutils/helpers_test.go @@ -1,47 +1,56 @@ package resourceutils import ( - "github.com/stretchr/testify/assert" "testing" -) - -func TestGetImageVersionNoTag(t *testing.T) { - imageString := "image" - expectedImageString := "latest" - - actualImageString := GetImageVersion(imageString) - - assert.Equal(t, expectedImageString, actualImageString) -} - -func TestGetImageVersionLatestTag(t *testing.T) { - imageString := "image:latest" - expectedImageString := "latest" - - actualImageString := GetImageVersion(imageString) - - assert.Equal(t, expectedImageString, actualImageString) -} - -func TestGetImageVersionVersionTag(t *testing.T) { - versionImageString := "image:1.2.3" - devImageString := "image:1.2.3-dev-123abc" - expectedVersionImageString := "1.2.3" - expectedDevImageString := "1.2.3-dev-123abc" - - actualVersionImageString := GetImageVersion(versionImageString) - actualDevImageString := GetImageVersion(devImageString) - assert.Equal(t, expectedVersionImageString, actualVersionImageString) - assert.Equal(t, expectedDevImageString, actualDevImageString) - -} - -func TestGetImageVersionShaTag(t *testing.T) { - imageString := "ghcr.io/org/repo@sha256:54d7ea8b48d0e7569766e0e10b9e38da778a5f65d764168dd7db76a37d6b8" - expectedImageString := "54d7ea8b48d0e7569766e0e10b9e38da778a5f65d764168dd7db76a37d6b8" - - actualImageString := GetImageVersion(imageString) + "github.com/kartverket/skiperator/pkg/log" + "github.com/stretchr/testify/assert" +) - assert.Equal(t, expectedImageString, actualImageString) +func TestVersions(t *testing.T) { + testCases := []struct { + imageString string + expectedValue string + }{ + {"image", "latest"}, + {"image:latest", "latest"}, + {"image:1.2.3-dev-123abc", "1.2.3-dev-123abc"}, + {"image:1.2.3", "1.2.3"}, + {"ghcr.io/org/repo@sha256:54d7ea8b48d0e7569766e0e10b9e38da778a5f65d764168dd7db76a37d6b8", "latest"}, + {"ghcr.io/org/one-app:sha-b15dc91c27ad2387bea81294593d5ce5a686bcc4@sha256:3cda54f1d25458f25fdde0398130da57a4ebb4a4cd759bc49035b7ebf9d83619", "sha-b15dc91c27ad2387bea81294593d5ce5a686bcc4"}, + {"ghcr.io/org/another-app:3fb7048", "3fb7048"}, + {"ghcr.io/org/some-team/third-app:v1.2.54", "v1.2.54"}, + {"ghcr.io/org/another-team/fourth-app:4.0.0.rc-36", "4.0.0.rc-36"}, + {"ghcr.io/org/another-team/fifth-app:4.0.0.rc-36-master-latest", "4.0.0.rc-36-master-latest"}, + {"ghcr.io/kartverket/vulnerability-disclosure-program@sha256:ab85022d117168585bdedc71cf9c67c3ca327533dc7cd2c5bcc42a83f308ea5d", "latest"}, + {"ghcr.io/kartverket/vulnerability-disclosure-program:4.0.1@sha256:ab85022d117168585bdedc71cf9c67c3ca327533dc7cd2c5bcc42a83f308ea5d", "4.0.1"}, + {"nginxinc/nginx-unprivileged:1.20.0-alpine", "1.20.0-alpine"}, + {"foo/bar:1.2.3+build.4", "1.2.3-build.4"}, + {"foo/bar:1.2.3+somethingLongXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", "1.2.3-somethingLongXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"}, + {"foo/bar:.1.2.3+somethingLongXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXA", "1.2.3-somethingLongXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXA"}, + {"foo/bar:-1.2.3", "1.2.3"}, + {"foo/bar:__1.2.3", "1.2.3"}, + {"foo/bar:.1.2.3", "1.2.3"}, + {"foo/bar@sha256:3cda54f1d25458f25fdde0398130da57a4ebb4a4cd759bc49035b7ebf9d83619", "latest"}, + {"foo/bar:latest@sha256:3cda54f1d25458f25fdde0398130da57a4ebb4a4cd759bc49035b7ebf9d83619", "latest"}, + {"foo/bar:stable@sha256:3cda54f1d25458f25fdde0398130da57a4ebb4a4cd759bc49035b7ebf9d83619", "stable"}, + {"foo/bar:unknown@sha256:3cda54f1d25458f25fdde0398130da57a4ebb4a4cd759bc49035b7ebf9d83619", "unknown"}, + {"foo/bar:1.2.3@sha256:3cda54f1d25458f25fdde0398130da57a4ebb4a4cd759bc49035b7ebf9d83619", "1.2.3"}, + {"foo/bar:1.2.3%suffix", "1.2.3-suffix"}, + {"foo/bar:1.2.3*suffix", "1.2.3-suffix"}, + {"foo/bar:1.2.3#suffix", "1.2.3-suffix"}, + {"foo/bar:1.2.3$suffix", "1.2.3-suffix"}, + {"foo/bar:1.2.3–suffix", "1.2.3-suffix"}, + {"foo/bar:1.2.3-suffix", "1.2.3-suffix"}, + {"registry:5000/foo/bar:1.2.3", "1.2.3"}, + } + + logger := log.NewLogger() + + for _, tc := range testCases { + t.Run(tc.imageString, func(t *testing.T) { + actualValue := HumanReadableVersion(&logger, tc.imageString) + assert.Equal(t, tc.expectedValue, actualValue) + }) + } } diff --git a/pkg/resourcegenerator/service/service.go b/pkg/resourcegenerator/service/service.go index 485879f5..261d8184 100644 --- a/pkg/resourcegenerator/service/service.go +++ b/pkg/resourcegenerator/service/service.go @@ -2,6 +2,8 @@ package service import ( "fmt" + "strings" + skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/kartverket/skiperator/pkg/reconciliation" @@ -10,7 +12,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "strings" ) const defaultPortName = "http" @@ -37,7 +38,7 @@ func Generate(r reconciliation.Reconciliation) error { service := corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: application.Name}} service.Labels = util.GetPodAppSelector(application.Name) - service.Labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(application.Spec.Image) + service.Labels["app.kubernetes.io/version"] = resourceutils.HumanReadableVersion(&ctxLog, application.Spec.Image) ports := append(getAdditionalPorts(application.Spec.AdditionalPorts), getServicePort(application.Spec.Port, application.Spec.AppProtocol)) if r.IsIstioEnabled() { diff --git a/tests/application/labels-imageversion/application-assert.yaml b/tests/application/labels-imageversion/application-assert.yaml index db229208..adde42db 100644 --- a/tests/application/labels-imageversion/application-assert.yaml +++ b/tests/application/labels-imageversion/application-assert.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Service metadata: labels: - app.kubernetes.io/version: "1234567890123456" + app.kubernetes.io/version: "latest" --- @@ -10,4 +10,4 @@ apiVersion: v1 kind: Pod metadata: labels: - app.kubernetes.io/version: "1234567890123456" + app.kubernetes.io/version: latest diff --git a/tests/application/labels-imageversion/application-patch.yaml b/tests/application/labels-imageversion/application-patch.yaml index 0b20ae9a..abc5cd75 100644 --- a/tests/application/labels-imageversion/application-patch.yaml +++ b/tests/application/labels-imageversion/application-patch.yaml @@ -1,7 +1,7 @@ apiVersion: skiperator.kartverket.no/v1alpha1 kind: Application metadata: - name: imageversionshalabel + name: imageversionlabelsha spec: image: image@sha256:1234567890123456 port: 8080