From a4bba733b61f44d368b487ad13dc460983af4e5e Mon Sep 17 00:00:00 2001 From: Snorre Selmer Date: Wed, 4 Sep 2024 14:48:00 +0200 Subject: [PATCH] Scoped recommended labels to apply to Pod and Service, added unit-tests and integration-tests for both Application and SKIPJob --- api/v1alpha1/application_types.go | 1 - api/v1alpha1/skipjob_types.go | 1 - api/v1alpha1/skipobj_interfaces.go | 23 --------- .../deployment/deployment.go | 1 + pkg/resourcegenerator/job/job.go | 8 +++- .../resourceutils/helpers.go | 23 +++++++++ .../resourceutils/helpers_test.go | 47 +++++++++++++++++++ .../resourceutils/metadata_test.go | 1 - pkg/resourcegenerator/service/service.go | 2 + .../application-assert.yaml | 13 +++++ .../application-patch.yaml | 7 +++ .../labels-imageversion/application.yaml | 7 +++ .../labels-imageversion/chainsaw-test.yaml | 16 +++++++ .../minimal/application-assert.yaml | 6 +++ tests/skipjob/minimal-job/skipjob-assert.yaml | 1 + 15 files changed, 130 insertions(+), 27 deletions(-) create mode 100644 pkg/resourcegenerator/resourceutils/helpers_test.go create mode 100644 tests/application/labels-imageversion/application-assert.yaml create mode 100644 tests/application/labels-imageversion/application-patch.yaml create mode 100644 tests/application/labels-imageversion/application.yaml create mode 100644 tests/application/labels-imageversion/chainsaw-test.yaml diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 98bff6c4..20624f26 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -418,7 +418,6 @@ func (a *Application) SetStatus(status SkiperatorStatus) { func (a *Application) GetDefaultLabels() map[string]string { return map[string]string{ "app.kubernetes.io/name": a.Name, - "app.kubernetes.io/version": getVersionLabel(a.Spec.Image), "app.kubernetes.io/managed-by": "skiperator", "skiperator.kartverket.no/controller": "application", "application.skiperator.no/app": a.Name, diff --git a/api/v1alpha1/skipjob_types.go b/api/v1alpha1/skipjob_types.go index 168fd572..dc05280c 100644 --- a/api/v1alpha1/skipjob_types.go +++ b/api/v1alpha1/skipjob_types.go @@ -269,7 +269,6 @@ func (skipJob *SKIPJob) FillDefaultStatus() { func (skipJob *SKIPJob) GetDefaultLabels() map[string]string { return map[string]string{ "app.kubernetes.io/name": skipJob.Name, - "app.kubernetes.io/version": getVersionLabel(skipJob.Spec.Container.Image), "app.kubernetes.io/managed-by": "skiperator", "skiperator.kartverket.no/controller": "skipjob", // Used by hahaha to know that the Pod should be watched for killing sidecars diff --git a/api/v1alpha1/skipobj_interfaces.go b/api/v1alpha1/skipobj_interfaces.go index f0838553..468aa145 100644 --- a/api/v1alpha1/skipobj_interfaces.go +++ b/api/v1alpha1/skipobj_interfaces.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "sigs.k8s.io/controller-runtime/pkg/client" - "strings" ) type SKIPObject interface { @@ -22,25 +21,3 @@ type CommonSpec struct { AccessPolicy *podtypes.AccessPolicy GCP *podtypes.GCP } - -func getVersionLabel(imageVersionString string) string { - parts := strings.Split(imageVersionString, ":") - - // Implicitly assume version "latest" if no version is specified - if len(parts) < 2 { - return "latest" - } - - versionPart := parts[1] - - // Remove "@sha256" from version text if version includes a hash - if strings.Contains(versionPart, "@") { - versionPart = strings.Split(versionPart, "@")[0] - } - - // Add build number to version if it is specified - if len(parts) > 2 { - return versionPart + "+" + parts[2] - } - return versionPart -} diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index bb0d6199..6e11cd06 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -108,6 +108,7 @@ func Generate(r reconciliation.Reconciliation) error { } else { podTemplateLabels = util.GetPodAppSelector(application.Name) } + podTemplateLabels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(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 e4a4cd5b..933d8b7a 100644 --- a/pkg/resourcegenerator/job/job.go +++ b/pkg/resourcegenerator/job/job.go @@ -6,6 +6,7 @@ import ( "github.com/kartverket/skiperator/pkg/reconciliation" "github.com/kartverket/skiperator/pkg/resourcegenerator/gcp" "github.com/kartverket/skiperator/pkg/resourcegenerator/pod" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/resourcegenerator/volume" "github.com/kartverket/skiperator/pkg/util" batchv1 "k8s.io/api/batch/v1" @@ -27,7 +28,10 @@ func Generate(r reconciliation.Reconciliation) error { meta := metav1.ObjectMeta{ Namespace: skipJob.Namespace, Name: skipJob.Name, - Labels: map[string]string{"app": skipJob.KindPostFixedName()}, + Labels: map[string]string{ + "app": skipJob.KindPostFixedName(), + "app.kubernetes.io/version": resourceutils.GetImageVersion(skipJob.Spec.Container.Image), + }, } job := batchv1.Job{ObjectMeta: meta} cronJob := batchv1.CronJob{ObjectMeta: meta} @@ -72,6 +76,7 @@ func getCronJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelS // it's not a default label, maybe it could be? // used for selecting workloads by netpols, grafana etc spec.JobTemplate.Labels["app"] = skipJob.KindPostFixedName() + spec.JobTemplate.Labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(skipJob.Spec.Container.Image) return spec } @@ -127,6 +132,7 @@ 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 jobSpec.Template.Labels["app"] = skipJob.KindPostFixedName() + jobSpec.Template.Labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(skipJob.Spec.Container.Image) return jobSpec } diff --git a/pkg/resourcegenerator/resourceutils/helpers.go b/pkg/resourcegenerator/resourceutils/helpers.go index d7bd46cf..19f434e1 100644 --- a/pkg/resourcegenerator/resourceutils/helpers.go +++ b/pkg/resourcegenerator/resourceutils/helpers.go @@ -3,6 +3,7 @@ package resourceutils import ( skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "strings" ) func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool { @@ -16,3 +17,25 @@ func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool { } return false } + +func GetImageVersion(imageVersionString string) string { + parts := strings.Split(imageVersionString, ":") + + // Implicitly assume version "latest" if no version is specified + if len(parts) < 2 { + return "latest" + } + + versionPart := parts[1] + + // Remove "@sha256" from version text if version includes a hash + if strings.Contains(versionPart, "@") { + versionPart = strings.Split(versionPart, "@")[0] + } + + // Add build number to version if it is specified + if len(parts) > 2 { + return versionPart + "+" + parts[2] + } + return versionPart +} diff --git a/pkg/resourcegenerator/resourceutils/helpers_test.go b/pkg/resourcegenerator/resourceutils/helpers_test.go new file mode 100644 index 00000000..cc77f659 --- /dev/null +++ b/pkg/resourcegenerator/resourceutils/helpers_test.go @@ -0,0 +1,47 @@ +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) + + assert.Equal(t, expectedImageString, actualImageString) +} diff --git a/pkg/resourcegenerator/resourceutils/metadata_test.go b/pkg/resourcegenerator/resourceutils/metadata_test.go index 639f8173..beaab318 100644 --- a/pkg/resourcegenerator/resourceutils/metadata_test.go +++ b/pkg/resourcegenerator/resourceutils/metadata_test.go @@ -32,7 +32,6 @@ func TestSetResourceLabels(t *testing.T) { expectedLabels := map[string]string{ "app.kubernetes.io/name": "testapp", - "app.kubernetes.io/version": "latest", "app.kubernetes.io/managed-by": "skiperator", "skiperator.kartverket.no/controller": "application", "application.skiperator.no/app": "testapp", diff --git a/pkg/resourcegenerator/service/service.go b/pkg/resourcegenerator/service/service.go index 0649e293..485879f5 100644 --- a/pkg/resourcegenerator/service/service.go +++ b/pkg/resourcegenerator/service/service.go @@ -5,6 +5,7 @@ import ( skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/kartverket/skiperator/pkg/reconciliation" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/util" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,6 +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) 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 new file mode 100644 index 00000000..db229208 --- /dev/null +++ b/tests/application/labels-imageversion/application-assert.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/version: "1234567890123456" + +--- + +apiVersion: v1 +kind: Pod +metadata: + labels: + app.kubernetes.io/version: "1234567890123456" diff --git a/tests/application/labels-imageversion/application-patch.yaml b/tests/application/labels-imageversion/application-patch.yaml new file mode 100644 index 00000000..e892b48c --- /dev/null +++ b/tests/application/labels-imageversion/application-patch.yaml @@ -0,0 +1,7 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: imageversionshalabel +spec: + image: image@sha256:1234567890123456 + port: 8080 \ No newline at end of file diff --git a/tests/application/labels-imageversion/application.yaml b/tests/application/labels-imageversion/application.yaml new file mode 100644 index 00000000..6a8a82d3 --- /dev/null +++ b/tests/application/labels-imageversion/application.yaml @@ -0,0 +1,7 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: imageversionlabellatest +spec: + image: image + port: 8080 diff --git a/tests/application/labels-imageversion/chainsaw-test.yaml b/tests/application/labels-imageversion/chainsaw-test.yaml new file mode 100644 index 00000000..ed7c052a --- /dev/null +++ b/tests/application/labels-imageversion/chainsaw-test.yaml @@ -0,0 +1,16 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: resource-label +spec: + skip: false + concurrent: true + skipDelete: false + steps: + - try: + - create: + file: application.yaml + - apply: + file: application-patch.yaml + - assert: + file: application-assert.yaml diff --git a/tests/application/minimal/application-assert.yaml b/tests/application/minimal/application-assert.yaml index 32094590..0e5f3935 100644 --- a/tests/application/minimal/application-assert.yaml +++ b/tests/application/minimal/application-assert.yaml @@ -5,6 +5,7 @@ metadata: annotations: argocd.argoproj.io/sync-options: "Prune=false" labels: + app.kubernetes.io/name: minimal app.kubernetes.io/managed-by: "skiperator" skiperator.kartverket.no/controller: "application" application.skiperator.no/app: minimal @@ -23,6 +24,7 @@ kind: Deployment metadata: name: minimal labels: + app.kubernetes.io/name: minimal app.kubernetes.io/managed-by: "skiperator" skiperator.kartverket.no/controller: "application" application.skiperator.no/app: minimal @@ -123,6 +125,7 @@ metadata: annotations: argocd.argoproj.io/sync-options: "Prune=false" labels: + app.kubernetes.io/name: minimal app.kubernetes.io/managed-by: "skiperator" skiperator.kartverket.no/controller: "application" application.skiperator.no/app: minimal @@ -156,6 +159,8 @@ metadata: annotations: argocd.argoproj.io/sync-options: "Prune=false" labels: + app.kubernetes.io/name: minimal + app.kubernetes.io/version: latest app.kubernetes.io/managed-by: "skiperator" skiperator.kartverket.no/controller: "application" application.skiperator.no/app: minimal @@ -185,6 +190,7 @@ metadata: annotations: argocd.argoproj.io/sync-options: "Prune=false" labels: + app.kubernetes.io/name: minimal app.kubernetes.io/managed-by: "skiperator" skiperator.kartverket.no/controller: "application" application.skiperator.no/app: minimal diff --git a/tests/skipjob/minimal-job/skipjob-assert.yaml b/tests/skipjob/minimal-job/skipjob-assert.yaml index 4e012cbe..516fd5c4 100644 --- a/tests/skipjob/minimal-job/skipjob-assert.yaml +++ b/tests/skipjob/minimal-job/skipjob-assert.yaml @@ -10,6 +10,7 @@ metadata: labels: skiperator.kartverket.no/skipjob: "true" skiperator.kartverket.no/skipjobName: minimal-job + app.kubernetes.io/version: "5.34.0" app.kubernetes.io/managed-by: "skiperator" skiperator.kartverket.no/controller: "skipjob" spec: