From c1443f63a2d0b765f385eada269e8588f10f974c Mon Sep 17 00:00:00 2001 From: Mohammad Abdolirad Date: Tue, 13 Oct 2026 14:47:34 +0200 Subject: [PATCH] tighten k8s tests - cover pod queries, certificate reuse, and job cleanup failures - reject bad selectors before listing, and name version parse cases --- internal/k8s/components_test.go | 166 +++++++++++++++ internal/k8s/events_test.go | 142 ++++++++++++- internal/k8s/jobs_test.go | 351 ++++++++++++++++++++++++++++++-- internal/k8s/pods_test.go | 251 +++++++++++++++++++++++ internal/k8s/selectors_test.go | 152 ++++++++------ internal/k8s/tlscert_test.go | 147 ++++++++++++- internal/k8s/version_test.go | 103 +++++++--- 7 files changed, 1205 insertions(+), 107 deletions(-) create mode 100644 internal/k8s/pods_test.go diff --git a/internal/k8s/components_test.go b/internal/k8s/components_test.go index 0d25ec3..b1cbc40 100644 --- a/internal/k8s/components_test.go +++ b/internal/k8s/components_test.go @@ -14,16 +14,19 @@ package k8s import ( + "errors" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/spec" corev1 "k8s.io/api/core/v1" + k8stesting "k8s.io/client-go/testing" ) func TestGetAvailableEnvironments_DistinguishesWildcardInstances(t *testing.T) { @@ -52,8 +55,171 @@ func TestGetAvailableEnvironments_DistinguishesWildcardInstances(t *testing.T) { pod("pr-456", "api", "review", helm.GenerateReleaseName("shop", "review/pr-456"), "review/pr-456"), pod("web-pr", "web", "review", helm.GenerateReleaseName("shop", "review/pr-789"), "review/pr-789"), ) + var field string + cs.PrependReactor("list", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { + list, ok := action.(k8stesting.ListAction) + if ok { + field = list.GetListRestrictions().Fields.String() + } + return false, nil, nil + }) client := NewClient(cs, "default") got, err := client.GetAvailableEnvironments(t.Context(), "shop", "api") require.NoError(t, err) assert.Equal(t, []string{"review", "review/pr-123", "review/pr-456"}, got) + // The fake client does not apply field selectors. Running phase is requested. + assert.Equal(t, "status.phase=Running", field) +} + +func TestGetAvailableComponents(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + componentPod("api-1", "apps", "shop", "api"), + componentPod("api-2", "apps", "shop", "api"), + componentPod("web", "apps", "shop", "web"), + componentPod("unlabeled", "apps", "shop", ""), + componentPod("other-project", "apps", "billing", "api"), + componentPod("other-ns", "other", "shop", "worker"), + ) + var field string + cs.PrependReactor("list", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { + list, ok := action.(k8stesting.ListAction) + if ok { + field = list.GetListRestrictions().Fields.String() + } + return false, nil, nil + }) + + got, err := NewClient(cs, "apps").GetAvailableComponents(t.Context(), "shop") + require.NoError(t, err) + // Names come from a map, so the order is not part of the contract. + assert.ElementsMatch(t, []string{"api", "web"}, got) + assert.Equal(t, "status.phase=Running", field) +} + +func TestGetAvailableComponents_InvalidProject(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + listed := false + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + listed = true + return true, nil, errors.New("should not list") + }) + _, err := NewClient(cs, "apps").GetAvailableComponents(t.Context(), "bad value!") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to build project selector") + assert.False(t, listed) +} + +func TestGetAvailableComponents_ListError(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("apiserver timeout") + }) + _, err := NewClient(cs, "apps").GetAvailableComponents(t.Context(), "shop") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to list pods") + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestValidateComponentExists(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset(componentPod("api-1", "apps", "shop", "api")) + client := NewClient(cs, "apps") + require.NoError(t, client.ValidateComponentExists(t.Context(), "shop", "api")) + + err := client.ValidateComponentExists(t.Context(), "shop", "web") + require.Error(t, err) + assert.ErrorContains(t, err, "component 'web' not found or has no running pods in project 'shop'") +} + +func TestValidateComponentExists_ListError(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("apiserver timeout") + }) + err := NewClient(cs, "apps").ValidateComponentExists(t.Context(), "shop", "api") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to validate component 'api'") + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestGetAvailableEnvironments_SkipsBlankAnnotations(t *testing.T) { + t.Parallel() + + pod := func(name, component string, annotations map[string]string) *corev1.Pod { + return &corev1.Pod{ + Name: name, + Namespace: "apps", + Labels: map[string]string{ + ProjectLabel: "shop", + ComponentLabel: component, + }, + Annotations: annotations, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + } + } + env := func(original string) map[string]string { + return map[string]string{spec.AnnotationEnvironmentInstance: original} + } + cs := fake.NewSimpleClientset( + pod("missing", "api", nil), + pod("blank", "api", env(" ")), + pod("dev-a", "api", env("dev")), + pod("dev-b", "api", env("dev")), + pod("staging", "api", env("staging")), + pod("other-component", "web", env("prod")), + ) + got, err := NewClient(cs, "apps").GetAvailableEnvironments(t.Context(), "shop", "api") + require.NoError(t, err) + assert.Equal(t, []string{"dev", "staging"}, got) +} + +func TestGetAvailableEnvironments_ListError(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("apiserver timeout") + }) + _, err := NewClient(cs, "apps").GetAvailableEnvironments(t.Context(), "shop", "api") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to list pods") + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestGetAvailableEnvironments_InvalidComponent(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + listed := false + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + listed = true + return true, nil, errors.New("should not list") + }) + _, err := NewClient(cs, "apps").GetAvailableEnvironments(t.Context(), "shop", "bad value!") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to build component selector") + assert.False(t, listed) +} + +// componentPod builds a running pod. An empty component omits the label. +func componentPod(name, namespace, project, component string) *corev1.Pod { + labels := map[string]string{ProjectLabel: project} + if component != "" { + labels[ComponentLabel] = component + } + return &corev1.Pod{ + Name: name, + Namespace: namespace, + Labels: labels, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + } } diff --git a/internal/k8s/events_test.go b/internal/k8s/events_test.go index 1c27fed..8f19f28 100644 --- a/internal/k8s/events_test.go +++ b/internal/k8s/events_test.go @@ -16,6 +16,8 @@ package k8s import ( "context" + "errors" + "net/http" "testing" "time" @@ -83,6 +85,8 @@ func sendAndDrain(t *testing.T, fw *watch.FakeWatcher, ch <-chan DeployEvent, ev // whose involved object name starts with the release prefix are // forwarded to the channel. func TestWatchDeployEvents_MatchingPrefixForwarded(t *testing.T) { + t.Parallel() + fw := watch.NewFake() cs := fake.NewClientset() // Replace the watch reactor to return our controlled fake watcher. @@ -112,6 +116,8 @@ func TestWatchDeployEvents_MatchingPrefixForwarded(t *testing.T) { // TestWatchDeployEvents_NonMatchingPrefixFiltered verifies that // events for a different release are not forwarded. func TestWatchDeployEvents_NonMatchingPrefixFiltered(t *testing.T) { + t.Parallel() + fw := watch.NewFake() cs := fake.NewClientset() cs.PrependWatchReactor("events", func(action clienttesting.Action) (bool, watch.Interface, error) { @@ -138,6 +144,8 @@ func TestWatchDeployEvents_NonMatchingPrefixFiltered(t *testing.T) { // TestWatchDeployEvents_ModifiedEventForwarded verifies that // MODIFIED watch events are forwarded with updated fields. func TestWatchDeployEvents_ModifiedEventForwarded(t *testing.T) { + t.Parallel() + fw := watch.NewFake() cs := fake.NewClientset() cs.PrependWatchReactor("events", func(action clienttesting.Action) (bool, watch.Interface, error) { @@ -153,7 +161,8 @@ func TestWatchDeployEvents_ModifiedEventForwarded(t *testing.T) { ev := makeEvent("uid-mod", testRelease+"-worker-abc", "BackOff", "back-off restarting", corev1.EventTypeWarning, "101") ev.Count = 3 - got, _ := sendAndDrain(t, fw, ch, watch.Modified, ev) + got, ok := sendAndDrain(t, fw, ch, watch.Modified, ev) + require.True(t, ok) assert.Equal(t, types.UID("uid-mod"), got.UID) assert.Equal(t, "Warning", got.Type) assert.Equal(t, int32(3), got.Count) @@ -162,6 +171,8 @@ func TestWatchDeployEvents_ModifiedEventForwarded(t *testing.T) { // TestWatchDeployEvents_ChannelClosedOnContextCancel verifies that // the event channel closes promptly after context cancellation. func TestWatchDeployEvents_ChannelClosedOnContextCancel(t *testing.T) { + t.Parallel() + fw := watch.NewFake() cs := fake.NewClientset() cs.PrependWatchReactor("events", func(action clienttesting.Action) (bool, watch.Interface, error) { @@ -187,6 +198,8 @@ func TestWatchDeployEvents_ChannelClosedOnContextCancel(t *testing.T) { // TestWatchDeployEvents_FieldTranslation verifies that core event // fields are correctly translated into DeployEvent fields. func TestWatchDeployEvents_FieldTranslation(t *testing.T) { + t.Parallel() + fw := watch.NewFake() cs := fake.NewClientset() cs.PrependWatchReactor("events", func(action clienttesting.Action) (bool, watch.Interface, error) { @@ -230,6 +243,8 @@ func TestWatchDeployEvents_FieldTranslation(t *testing.T) { // TestToDeployEvent_ZeroCount verifies that a zero Count is normalized to 1. func TestToDeployEvent_ZeroCount(t *testing.T) { + t.Parallel() + ev := watch.Event{ Type: watch.Added, Object: &corev1.Event{ @@ -250,6 +265,8 @@ func TestToDeployEvent_ZeroCount(t *testing.T) { // TestToDeployEvent_NonEvent verifies that non-Event objects return false. func TestToDeployEvent_NonEvent(t *testing.T) { + t.Parallel() + ev := watch.Event{ Type: watch.Added, Object: &corev1.Pod{}, @@ -261,6 +278,8 @@ func TestToDeployEvent_NonEvent(t *testing.T) { // TestToDeployEvent_TimestampFallbacks verifies EventTime and // CreationTimestamp fallbacks when LastTimestamp is zero. func TestToDeployEvent_TimestampFallbacks(t *testing.T) { + t.Parallel() + eventTime := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) ev := watch.Event{ Type: watch.Added, @@ -283,5 +302,122 @@ func TestToDeployEvent_TimestampFallbacks(t *testing.T) { assert.Equal(t, eventTime.UTC(), got.Timestamp.UTC()) } -// Ensure fake.Clientset satisfies runtime.Object so the test compiles. -var _ runtime.Object = (*corev1.Event)(nil) +func TestToDeployEvent_CreationTimestampFallback(t *testing.T) { + t.Parallel() + + created := time.Date(2026, 4, 2, 8, 0, 0, 0, time.UTC) + ev := watch.Event{ + Type: watch.Added, + Object: &corev1.Event{ + UID: "uid-created", + InvolvedObject: corev1.ObjectReference{ + Kind: "Pod", + Name: "myapp-prod-api", + }, + Type: corev1.EventTypeNormal, + Reason: "Created", + Count: 1, + CreationTimestamp: metav1.NewTime(created), + }, + } + got, ok := toDeployEvent(ev, "myapp-prod-") + require.True(t, ok) + assert.Equal(t, created.UTC(), got.Timestamp.UTC()) +} + +func TestToDeployEvent_RequiresReleaseDash(t *testing.T) { + t.Parallel() + + ev := watch.Event{ + Type: watch.Added, + Object: &corev1.Event{ + InvolvedObject: corev1.ObjectReference{ + Kind: "Pod", + Name: "myapp-prod", + }, + }, + } + _, ok := toDeployEvent(ev, "myapp-prod-") + assert.False(t, ok) +} + +func TestWatchDeployEvents_ListError(t *testing.T) { + t.Parallel() + + cs := fake.NewClientset() + cs.PrependReactor("list", "events", func(clienttesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("apiserver timeout") + }) + ch, err := WatchDeployEvents(t.Context(), cs, testNamespace, testReleasePrefix) + require.Error(t, err) + assert.Nil(t, ch) + assert.ErrorContains(t, err, `list events in "`+testNamespace+`"`) + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestWatchDeployEvents_EmptyResourceVersion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rv string + }{ + {name: "empty", rv: ""}, + {name: "zero", rv: "0"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fw := watch.NewFake() + cs := fake.NewClientset() + cs.PrependReactor("list", "events", func(clienttesting.Action) (bool, runtime.Object, error) { + return true, &corev1.EventList{ResourceVersion: tt.rv}, nil + }) + cs.PrependWatchReactor("events", func(clienttesting.Action) (bool, watch.Interface, error) { + return true, fw, nil + }) + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + ch, err := WatchDeployEvents(ctx, cs, testNamespace, testReleasePrefix) + require.NoError(t, err) + + ev := makeEvent("uid-rv", testRelease+"-api-abc", "Scheduled", "pod scheduled", corev1.EventTypeNormal, "101") + got, ok := sendAndDrain(t, fw, ch, watch.Added, ev) + require.True(t, ok) + assert.Equal(t, types.UID("uid-rv"), got.UID) + }) + } +} + +func TestWatchDeployEvents_SkipsErrorEvents(t *testing.T) { + t.Parallel() + + fw := watch.NewFake() + cs := fake.NewClientset() + cs.PrependWatchReactor("events", func(clienttesting.Action) (bool, watch.Interface, error) { + return true, fw, nil + }) + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + ch, err := WatchDeployEvents(ctx, cs, testNamespace, testReleasePrefix) + require.NoError(t, err) + + // 410 stops RetryWatcher after it forwards the error. The watcher must + // not turn that error into a deploy event. + fw.Error(&metav1.Status{ + Status: metav1.StatusFailure, + Message: "expired", + Code: http.StatusGone, + }) + select { + case _, ok := <-ch: + assert.False(t, ok, "watch errors are not deploy events") + case <-time.After(2 * time.Second): + t.Fatal("channel not closed after watch error") + } +} diff --git a/internal/k8s/jobs_test.go b/internal/k8s/jobs_test.go index 82e467b..8c6907f 100644 --- a/internal/k8s/jobs_test.go +++ b/internal/k8s/jobs_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/kubernetes/fake" "deployah.dev/deployah/internal/helm" @@ -70,7 +71,11 @@ func TestBuildTaskJob_IndexedAndLabels(t *testing.T) { assert.Equal(t, "backfill", job.Labels[spec.LabelComponent]) assert.Equal(t, "backfill", job.Labels[spec.LabelTask]) assert.Equal(t, "dev", job.Labels[spec.LabelEnvironment]) + assert.Equal(t, spec.ManagedByValue, job.Labels[spec.LabelManagedBy]) assert.Equal(t, "shop-dev", job.Labels[InstanceLabel]) + assert.Equal(t, spec.SourceSpec, job.Annotations[spec.AnnotationSource]) + assert.Equal(t, "shop", job.Annotations[spec.AnnotationProject]) + assert.Equal(t, corev1.RestartPolicyOnFailure, job.Spec.Template.Spec.RestartPolicy) require.Len(t, job.Spec.Template.Spec.Containers, 1) assert.Equal(t, "busybox:1.36", job.Spec.Template.Spec.Containers[0].Image) assert.Equal(t, []string{"backfill"}, job.Spec.Template.Spec.Containers[0].Command) @@ -82,6 +87,54 @@ func TestBuildTaskJob_IndexedAndLabels(t *testing.T) { assert.Equal(t, int32(spec.DefaultCLIJobTTLSeconds), *job.Spec.TTLSecondsAfterFinished) } +func TestBuildTaskJob_TimeoutAndBackoff(t *testing.T) { + t.Parallel() + + backoff := 1 + job, err := BuildTaskJob(TaskJobOptions{ + Project: "shop", + Environment: "dev", + Namespace: "default", + TaskName: "migrate", + Task: spec.Task{ + Image: "busybox:1.36", + Command: []string{"true"}, + Timeout: "2m", + BackoffLimit: &backoff, + }, + }) + require.NoError(t, err) + require.NotNil(t, job.Spec.ActiveDeadlineSeconds) + assert.Equal(t, int64(120), *job.Spec.ActiveDeadlineSeconds) + require.NotNil(t, job.Spec.BackoffLimit) + assert.Equal(t, int32(1), *job.Spec.BackoffLimit) +} + +func TestBuildTaskJob_OmitsUnsetContainerFields(t *testing.T) { + t.Parallel() + + job, err := BuildTaskJob(TaskJobOptions{ + Project: "shop", + Environment: "dev", + Namespace: "default", + TaskName: "migrate", + Task: spec.Task{ + Image: "busybox:1.36", + Resources: spec.Resources{ + CPU: spec.MustQuantity("0"), + Memory: spec.MustQuantity("0"), + EphemeralStorage: spec.MustQuantity("0"), + }, + }, + }) + require.NoError(t, err) + require.Len(t, job.Spec.Template.Spec.Containers, 1) + container := job.Spec.Template.Spec.Containers[0] + assert.Nil(t, container.Command) + assert.Nil(t, container.Args) + assert.Empty(t, container.Resources.Requests) +} + func TestBuildTaskJob_WildcardEnvironmentLabel(t *testing.T) { t.Parallel() @@ -220,6 +273,16 @@ func TestRunConfigMapGenerateName_FitsAfterKubernetesSuffix(t *testing.T) { jobPrefix: "shop-dev-migrate-", want: "shop-dev-migrate-run-", }, + { + name: "empty prefix", + jobPrefix: "", + want: "deployah-run-", + }, + { + name: "dash only", + jobPrefix: "-", + want: "deployah-run-", + }, { name: "long hashed name", jobPrefix: generateNamePrefix(release, task), @@ -301,7 +364,8 @@ func TestCreateTaskJob_Error(t *testing.T) { Task: spec.Task{Image: "busybox:1.36", Command: []string{"true"}}, }, "shop-dev-backfill-create")) require.Error(t, err) - assert.Contains(t, err.Error(), "quota exceeded") + assert.ErrorContains(t, err, "create job") + assert.ErrorContains(t, err, "quota exceeded") } func TestCreateTaskJob_UniqueNames(t *testing.T) { @@ -362,7 +426,8 @@ func TestWaitForJob_SuccessAndFailure(t *testing.T) { cs := fake.NewSimpleClientset(job) err := WaitForJob(t.Context(), cs, "default", "bad") require.Error(t, err) - assert.Contains(t, err.Error(), "backoff limit exceeded") + assert.ErrorContains(t, err, "wait for job bad") + assert.ErrorContains(t, err, "backoff limit exceeded") }) t.Run("failed condition without message uses reason", func(t *testing.T) { @@ -381,7 +446,8 @@ func TestWaitForJob_SuccessAndFailure(t *testing.T) { cs := fake.NewSimpleClientset(job) err := WaitForJob(t.Context(), cs, "default", "bad") require.Error(t, err) - assert.Contains(t, err.Error(), "BackoffLimitExceeded") + assert.ErrorContains(t, err, "wait for job bad") + assert.ErrorContains(t, err, "BackoffLimitExceeded") }) t.Run("nil completions treats one success as done", func(t *testing.T) { @@ -459,17 +525,64 @@ func TestWaitForJob_GivesUpAfterConsecutiveNotFound(t *testing.T) { func TestWaitForJob_PermanentGetError(t *testing.T) { t.Parallel() - cs := fake.NewSimpleClientset() - cs.PrependReactor("get", "jobs", func(action k8stesting.Action) (bool, runtime.Object, error) { - return true, nil, apierrors.NewForbidden( - schema.GroupResource{Group: "batch", Resource: "jobs"}, - "denied", - errors.New("denied"), - ) - }) - err := WaitForJob(t.Context(), cs, "default", "denied") - require.Error(t, err) - assert.ErrorContains(t, err, "wait for job") + tests := []struct { + name string + err error + }{ + { + name: "forbidden", + err: apierrors.NewForbidden( + schema.GroupResource{Group: "batch", Resource: "jobs"}, + "denied", + errors.New("denied"), + ), + }, + { + name: "unauthorized", + err: apierrors.NewUnauthorized("no"), + }, + { + name: "bad request", + err: apierrors.NewBadRequest("bad"), + }, + { + name: "invalid", + err: apierrors.NewInvalid( + schema.GroupKind{Group: "batch", Kind: "Job"}, + "denied", + field.ErrorList{field.Invalid(field.NewPath("spec"), "", "bad")}, + ), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + cs.PrependReactor("get", "jobs", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, tt.err + }) + err := WaitForJob(t.Context(), cs, "default", "denied") + require.ErrorIs(t, err, tt.err) + assert.ErrorContains(t, err, "wait for job denied") + }) + } +} + +func TestWaitForJob_Canceled(t *testing.T) { + t.Parallel() + + job := &batchv1.Job{ + Name: "slow", + Namespace: "default", + Spec: batchv1.JobSpec{Completions: new(int32(1))}, + } + cs := fake.NewSimpleClientset(job) + ctx, cancel := context.WithCancel(t.Context()) + cancel() + err := WaitForJob(ctx, cs, "default", "slow") + require.ErrorIs(t, err, context.Canceled) + assert.ErrorContains(t, err, "wait for job slow") } func TestWaitForJob_RetriesTransportError(t *testing.T) { @@ -614,11 +727,58 @@ func TestListJobs_Error(t *testing.T) { }) _, err := ListJobs(t.Context(), cs, "default", "shop", "dev") require.Error(t, err) - assert.Contains(t, err.Error(), "apiserver timeout") + assert.ErrorContains(t, err, "list jobs") + assert.ErrorContains(t, err, "apiserver timeout") err = DeleteJobs(t.Context(), cs, "default", "shop", "dev") require.Error(t, err) - assert.Contains(t, err.Error(), "apiserver timeout") + assert.ErrorContains(t, err, "list jobs") + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestListJobs_RejectsInvalidSelector(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + project string + environment string + want string + }{ + { + name: "nested wildcard", + project: "shop", + environment: "review/foo/bar", + want: `exactly one '/'`, + }, + { + name: "project is not a label value", + project: "bad value!", + environment: "dev", + want: "project label", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + listed := false + cs.PrependReactor("list", "jobs", func(k8stesting.Action) (bool, runtime.Object, error) { + listed = true + return true, nil, errors.New("should not list") + }) + _, err := ListJobs(t.Context(), cs, "default", tt.project, tt.environment) + require.Error(t, err) + assert.ErrorContains(t, err, "build job selector") + assert.ErrorContains(t, err, tt.want) + + err = DeleteJobs(t.Context(), cs, "default", tt.project, tt.environment) + require.Error(t, err) + assert.ErrorContains(t, err, tt.want) + assert.False(t, listed) + }) + } } func TestDeleteJobs(t *testing.T) { @@ -769,7 +929,7 @@ func TestBuildTaskJob_InvalidTimeout(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "timeout") + assert.ErrorContains(t, err, "timeout") } func TestBuildTaskJob_AppliesProfile(t *testing.T) { @@ -884,9 +1044,60 @@ func TestCreateRunJob_TemporaryConfigMapAndOwnerRef(t *testing.T) { require.Len(t, cm.OwnerReferences, 1) assert.Equal(t, job.Name, cm.OwnerReferences[0].Name) assert.Equal(t, job.UID, cm.OwnerReferences[0].UID) + assert.Equal(t, job.Labels, cm.Labels) + assert.Equal(t, job.Annotations, cm.Annotations) } } +func TestCreateRunJob_NoFileValuesSkipsConfigMap(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + values map[string]string + }{ + {name: "nil"}, + {name: "empty", values: map[string]string{}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + assignGenerateName(cs) + created, err := CreateRunJob(t.Context(), cs, runJob(t), tt.values) + require.NoError(t, err) + assert.NotEmpty(t, created.Name) + assert.Empty(t, created.Spec.Template.Spec.Containers[0].EnvFrom) + + cms, cmErr := cs.CoreV1().ConfigMaps("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, cmErr) + assert.Empty(t, cms.Items) + }) + } +} + +func TestCreateRunJob_ConfigMapCreateError(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + assignGenerateName(cs) + cs.PrependReactor("create", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("quota exceeded") + }) + _, err := CreateRunJob(t.Context(), cs, runJob(t), map[string]string{"REGION": "eu"}) + require.Error(t, err) + assert.ErrorContains(t, err, "create run configmap") + assert.ErrorContains(t, err, "quota exceeded") + + jobs, jobErr := cs.BatchV1().Jobs("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, jobErr) + assert.Empty(t, jobs.Items) + cms, cmErr := cs.CoreV1().ConfigMaps("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, cmErr) + assert.Empty(t, cms.Items) +} + func TestCreateRunJob_RollbackSurvivesCanceledContext(t *testing.T) { t.Parallel() @@ -982,6 +1193,112 @@ func TestCreateRunJob_RollbackDeletesJobThenConfigMap(t *testing.T) { } } +func TestCreateRunJob_CleanupFailureReportsLeftovers(t *testing.T) { + t.Parallel() + + t.Run("job create failure and configmap delete failure", func(t *testing.T) { + t.Parallel() + + jobErr := errors.New("job denied") + deleteErr := errors.New("configmap delete denied") + cs := fake.NewSimpleClientset() + assignGenerateName(cs) + cs.PrependReactor("create", "jobs", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, jobErr + }) + cs.PrependReactor("delete", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, deleteErr + }) + _, err := CreateRunJob(t.Context(), cs, runJob(t), map[string]string{"REGION": "eu"}) + require.ErrorIs(t, err, jobErr) + require.ErrorIs(t, err, deleteErr) + assert.ErrorContains(t, err, "also failed to delete configmap") + + jobs, jobListErr := cs.BatchV1().Jobs("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, jobListErr) + assert.Empty(t, jobs.Items) + cms, cmErr := cs.CoreV1().ConfigMaps("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, cmErr) + require.Len(t, cms.Items, 1) + }) + + t.Run("not found configmap delete keeps the job error", func(t *testing.T) { + t.Parallel() + + jobErr := errors.New("job denied") + cs := fake.NewSimpleClientset() + assignGenerateName(cs) + cs.PrependReactor("create", "jobs", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, jobErr + }) + cs.PrependReactor("delete", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "run") + }) + _, err := CreateRunJob(t.Context(), cs, runJob(t), map[string]string{"REGION": "eu"}) + require.ErrorIs(t, err, jobErr) + assert.ErrorContains(t, err, "create job") + assert.NotContains(t, err.Error(), "also failed") + }) + + t.Run("owner update failure and job delete failure", func(t *testing.T) { + t.Parallel() + + ownerErr := errors.New("owner denied") + deleteErr := errors.New("job delete denied") + cs := fake.NewSimpleClientset() + assignGenerateName(cs) + cs.PrependReactor("update", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, ownerErr + }) + cs.PrependReactor("delete", "jobs", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, deleteErr + }) + _, err := CreateRunJob(t.Context(), cs, runJob(t), map[string]string{"REGION": "eu"}) + require.ErrorIs(t, err, ownerErr) + require.ErrorIs(t, err, deleteErr) + assert.ErrorContains(t, err, "leftover job") + + jobs, jobListErr := cs.BatchV1().Jobs("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, jobListErr) + require.Len(t, jobs.Items, 1) + cms, cmErr := cs.CoreV1().ConfigMaps("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, cmErr) + assert.Empty(t, cms.Items) + }) + + t.Run("owner update failure and both deletes fail", func(t *testing.T) { + t.Parallel() + + ownerErr := errors.New("owner denied") + jobDeleteErr := errors.New("job delete denied") + cmDeleteErr := errors.New("configmap delete denied") + cs := fake.NewSimpleClientset() + assignGenerateName(cs) + cs.PrependReactor("update", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, ownerErr + }) + cs.PrependReactor("delete", "jobs", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, jobDeleteErr + }) + cs.PrependReactor("delete", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, cmDeleteErr + }) + _, err := CreateRunJob(t.Context(), cs, runJob(t), map[string]string{"REGION": "eu"}) + require.ErrorIs(t, err, ownerErr) + require.ErrorIs(t, err, jobDeleteErr) + require.ErrorIs(t, err, cmDeleteErr) + assert.ErrorContains(t, err, "also failed to delete job") + assert.ErrorContains(t, err, "also failed to delete configmap") + + jobs, jobListErr := cs.BatchV1().Jobs("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, jobListErr) + require.Len(t, jobs.Items, 1) + cms, cmErr := cs.CoreV1().ConfigMaps("default").List(t.Context(), metav1.ListOptions{}) + require.NoError(t, cmErr) + require.Len(t, cms.Items, 1) + }) +} + func runJob(t *testing.T) *batchv1.Job { t.Helper() job, err := BuildTaskJob(TaskJobOptions{ diff --git a/internal/k8s/pods_test.go b/internal/k8s/pods_test.go new file mode 100644 index 0000000..6139e37 --- /dev/null +++ b/internal/k8s/pods_test.go @@ -0,0 +1,251 @@ +// Copyright 2025 The Deployah Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package k8s + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + + "deployah.dev/deployah/internal/helm" + + corev1 "k8s.io/api/core/v1" + k8stesting "k8s.io/client-go/testing" +) + +func TestGetRunningPods_ReturnsMatchingPodInfo(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + podInfoPod("api-a", "apps", "shop", "api", "dev", "shop-dev", []string{"app", "sidecar"}), + podInfoPod("api-b", "apps", "shop", "api", "dev", "shop-dev", []string{"app"}), + podInfoPod("web", "apps", "shop", "web", "dev", "shop-dev", []string{"web"}), + podInfoPod("other-project", "apps", "billing", "api", "dev", "billing-dev", []string{"app"}), + podInfoPod("other-ns", "other", "shop", "api", "dev", "shop-dev", []string{"app"}), + ) + // The fake client does not apply field selectors. The running-phase + // filter is the list request sent to the API server. + var field string + cs.PrependReactor("list", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { + list, ok := action.(k8stesting.ListAction) + if ok { + field = list.GetListRestrictions().Fields.String() + } + return false, nil, nil + }) + + got, err := NewClient(cs, "apps").GetRunningPods(t.Context(), "shop", "api", "dev") + require.NoError(t, err) + assert.ElementsMatch(t, []PodInfo{ + {Name: "api-a", Namespace: "apps", Containers: []string{"app", "sidecar"}, Status: string(corev1.PodRunning)}, + {Name: "api-b", Namespace: "apps", Containers: []string{"app"}, Status: string(corev1.PodRunning)}, + }, got) + assert.Equal(t, "status.phase=Running", field) +} + +func TestGetRunningPods_NoneMatch(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + podInfoPod("web", "apps", "shop", "web", "dev", "shop-dev", []string{"web"}), + ) + got, err := NewClient(cs, "apps").GetRunningPods(t.Context(), "shop", "api", "dev") + require.NoError(t, err) + assert.Empty(t, got) +} + +func TestGetRunningPods_LogicalEnvironmentIncludesInstances(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + podInfoPod("logical", "apps", "shop", "api", "review", helm.GenerateReleaseName("shop", "review"), []string{"app"}), + podInfoPod("pr-123", "apps", "shop", "api", "review", helm.GenerateReleaseName("shop", "review/pr-123"), []string{"app"}), + podInfoPod("web", "apps", "shop", "web", "review", helm.GenerateReleaseName("shop", "review"), []string{"web"}), + ) + got, err := NewClient(cs, "apps").GetRunningPods(t.Context(), "shop", "api", "review") + require.NoError(t, err) + assert.ElementsMatch(t, []PodInfo{ + {Name: "logical", Namespace: "apps", Containers: []string{"app"}, Status: string(corev1.PodRunning)}, + {Name: "pr-123", Namespace: "apps", Containers: []string{"app"}, Status: string(corev1.PodRunning)}, + }, got) +} + +func TestGetRunningPods_WildcardInstanceExcludesSiblings(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + podInfoPod("logical", "apps", "shop", "api", "review", helm.GenerateReleaseName("shop", "review"), []string{"app"}), + podInfoPod("pr-123", "apps", "shop", "api", "review", helm.GenerateReleaseName("shop", "review/pr-123"), []string{"app"}), + podInfoPod("pr-456", "apps", "shop", "api", "review", helm.GenerateReleaseName("shop", "review/pr-456"), []string{"app"}), + ) + got, err := NewClient(cs, "apps").GetRunningPods(t.Context(), "shop", "api", "review/pr-123") + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, "pr-123", got[0].Name) +} + +func TestGetRunningPods_InvalidEnvironment(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + listed := false + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + listed = true + return true, nil, errors.New("should not list") + }) + _, err := NewClient(cs, "apps").GetRunningPods(t.Context(), "shop", "api", "review/PR-123") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to build selector") + assert.ErrorContains(t, err, "lowercase DNS label") + assert.False(t, listed) +} + +func TestGetRunningPods_ListError(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("apiserver timeout") + }) + _, err := NewClient(cs, "apps").GetRunningPods(t.Context(), "shop", "api", "dev") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to list pods") + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestGetPodInfo(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + podInfoPod("api-a", "apps", "shop", "api", "dev", "shop-dev", []string{"app", "sidecar"}), + ) + client := NewClient(cs, "apps") + + got, err := client.GetPodInfo(t.Context(), "api-a") + require.NoError(t, err) + assert.Equal(t, &PodInfo{ + Name: "api-a", + Namespace: "apps", + Containers: []string{"app", "sidecar"}, + Status: string(corev1.PodRunning), + }, got) + + _, err = client.GetPodInfo(t.Context(), "missing") + require.Error(t, err) + assert.ErrorContains(t, err, "failed to get pod missing") +} + +func TestGetPodStatus_CountsReadyRunningPods(t *testing.T) { + t.Parallel() + + release := "shop-dev" + cs := fake.NewSimpleClientset( + statusPod("ready", "apps", release, corev1.PodRunning, []corev1.ContainerStatus{{Name: "app", Ready: true}}), + statusPod("not-ready", "apps", release, corev1.PodRunning, []corev1.ContainerStatus{{Name: "app", Ready: false}}), + statusPod("pending", "apps", release, corev1.PodPending, []corev1.ContainerStatus{{Name: "app", Ready: true}}), + statusPod("succeeded", "apps", release, corev1.PodSucceeded, []corev1.ContainerStatus{{Name: "app", Ready: true}}), + // A running pod with no container statuses counts as ready: the check + // returns true when every reported status is ready, including none. + statusPod("no-status", "apps", release, corev1.PodRunning, nil), + statusPod("other-release", "apps", "shop-prod", corev1.PodRunning, []corev1.ContainerStatus{{Name: "app", Ready: true}}), + statusPod("other-ns", "other", release, corev1.PodRunning, []corev1.ContainerStatus{{Name: "app", Ready: true}}), + ) + + total, ready, summary, err := NewClient(cs, "apps").GetPodStatus(t.Context(), release) + require.NoError(t, err) + assert.Equal(t, 5, total) + assert.Equal(t, 2, ready) + assert.Equal(t, "2/5", summary) +} + +func TestGetPodStatus_ListError(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("apiserver timeout") + }) + total, ready, summary, err := NewClient(cs, "apps").GetPodStatus(t.Context(), "shop-dev") + require.Error(t, err) + assert.Equal(t, 0, total) + assert.Equal(t, 0, ready) + assert.Equal(t, "0/0", summary) + assert.ErrorContains(t, err, "list pods for release shop-dev") + assert.ErrorContains(t, err, "apiserver timeout") +} + +func TestGetPodStatus_InvalidReleaseName(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset() + listed := false + cs.PrependReactor("list", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + listed = true + return true, nil, errors.New("should not list") + }) + total, ready, summary, err := NewClient(cs, "apps").GetPodStatus(t.Context(), "bad value!") + require.Error(t, err) + assert.Equal(t, 0, total) + assert.Equal(t, 0, ready) + assert.Equal(t, "0/0", summary) + assert.ErrorContains(t, err, "build selector for release bad value!") + assert.False(t, listed) +} + +func TestValidatePodExists(t *testing.T) { + t.Parallel() + + cs := fake.NewSimpleClientset( + podInfoPod("api-a", "apps", "shop", "api", "dev", "shop-dev", []string{"app"}), + ) + client := NewClient(cs, "apps") + require.NoError(t, client.ValidatePodExists(t.Context(), "api-a")) + + err := client.ValidatePodExists(t.Context(), "missing") + require.Error(t, err) + assert.ErrorContains(t, err, "pod missing not found") +} + +func podInfoPod(name, namespace, project, component, environment, instance string, containers []string) *corev1.Pod { + specContainers := make([]corev1.Container, 0, len(containers)) + for _, container := range containers { + specContainers = append(specContainers, corev1.Container{Name: container}) + } + return &corev1.Pod{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + ProjectLabel: project, + ComponentLabel: component, + EnvironmentLabel: environment, + InstanceLabel: instance, + }, + Spec: corev1.PodSpec{Containers: specContainers}, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + } +} + +func statusPod(name, namespace, release string, phase corev1.PodPhase, statuses []corev1.ContainerStatus) *corev1.Pod { + return &corev1.Pod{ + Name: name, + Namespace: namespace, + Labels: map[string]string{InstanceLabel: release}, + Status: corev1.PodStatus{Phase: phase, ContainerStatuses: statuses}, + } +} diff --git a/internal/k8s/selectors_test.go b/internal/k8s/selectors_test.go index 234b68a..a314178 100644 --- a/internal/k8s/selectors_test.go +++ b/internal/k8s/selectors_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/labels" "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/spec" @@ -50,7 +49,7 @@ func TestSelectorBuilder_WithProject(t *testing.T) { got, err := NewSelectorBuilder().WithProject(tt.value) if tt.wantErr { require.Error(t, err) - assert.Contains(t, err.Error(), tt.errContains) + assert.ErrorContains(t, err, tt.errContains) return } require.NoError(t, err) @@ -65,12 +64,18 @@ func TestSelectorBuilder_WithComponent(t *testing.T) { t.Parallel() tests := []struct { - name string - value string - want string + name string + value string + want string + wantErr bool + errContains string }{ {name: "empty is a no-op", value: "", want: ""}, {name: "valid component sets requirement", value: "api", want: "deployah.dev/component=api"}, + { + name: "value with invalid label characters returns error", value: "bad value!", + wantErr: true, errContains: "failed to create component label requirement", + }, } for _, tt := range tests { @@ -78,6 +83,45 @@ func TestSelectorBuilder_WithComponent(t *testing.T) { t.Parallel() got, err := NewSelectorBuilder().WithComponent(tt.value) + if tt.wantErr { + require.Error(t, err) + assert.ErrorContains(t, err, tt.errContains) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got.Build()) + }) + } +} + +func TestSelectorBuilder_WithInstance(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value string + want string + wantErr bool + errContains string + }{ + {name: "empty is a no-op", value: "", want: ""}, + {name: "valid instance sets requirement", value: "shop-dev", want: "app.kubernetes.io/instance=shop-dev"}, + { + name: "value with invalid label characters returns error", value: "bad value!", + wantErr: true, errContains: "failed to create instance label requirement", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := NewSelectorBuilder().WithInstance(tt.value) + if tt.wantErr { + require.Error(t, err) + assert.ErrorContains(t, err, tt.errContains) + return + } require.NoError(t, err) assert.Equal(t, tt.want, got.Build()) }) @@ -90,9 +134,11 @@ func TestSelectorBuilder_WithEnvironment(t *testing.T) { t.Parallel() tests := []struct { - name string - value string - want string + name string + value string + want string + wantErr bool + errContains string }{ {name: "empty is a no-op", value: "", want: ""}, {name: "plain environment name is unchanged", value: "production", want: "deployah.dev/environment=production"}, @@ -100,6 +146,10 @@ func TestSelectorBuilder_WithEnvironment(t *testing.T) { name: "wildcard environment uses logical map key", value: "review/pr-42", want: "deployah.dev/environment=review", }, + { + name: "value with invalid label characters returns error", value: "bad value!", + wantErr: true, errContains: "failed to create environment label requirement", + }, } for _, tt := range tests { @@ -107,6 +157,11 @@ func TestSelectorBuilder_WithEnvironment(t *testing.T) { t.Parallel() got, err := NewSelectorBuilder().WithEnvironment(tt.value) + if tt.wantErr { + require.Error(t, err) + assert.ErrorContains(t, err, tt.errContains) + return + } require.NoError(t, err) assert.Equal(t, tt.want, got.Build()) }) @@ -151,12 +206,20 @@ func TestBuildSelector(t *testing.T) { {name: "project only", project: "my-app", want: "deployah.dev/project=my-app"}, {name: "component only", component: "api", want: "deployah.dev/component=api"}, {name: "environment only", environment: "production", want: "deployah.dev/environment=production"}, - {name: "all set combines requirements", project: "my-app", component: "api", environment: "staging"}, + { + name: "all set combines requirements", project: "my-app", component: "api", environment: "staging", + want: "deployah.dev/component=api,deployah.dev/environment=staging,deployah.dev/project=my-app", + }, { name: "invalid project value returns error before building the rest", project: "invalid project!", component: "api", environment: "staging", wantErr: true, errContains: "failed to create project label requirement", }, + { + name: "invalid component value returns error", + project: "my-app", component: "bad value!", environment: "staging", + wantErr: true, errContains: "failed to create component label requirement", + }, } for _, tt := range tests { @@ -167,19 +230,10 @@ func TestBuildSelector(t *testing.T) { if tt.wantErr { require.Error(t, err) assert.Empty(t, got) - assert.Contains(t, err.Error(), tt.errContains) + assert.ErrorContains(t, err, tt.errContains) return } require.NoError(t, err) - if tt.project != "" && tt.component != "" && tt.environment != "" { - // All three requirements are present; assert containment - // rather than exact string, since selector ordering is - // an implementation detail. - assert.Contains(t, got, "deployah.dev/project="+tt.project) - assert.Contains(t, got, "deployah.dev/component="+tt.component) - assert.Contains(t, got, "deployah.dev/environment="+tt.environment) - return - } assert.Equal(t, tt.want, got) }) } @@ -218,19 +272,12 @@ func TestBuildComponentSelector(t *testing.T) { name string project string component string - check func(t *testing.T, got string) + want string }{ - { - name: "both empty", project: "", component: "", - check: func(t *testing.T, got string) { t.Helper(); assert.Empty(t, got) }, - }, + {name: "both empty", project: "", component: "", want: ""}, { name: "both set", project: "my-app", component: "api", - check: func(t *testing.T, got string) { - t.Helper() - assert.Contains(t, got, "deployah.dev/project=my-app") - assert.Contains(t, got, "deployah.dev/component=api") - }, + want: "deployah.dev/component=api,deployah.dev/project=my-app", }, } @@ -240,7 +287,7 @@ func TestBuildComponentSelector(t *testing.T) { got, err := BuildComponentSelector(tt.project, tt.component) require.NoError(t, err) - tt.check(t, got) + assert.Equal(t, tt.want, got) }) } } @@ -254,38 +301,16 @@ func TestBuildLabelSelector(t *testing.T) { name string project string environment string + want string wantErr bool errContains string - check func(t *testing.T, sel labels.Selector) }{ - { - name: "both empty matches everything", - // labels.NewSelector() returns a nil-backed internalSelector, - // so sel is never a bare nil interface even here; assert on - // Empty() instead of nilness. - check: func(t *testing.T, sel labels.Selector) { t.Helper(); assert.True(t, sel.Empty()) }, - }, - { - name: "project only", project: "my-app", - check: func(t *testing.T, sel labels.Selector) { - t.Helper() - assert.Equal(t, "deployah.dev/project=my-app", sel.String()) - }, - }, - { - name: "environment only uses logical map key", environment: "review/pr-7", - check: func(t *testing.T, sel labels.Selector) { - t.Helper() - assert.Equal(t, "deployah.dev/environment=review", sel.String()) - }, - }, + {name: "both empty matches everything", want: ""}, + {name: "project only", project: "my-app", want: "deployah.dev/project=my-app"}, + {name: "environment only uses logical map key", environment: "review/pr-7", want: "deployah.dev/environment=review"}, { name: "both set", project: "my-app", environment: "production", - check: func(t *testing.T, sel labels.Selector) { - t.Helper() - assert.Contains(t, sel.String(), "deployah.dev/project=my-app") - assert.Contains(t, sel.String(), "deployah.dev/environment=production") - }, + want: "deployah.dev/environment=production,deployah.dev/project=my-app", }, { name: "invalid project value returns error", project: "bad value!", @@ -293,7 +318,7 @@ func TestBuildLabelSelector(t *testing.T) { }, { name: "invalid environment value returns error", environment: "bad value!", - wantErr: true, errContains: "invalid", + wantErr: true, errContains: "environment name 'bad value!' is invalid", }, } @@ -305,11 +330,11 @@ func TestBuildLabelSelector(t *testing.T) { if tt.wantErr { require.Error(t, err) assert.Nil(t, sel) - assert.Contains(t, err.Error(), tt.errContains) + assert.ErrorContains(t, err, tt.errContains) return } require.NoError(t, err) - tt.check(t, sel) + assert.Equal(t, tt.want, sel.String()) }) } } @@ -341,11 +366,11 @@ func TestBuildSelector_InvalidWildcardRejected(t *testing.T) { _, err := BuildSelector("shop", "api", "review/PR-123") require.Error(t, err) - assert.Contains(t, err.Error(), "lowercase DNS label") + assert.ErrorContains(t, err, "lowercase DNS label") _, err = BuildLabelSelector("shop", "review/foo/bar") require.Error(t, err) - assert.Contains(t, err.Error(), "exactly one '/'") + assert.ErrorContains(t, err, "exactly one '/'") } func TestEnvironmentFromMeta(t *testing.T) { @@ -356,7 +381,8 @@ func TestEnvironmentFromMeta(t *testing.T) { annotations map[string]string want string }{ - {name: "missing annotation"}, + {name: "nil annotations"}, + {name: "empty map", annotations: map[string]string{}}, { name: "plain production", annotations: map[string]string{ diff --git a/internal/k8s/tlscert_test.go b/internal/k8s/tlscert_test.go index ccff4f0..46fb07c 100644 --- a/internal/k8s/tlscert_test.go +++ b/internal/k8s/tlscert_test.go @@ -17,16 +17,19 @@ package k8s import ( "crypto/tls" "crypto/x509" + "errors" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" "deployah.dev/deployah/internal/spec" corev1 "k8s.io/api/core/v1" + k8stesting "k8s.io/client-go/testing" certutil "k8s.io/client-go/util/cert" ) @@ -86,10 +89,21 @@ func TestEnsureSelfSignedCert(t *testing.T) { wrongHostCert, wrongHostKey, err := GenerateSelfSignedCert("other.example.com") require.NoError(t, err) + stillValidCert, stillValidKey, err := certutil.GenerateSelfSignedCertKeyWithOptions(certutil.SelfSignedCertKeyOptions{ + Host: tlsTestFQDN, + AlternateDNS: []string{tlsTestFQDN}, + MaxAge: 31 * 24 * time.Hour, // just outside the 30-day renew window + }) + require.NoError(t, err) + + _, otherKey, err := GenerateSelfSignedCert(tlsTestFQDN) + require.NoError(t, err) + tests := []struct { name string seed *corev1.Secret wantReuse []byte // when set, returned cert must equal this + wantKey []byte wantRegenerate []byte // when set, returned cert must differ from this checkLeaf bool }{ @@ -100,6 +114,13 @@ func TestEnsureSelfSignedCert(t *testing.T) { name: "reuses existing valid secret", seed: tlsSecret(validCert, validKey, corev1.SecretTypeTLS), wantReuse: validCert, + wantKey: validKey, + }, + { + name: "reuses a certificate outside the renew window", + seed: tlsSecret(stillValidCert, stillValidKey, corev1.SecretTypeTLS), + wantReuse: stillValidCert, + wantKey: stillValidKey, }, { name: "regenerates when expiring soon", @@ -113,6 +134,23 @@ func TestEnsureSelfSignedCert(t *testing.T) { wantRegenerate: wrongHostCert, checkLeaf: true, }, + { + name: "regenerates when the key does not match", + seed: tlsSecret(validCert, otherKey, corev1.SecretTypeTLS), + wantRegenerate: validCert, + checkLeaf: true, + }, + { + name: "regenerates when cert bytes are empty", + seed: tlsSecret(nil, validKey, corev1.SecretTypeTLS), + checkLeaf: true, + }, + { + name: "regenerates for invalid PEM", + seed: tlsSecret([]byte("not-pem"), []byte("not-pem"), corev1.SecretTypeTLS), + wantRegenerate: []byte("not-pem"), + checkLeaf: true, + }, { name: "regenerates for non-TLS secret type", seed: tlsSecret([]byte("not-actually-tls"), []byte("not-actually-tls"), corev1.SecretTypeOpaque), @@ -140,7 +178,7 @@ func TestEnsureSelfSignedCert(t *testing.T) { if tt.wantReuse != nil { assert.Equal(t, tt.wantReuse, gotCert, "existing valid cert must be reused unchanged") - assert.Equal(t, validKey, gotKey, "existing valid key must be reused unchanged") + assert.Equal(t, tt.wantKey, gotKey, "existing valid key must be reused unchanged") } if tt.wantRegenerate != nil { assert.NotEqual(t, tt.wantRegenerate, gotCert, "must regenerate instead of reuse") @@ -199,3 +237,110 @@ func TestMaterializeSelfSignedTLS(t *testing.T) { assert.Equal(t, keyPEM, resolved.Components["web"].TLSKeyPEM) }) } + +func TestEnsureSelfSignedCert_GetError(t *testing.T) { + t.Parallel() + + cs := fake.NewClientset() + cs.PrependReactor("get", "secrets", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unavailable") + }) + certPEM, keyPEM, err := EnsureSelfSignedCert(t.Context(), cs, testNamespace, tlsTestFQDN) + require.Error(t, err) + assert.Nil(t, certPEM) + assert.Nil(t, keyPEM) + assert.ErrorContains(t, err, "get TLS secret "+tlsTestFQDN+"-tls") + assert.ErrorContains(t, err, "unavailable") +} + +func TestEnsureSelfSignedCert_OtherNamespaceGenerates(t *testing.T) { + t.Parallel() + + certPEM, keyPEM, err := GenerateSelfSignedCert(tlsTestFQDN) + require.NoError(t, err) + secret := tlsSecret(certPEM, keyPEM, corev1.SecretTypeTLS) + secret.Namespace = "other" + gotCert, gotKey, err := EnsureSelfSignedCert(t.Context(), fake.NewClientset(secret), testNamespace, tlsTestFQDN) + require.NoError(t, err) + assert.NotEqual(t, certPEM, gotCert) + assert.NotEqual(t, keyPEM, gotKey) + _, err = tls.X509KeyPair(gotCert, gotKey) + require.NoError(t, err) +} + +func TestHasSelfSignedComponents(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + resolved *spec.ResolvedSpec + want bool + }{ + {name: "nil"}, + {name: "no components", resolved: &spec.ResolvedSpec{}}, + { + name: "self signed without fqdn", + resolved: &spec.ResolvedSpec{Components: map[string]spec.ResolvedComponent{ + "web": {TLSMode: spec.TLSModeSelfSigned}, + }}, + }, + { + name: "cert manager", + resolved: &spec.ResolvedSpec{Components: map[string]spec.ResolvedComponent{ + "api": {FQDN: "api.example.com", TLSMode: spec.TLSModeCertManager}, + }}, + }, + { + name: "self signed with fqdn", + resolved: &spec.ResolvedSpec{Components: map[string]spec.ResolvedComponent{ + "web": {FQDN: tlsTestFQDN, TLSMode: spec.TLSModeSelfSigned}, + }}, + want: true, + }, + { + name: "one self signed component is enough", + resolved: &spec.ResolvedSpec{Components: map[string]spec.ResolvedComponent{ + "api": {FQDN: "api.example.com", TLSMode: spec.TLSModeCertManager}, + "web": {FQDN: tlsTestFQDN, TLSMode: spec.TLSModeSelfSigned}, + }}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, HasSelfSignedComponents(tt.resolved)) + }) + } +} + +func TestMaterializeSelfSignedTLS_SkipsEmptyFQDN(t *testing.T) { + t.Parallel() + + resolved := &spec.ResolvedSpec{Components: map[string]spec.ResolvedComponent{ + "web": {TLSMode: spec.TLSModeSelfSigned}, + "api": {FQDN: tlsTestFQDN, TLSMode: spec.TLSModeSelfSigned}, + }} + require.NoError(t, MaterializeSelfSignedTLS(t.Context(), nil, testNamespace, resolved)) + assert.Empty(t, resolved.Components["web"].TLSCertPEM) + assert.Empty(t, resolved.Components["web"].TLSKeyPEM) + assert.NotEmpty(t, resolved.Components["api"].TLSCertPEM) + assert.NotEmpty(t, resolved.Components["api"].TLSKeyPEM) +} + +func TestMaterializeSelfSignedTLS_SecretGetError(t *testing.T) { + t.Parallel() + + cs := fake.NewClientset() + cs.PrependReactor("get", "secrets", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unavailable") + }) + resolved := &spec.ResolvedSpec{Components: map[string]spec.ResolvedComponent{ + "web": {FQDN: tlsTestFQDN, TLSMode: spec.TLSModeSelfSigned}, + }} + err := MaterializeSelfSignedTLS(t.Context(), cs, testNamespace, resolved) + require.Error(t, err) + assert.ErrorContains(t, err, "component web") + assert.ErrorContains(t, err, "get TLS secret") + assert.Empty(t, resolved.Components["web"].TLSCertPEM) +} diff --git a/internal/k8s/version_test.go b/internal/k8s/version_test.go index 3d74074..19765c0 100644 --- a/internal/k8s/version_test.go +++ b/internal/k8s/version_test.go @@ -15,76 +15,125 @@ package k8s import ( + "errors" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/version" "k8s.io/client-go/kubernetes/fake" fakediscovery "k8s.io/client-go/discovery/fake" + k8stesting "k8s.io/client-go/testing" ) -func fakeClientWithVersion(major, minor string) *fake.Clientset { +func fakeClientWithVersion(tb testing.TB, major, minor string) *fake.Clientset { + tb.Helper() cs := fake.NewSimpleClientset() fd, ok := cs.Discovery().(*fakediscovery.FakeDiscovery) - if !ok { - panic("expected *fakediscovery.FakeDiscovery") - } + require.True(tb, ok) fd.FakedServerVersion = &version.Info{Major: major, Minor: minor} return cs } func TestCheckMinimumVersion_Pass(t *testing.T) { t.Parallel() - err := CheckMinimumVersion(fakeClientWithVersion("1", "32"), 1, 32, "kind: stateful") + err := CheckMinimumVersion(fakeClientWithVersion(t, "1", "32"), 1, 32, "kind: stateful") assert.NoError(t, err) } func TestCheckMinimumVersion_HigherPass(t *testing.T) { t.Parallel() - err := CheckMinimumVersion(fakeClientWithVersion("1", "33+"), 1, 32, "kind: stateful") + err := CheckMinimumVersion(fakeClientWithVersion(t, "1", "33+"), 1, 32, "kind: stateful") + assert.NoError(t, err) +} + +func TestCheckMinimumVersion_NewerMajorPasses(t *testing.T) { + t.Parallel() + err := CheckMinimumVersion(fakeClientWithVersion(t, "2", "0"), 1, 32, "kind: stateful") assert.NoError(t, err) } func TestCheckMinimumVersion_Fail(t *testing.T) { t.Parallel() - err := CheckMinimumVersion(fakeClientWithVersion("1", "31"), 1, 32, "kind: stateful requires Kubernetes 1.32+") + err := CheckMinimumVersion(fakeClientWithVersion(t, "1", "31"), 1, 32, "kind: stateful requires Kubernetes 1.32+") + require.Error(t, err) + assert.ErrorContains(t, err, "below required 1.32") + assert.ErrorContains(t, err, "kind: stateful") +} + +func TestCheckMinimumVersion_OlderMajorFails(t *testing.T) { + t.Parallel() + err := CheckMinimumVersion(fakeClientWithVersion(t, "0", "40"), 1, 32, "kind: stateful") + require.Error(t, err) + assert.ErrorContains(t, err, "version 0.40 is below required 1.32") + assert.ErrorContains(t, err, "kind: stateful") +} + +func TestCheckMinimumVersion_DiscoveryError(t *testing.T) { + t.Parallel() + cs := fake.NewSimpleClientset() + fd, ok := cs.Discovery().(*fakediscovery.FakeDiscovery) + require.True(t, ok) + fd.PrependReactor("get", "version", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unreachable") + }) + err := CheckMinimumVersion(cs, 1, 32, "kind: stateful") require.Error(t, err) - assert.Contains(t, err.Error(), "below required 1.32") - assert.Contains(t, err.Error(), "kind: stateful") + assert.ErrorContains(t, err, "discover cluster version") + assert.ErrorContains(t, err, "unreachable") } func TestCheckMinimumVersion_ParseError(t *testing.T) { t.Parallel() - err := CheckMinimumVersion(fakeClientWithVersion("x", "32"), 1, 32, "reason") + err := CheckMinimumVersion(fakeClientWithVersion(t, "x", "32"), 1, 32, "reason") require.Error(t, err) - assert.Contains(t, err.Error(), "parse cluster major version") + assert.ErrorContains(t, err, `parse cluster major version "x"`) } func TestParseVersionPart(t *testing.T) { t.Parallel() tests := []struct { + name string in string want int }{ - {"1", 1}, - {"32", 32}, - {"32+", 32}, - {"32.0", 32}, + {name: "major", in: "1", want: 1}, + {name: "minor", in: "32", want: 32}, + {name: "plus suffix", in: "32+", want: 32}, + {name: "dotted", in: "32.0", want: 32}, + {name: "whitespace and plus", in: " 33+ ", want: 33}, + {name: "dash suffix", in: "32-gke.1", want: 32}, } for _, tt := range tests { - got, err := parseVersionPart(tt.in) - require.NoError(t, err, tt.in) - assert.Equal(t, tt.want, got, tt.in) + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := parseVersionPart(tt.in) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) } } func TestParseVersionPart_Empty(t *testing.T) { t.Parallel() - _, err := parseVersionPart("") - require.Error(t, err) - assert.Contains(t, err.Error(), "empty version part") + tests := []struct { + name string + in string + }{ + {name: "empty", in: ""}, + {name: "whitespace", in: " "}, + {name: "plus only", in: "+"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := parseVersionPart(tt.in) + require.Error(t, err) + assert.ErrorContains(t, err, "empty version part") + }) + } } func TestParseVersionPart_NonNumeric(t *testing.T) { @@ -95,7 +144,15 @@ func TestParseVersionPart_NonNumeric(t *testing.T) { func TestCheckMinimumVersion_MinorParseError(t *testing.T) { t.Parallel() - err := CheckMinimumVersion(fakeClientWithVersion("1", "abc"), 1, 32, "reason") + err := CheckMinimumVersion(fakeClientWithVersion(t, "1", "abc"), 1, 32, "reason") + require.Error(t, err) + assert.ErrorContains(t, err, `parse cluster minor version "abc"`) +} + +func TestCheckMinimumVersion_EmptyMinor(t *testing.T) { + t.Parallel() + err := CheckMinimumVersion(fakeClientWithVersion(t, "1", ""), 1, 32, "reason") require.Error(t, err) - assert.Contains(t, err.Error(), "parse cluster minor version") + assert.ErrorContains(t, err, `parse cluster minor version ""`) + assert.ErrorContains(t, err, "empty version part") }