Skip to content

Commit

Permalink
- More label test-cases, finally all of them pass.
Browse files Browse the repository at this point in the history
- Refactored function-name to better reflect what it does
  • Loading branch information
SnorreSelmer committed Nov 8, 2024
1 parent 63f32df commit c27959e
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 30 deletions.
2 changes: 1 addition & 1 deletion pkg/resourcegenerator/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,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(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
Expand Down
2 changes: 1 addition & 1 deletion pkg/resourcegenerator/job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,5 @@ func getJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelec

func setJobLabels(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(skipJob.Spec.Container.Image)
}
53 changes: 29 additions & 24 deletions pkg/resourcegenerator/resourceutils/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import (
"strings"
)

const LabelValueMaxLength int = 63
func matchesRegex(s string, pattern string) bool {
obj, err := regexp.Match(pattern, []byte(s))
return obj && err == nil
}

func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool {
replicas, err := skiperatorv1alpha1.GetStaticReplicas(jsonReplicas)
Expand All @@ -21,42 +24,44 @@ func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool {
return false
}

// MatchesRegex checks if a string matches a regexp
func matchesRegex(s string, pattern string) bool {
obj, err := regexp.Match(pattern, []byte(s))
return obj && err == nil
}
// HumanReadableVersion returns the version part of an image string
func HumanReadableVersion(imageReference string) string {
const LabelValueMaxLength = 63

var allowedChars = regexp.MustCompile(`[A-Za-z0-9_.-]`)

// GetImageVersion returns the version part of an image string
func GetImageVersion(imageVersionString string) string {
// Find position of first "@", remove it and everything after it
if strings.Contains(imageVersionString, "@") {
imageVersionString = strings.Split(imageVersionString, "@")[0]
imageVersionString = imageVersionString + ":unknown"
if strings.Contains(imageReference, "@") {
imageReference = strings.Split(imageReference, "@")[0]
}

// If no version is given, assume "latest"
if !strings.Contains(imageVersionString, ":") {
lastColonPos := strings.LastIndex(imageReference, ":")
if lastColonPos == -1 || lastColonPos == len(imageReference)-1 {
return "latest"
}
versionPart := imageReference[lastColonPos+1:]
imageReference = imageReference[:lastColonPos]

// Split image string into parts
parts := strings.Split(imageVersionString, ":")

versionPart := parts[1]
// While first character is not part of regex [a-z0-9A-Z] then remove it
for len(versionPart) > 0 && !matchesRegex(versionPart[:1], "[a-zA-Z0-9]") {
versionPart = versionPart[1:]
}

// Replace "+" with "-" in version text if version includes one
versionPart = strings.ReplaceAll(versionPart, "+", "-")
// For each character in versionPart, replace characters that are not allowed in label-value
var result strings.Builder
for _, c := range versionPart {
if allowedChars.MatchString(string(c)) {
result.WriteRune(c)
} else {
result.WriteRune('-')
}
}
versionPart = result.String()

// Limit label-value to 63 characters
if len(versionPart) > LabelValueMaxLength {
versionPart = versionPart[:LabelValueMaxLength]
}

// While first character is not part of regex [a-z0-9A-Z] then remove it
for len(versionPart) > 0 && !MatchesRegex(versionPart[:1], "[a-zA-Z0-9]") {
versionPart = versionPart[1:]
}

return versionPart
}
19 changes: 16 additions & 3 deletions pkg/resourcegenerator/resourceutils/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,38 @@ func TestVersions(t *testing.T) {
{"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", "unknown"},
{"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", "unknown"},
{"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"},
}

for _, tc := range testCases {
t.Run(tc.imageString, func(t *testing.T) {
actualValue := GetImageVersion(tc.imageString)
actualValue := HumanReadableVersion(tc.imageString)
assert.Equal(t, tc.expectedValue, actualValue)
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/resourcegenerator/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,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(application.Spec.Image)

ports := append(getAdditionalPorts(application.Spec.AdditionalPorts), getServicePort(application.Spec.Port, application.Spec.AppProtocol))
if r.IsIstioEnabled() {
Expand Down

0 comments on commit c27959e

Please sign in to comment.